Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#361] Put all warnings under ifs to always have them all #409

Merged
merged 2 commits into from
Feb 21, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
[#361] Put all warnings under ifs to always have them all
Resolves #361
  • Loading branch information
chshersh committed Feb 20, 2020
commit 0effe8cc1f16a2d198fee342109c6843b46cf475
2 changes: 2 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,10 @@ script:

echo "Testing that generated projects are built with cabal..."
cd "$TRAVIS_BUILD_DIR/summoner-cli/test/golden/smallProject/"
echo "packages: ." > cabal.project
cabal v2-build all
cd "$TRAVIS_BUILD_DIR/summoner-cli/test/golden/fullProject/"
echo "packages: ." > cabal.project
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clever fix!

cabal v2-build all
else
stack test --system-ghc --no-terminal
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ The changelog is available [on GitHub][2].
* [#363](https://github.com/kowainik/summoner/issues/363):
Move from `generic-deriving` to `generic-data`.
(by [@chshersh](https://github.com/chshersh))
* [#361](https://github.com/kowainik/summoner/issues/361):
Always put all warnings in `ghc-options` inside common stanza under `if`s.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add an example of if in here if you don't mind 🙂
Also, maybe makes sense to mention that the condition is on GHC in these ifs? Or only one of these two options is fine as well, what do you think?

(by [@chshersh](https://github.com/chshersh))
* Use `colourista` for pretty terminal formatting.
(by [@chshersh](https://github.com/chshersh))
* __#TUI__ Allow `brick-0.52`.
Expand Down
18 changes: 9 additions & 9 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,15 +100,15 @@ Features related to the structure and content of the generated projects.

```
-Wall
-Wincomplete-uni-patterns
-Wincomplete-record-updates
-Wcompat
-Widentities
-Wredundant-constraints (GHC >= 8.0)
-fhide-source-paths (GHC >= 8.2.2)
-Wmissing-export-lists (GHC >= 8.4.1)
-Wpartial-fields (GHC >= 8.4.1)
-Wmissing-deriving-strategies (GHC >= 8.8.1)
-Wincomplete-uni-patterns
-Wincomplete-record-updates
-Wredundant-constraints (GHC ⩾ 8.0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

niiice 😼 💅

-fhide-source-paths (GHC ⩾ 8.2)
-Wmissing-export-lists (GHC ⩾ 8.4)
-Wpartial-fields (GHC ⩾ 8.4)
-Wmissing-deriving-strategies (GHC ⩾ 8.8)
```

Besides, the following GHC options are added to the executable, tests and benchmark stanzas:
Expand Down Expand Up @@ -190,8 +190,8 @@ To start using Summoner make sure that you have the following tools installed on

We also have minimal version requirements for build tools:

* [Cabal >= 2.4](https://www.haskell.org/cabal/)
* [Stack >= 2.1](http://haskellstack.org)
* [Cabal 2.4](https://www.haskell.org/cabal/)
* [Stack 2.1.3](http://haskellstack.org)

However, it is always recommended to use the newest versions of build tools.

Expand Down
16 changes: 5 additions & 11 deletions summoner-cli/src/Summoner/License.hs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ to work with them.
module Summoner.License
( LicenseName(..)
, License(..)
, cabalLicense
, customizeLicense
, githubLicenseQueryNames
, parseLicenseName
Expand Down Expand Up @@ -41,7 +40,7 @@ data LicenseName
| AGPL3
| Apache20
| MPL20
| None
| NONE
deriving stock (Eq, Ord, Enum, Bounded, Generic)

instance Show LicenseName where
Expand All @@ -55,7 +54,7 @@ instance Show LicenseName where
show AGPL3 = "AGPL-3"
show Apache20 = "Apache-2.0"
show MPL20 = "MPL-2.0"
show None = "None"
show NONE = "NONE"

newtype License = License
{ unLicense :: Text
Expand All @@ -65,11 +64,6 @@ newtype License = License
instance FromJSON License where
parseJSON = withObject "License" $ \o -> License <$> o .: "body"

-- | As it will be shown in the @cabal@ file.
cabalLicense :: LicenseName -> Text
cabalLicense None = "AllRightsReserved"
cabalLicense l = show l

Comment on lines -68 to -72
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That is a nice simplification!

-- | Used for downloading the license text form @Github@.
githubLicenseQueryNames :: LicenseName -> Text
githubLicenseQueryNames = \case
Expand All @@ -83,7 +77,7 @@ githubLicenseQueryNames = \case
AGPL3 -> "agpl-3.0"
Apache20 -> "apache-2.0"
MPL20 -> "mpl-2.0"
None -> "none"
NONE -> "none"

parseLicenseName :: Text -> Maybe LicenseName
parseLicenseName = inverseMap show
Expand All @@ -103,7 +97,7 @@ customizeLicense l license@(License licenseText) nm year
in beforeY <> year <> beforeN <> nm <> afterN

fetchLicense :: LicenseName -> IO License
fetchLicense None = pure $ License $ licenseShortDesc None
fetchLicense NONE = pure $ License $ licenseShortDesc NONE
fetchLicense name = do
let licenseLink = "https://api.github.com/licenses/" <> githubLicenseQueryNames name
licenseJson <- "curl" $|
Expand All @@ -130,7 +124,7 @@ licenseShortDesc = \case
AGPL3 -> "GNU Affero General Public License, version 3"
Apache20 -> "Apache License, version 2.0"
MPL20 -> "Mozilla Public License, version 2.0."
None -> "License file won't be added. Explicitly 'All Rights Reserved', eg \
NONE -> "License file won't be added. Explicitly 'All Rights Reserved', eg \
\for proprietary software. The package may not be legally modified or \
\redistributed by anyone but the rightsholder"

Expand Down
2 changes: 1 addition & 1 deletion summoner-cli/src/Summoner/Project.hs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ generateProject isOffline projectName Config{..} = do

putText licenseText
settingsLicenseName <- if isOffline
then None <$ infoMessage "'AllRightsReserved' license is used in offline mode"
then NONE <$ infoMessage "'NONE' license is used in offline mode"
else choose parseLicenseName "License: " $ ordNub (cLicense : universe)

-- License creation
Expand Down
72 changes: 33 additions & 39 deletions summoner-cli/src/Summoner/Template/Cabal.hs
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ import NeatInterpolation (text)

import Summoner.CustomPrelude (CustomPrelude (..))
import Summoner.Default (defaultCabal)
import Summoner.GhcVer (GhcVer (..), cabalBaseVersions, showGhcVer)
import Summoner.License (LicenseName (..), cabalLicense)
import Summoner.GhcVer (cabalBaseVersions, showGhcVer)
import Summoner.License (LicenseName (..))
import Summoner.Settings (Settings (..))
import Summoner.Template.Mempty (memptyIfFalse)
import Summoner.Text (endLine, intercalateMap, packageToModule)
Expand Down Expand Up @@ -52,7 +52,7 @@ cabalFile Settings{..} = File (toString settingsRepo ++ ".cabal") cabalFileConte
[ "homepage: " <> githubUrl | settingsGitHub ] ++
[ "bug-reports: " <> githubBugReports | settingsGitHub ] ++
( "license: " <> licenseName) :
[ "license-file: LICENSE" | settingsLicenseName /= None] ++
[ "license-file: LICENSE" | settingsLicenseName /= NONE] ++
[ "author: " <> settingsFullName
, "maintainer: " <> settingsEmail
, "copyright: " <> settingsYear <> " " <> settingsFullName ] ++
Expand All @@ -68,7 +68,7 @@ cabalFile Settings{..} = File (toString settingsRepo ++ ".cabal") cabalFileConte
githubBugReports = githubUrl <> "/issues"

licenseName, libModuleName :: Text
licenseName = cabalLicense settingsLicenseName
licenseName = show settingsLicenseName
libModuleName = packageToModule settingsRepo

testedGhcs :: Text
Expand All @@ -86,7 +86,6 @@ cabalFile Settings{..} = File (toString settingsRepo ++ ".cabal") cabalFileConte
location: ${githubUrl}.git
|]


commonStanza :: Text
commonStanza =
[text|
Expand All @@ -95,15 +94,42 @@ cabalFile Settings{..} = File (toString settingsRepo ++ ".cabal") cabalFileConte
build-depends: $settingsBaseType $baseBounds
$commaPreludeLibrary

ghc-options: -Wall
$ghcOptions
$ghcOptions

default-language: Haskell2010
|] <> defaultExtensions

baseBounds :: Text
baseBounds = cabalBaseVersions settingsTestedVersions

ghcOptions :: Text
ghcOptions = case settingsGhcOptions of
[] -> defaultGhcOptions
x:xs ->
let customGhcOptions = T.intercalate "\n" $ x : map (T.replicate 21 " " <>) xs in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could intercalateMap function be handy in here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it can help here indeed! I missed that

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or not.. looking into it I see, that the logic here is a bit different. Sorry, false alarm!

[text|
ghc-options: $customGhcOptions
|]

defaultGhcOptions :: Text
defaultGhcOptions =
[text|
ghc-options: -Wall
-Wcompat
-Widentities
-Wincomplete-uni-patterns
-Wincomplete-record-updates
if impl(ghc >= 8.0)
ghc-options: -Wredundant-constraints
if impl(ghc >= 8.2)
ghc-options: -fhide-source-paths
if impl(ghc >= 8.4)
ghc-options: -Wmissing-export-lists
-Wpartial-fields
if impl(ghc >= 8.8)
ghc-options: -Wmissing-deriving-strategies
|]

libraryStanza :: Text
libraryStanza =
[text|
Expand Down Expand Up @@ -180,35 +206,3 @@ cabalFile Settings{..} = File (toString settingsRepo ++ ".cabal") cabalFileConte
xs -> " default-extensions: "
<> T.intercalate "\n " xs
<> "\n"

ghcOptions :: Text
ghcOptions = case settingsGhcOptions of
[] -> defaultGhcOptions
xs -> T.intercalate "\n" xs

defaultGhcOptions :: Text
defaultGhcOptions =
[text|
-Wincomplete-uni-patterns
-Wincomplete-record-updates
-Wcompat
-Widentities
$versionGhcOptions
|]

versionGhcOptions :: Text
versionGhcOptions
= memptyIfFalse (settingsTestedVersions `hasLeast` Ghc802)
"-Wredundant-constraints\n"
<> memptyIfFalse (settingsTestedVersions `hasLeast` Ghc822)
"-fhide-source-paths\n"
<> memptyIfFalse (settingsTestedVersions `hasLeast` Ghc844)
[text|
-Wmissing-export-lists
-Wpartial-fields
|]
<> memptyIfFalse (settingsTestedVersions `hasLeast` Ghc882)
"-Wmissing-deriving-strategies\n"
where
hasLeast :: [GhcVer] -> GhcVer -> Bool
hasLeast list el = all (>= el) list
4 changes: 2 additions & 2 deletions summoner-cli/src/Summoner/Template/Doc.hs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module Summoner.Template.Doc
( docFiles
) where

import Summoner.License (License (..), LicenseName (None))
import Summoner.License (License (..), LicenseName (NONE))
import Summoner.Settings (Settings (..))
import Summoner.Tree (TreeFs (..))

Expand All @@ -30,7 +30,7 @@ docFiles Settings{..} =
maybeToList (File "CONTRIBUTING.md" <$> settingsContributing)
where
hasLicense :: Bool
hasLicense = settingsLicenseName /= None
hasLicense = settingsLicenseName /= NONE

licenseName :: Text
licenseName = show settingsLicenseName
Expand Down
2 changes: 1 addition & 1 deletion summoner-cli/test/Test/Golden.hs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ smallProject = Settings
, settingsEmail = "xrom.xkov@gmail.com"
, settingsYear = "2018"
, settingsCategories = ""
, settingsLicenseName = None
, settingsLicenseName = NONE
, settingsLicenseText = ""
, settingsGitHub = False
, settingsGhActions = False
Expand Down
3 changes: 1 addition & 2 deletions summoner-cli/test/golden/fullProject/fullProject.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ common common-options
build-depends: base-noprelude >= 4.9.1.0 && < 4.14
, relude

ghc-options: -Wall
-Wcompat
ghc-options: -Wcompat
-Widentities

default-language: Haskell2010
Expand Down
18 changes: 11 additions & 7 deletions summoner-cli/test/golden/smallProject/smallProject.cabal
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ name: smallProject
version: 0.0.0.0
synopsis: Small test project
description: Small test project
license: AllRightsReserved
license: NONE
author: Kowainik
maintainer: xrom.xkov@gmail.com
copyright: 2018 Kowainik
Expand All @@ -17,15 +17,19 @@ common common-options


ghc-options: -Wall
-Wincomplete-uni-patterns
-Wincomplete-record-updates
-Wcompat
-Widentities
-Wredundant-constraints
-fhide-source-paths
-Wmissing-export-lists
-Wincomplete-uni-patterns
-Wincomplete-record-updates
if impl(ghc >= 8.0)
ghc-options: -Wredundant-constraints
if impl(ghc >= 8.2)
ghc-options: -fhide-source-paths
if impl(ghc >= 8.4)
ghc-options: -Wmissing-export-lists
-Wpartial-fields
-Wmissing-deriving-strategies
if impl(ghc >= 8.8)
ghc-options: -Wmissing-deriving-strategies

default-language: Haskell2010

Expand Down
2 changes: 1 addition & 1 deletion summoner-tui/src/Summoner/Tui/Kit.hs
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ configToSummonKit cRepo cOffline cConfigFile files Config{..} = SummonKit
{ projectRepo = cRepo
, projectDesc = defaultDescription
, projectCategory = ""
, projectLicense = if cOffline then None else cLicense
, projectLicense = if cOffline then NONE else cLicense
}
, summonKitProjectMeta = ProjectMeta
{ projectMetaLib = kitLib
Expand Down