Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
## Unreleased
- Where applicable, use `elif` in Cabal files (introduced in
`cabal-version: 2.2`) (see #605)

## Changes in 0.38.0
- Generate `build-tool-depends` instead of `build-tools` starting with
Expand Down
29 changes: 29 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,35 @@ becomes
else
ghc-options: -O0

Conditionals with a `else` field that contains only one `when` field will make
use of `elif` in Cabal files (introduced in `cabal-version: 2.2`).

For example,

when:
- condition: os(windows)
then:
source-dirs: windows
else:
when:
- condition: "os(darwin) || os(linux)"
then:
source-dirs: unix-like
else:
source-dirs: unsupported-os

becomes

if os(windows)
hs-source-dirs:
windows
elif os(darwin) || os(linux)
hs-source-dirs:
unix-like
else
hs-source-dirs:
unsupported-os

Copy link
Owner

Choose a reason for hiding this comment

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

I don't think that this is relevant user documentation. Nested if-statements and elif are semantically equivalent. As I understand it, this is a purely cosmetic change.

I say, either remove this completely, or add a separate section that describes how .cabal files are formatted. This section could then also include what I mentioned here: #606 (comment)

**Note:** Conditionals with `condition: false` are omitted from the generated
`.cabal` file.

Expand Down
74 changes: 73 additions & 1 deletion src/Hpack/Config.hs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,9 @@ module Hpack.Config (
, VerbatimValue(..)
, verbatimValueToString
, CustomSetup(..)
, HasEmpty(..)
, Section(..)
, maybeSectionAConditional
, Library(..)
, Executable(..)
, Conditional(..)
Expand Down Expand Up @@ -527,6 +529,9 @@ instance Monoid Empty where
mempty = Empty
mappend = (<>)

instance HasEmpty Empty where
empty = Empty

instance Semigroup Empty where
Empty <> Empty = Empty

Expand Down Expand Up @@ -868,7 +873,7 @@ ensureRequiredCabalVersion inferredLicense pkg@Package{..} = pkg {
executableHasGeneratedModules :: Section Executable -> Bool
executableHasGeneratedModules = any (not . null . executableGeneratedModules)

sectionCabalVersion :: (Section a -> [Module]) -> Section a -> Maybe CabalVersion
sectionCabalVersion :: (Eq a, HasEmpty a) => (Section a -> [Module]) -> Section a -> Maybe CabalVersion
sectionCabalVersion getMentionedModules sect = maximum $ [
makeVersion [2,2] <$ guard (sectionSatisfies (not . null . sectionCxxSources) sect)
, makeVersion [2,2] <$ guard (sectionSatisfies (not . null . sectionCxxOptions) sect)
Expand All @@ -880,6 +885,7 @@ ensureRequiredCabalVersion inferredLicense pkg@Package{..} = pkg {
uses "RebindableSyntax"
&& (uses "OverloadedStrings" || uses "OverloadedLists")
&& pathsModule `elem` getMentionedModules sect)
, makeVersion [2,2] <$ guard (sectionSatisfies (isJust . maybeSectionAConditional) sect)
Copy link
Owner

Choose a reason for hiding this comment

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

What this is saying is that if a nested conditional can be transformed into an elif, then require Cabal 2.2.

What I think you would want to do is the other way around: If the rendered .cabal file has cabal-version: 2.2 (or later) then produce elifs.

] ++ map versionFromSystemBuildTool systemBuildTools
where
defaultExtensions :: [String]
Expand Down Expand Up @@ -1040,6 +1046,10 @@ data CustomSetup = CustomSetup {
customSetupDependencies :: Dependencies
} deriving (Eq, Show)

-- | A class for types that have a value corresponding to being \'empty\'.
class HasEmpty a where
empty :: a

data Library = Library {
libraryExposed :: Maybe Bool
, libraryVisibility :: Maybe String
Expand All @@ -1050,12 +1060,30 @@ data Library = Library {
, librarySignatures :: [String]
} deriving (Eq, Show)

instance HasEmpty Library where
empty = Library
{ libraryExposed = Nothing
, libraryVisibility = Nothing
, libraryExposedModules = []
, libraryOtherModules = []
, libraryGeneratedModules = []
, libraryReexportedModules = []
, librarySignatures = []
}

data Executable = Executable {
executableMain :: Maybe FilePath
, executableOtherModules :: [Module]
, executableGeneratedModules :: [Module]
} deriving (Eq, Show)

instance HasEmpty Executable where
empty = Executable
{ executableMain = Nothing
, executableOtherModules = []
, executableGeneratedModules = []
}

data BuildTool = BuildTool String String | LocalBuildTool String
deriving (Show, Eq, Ord)

Expand Down Expand Up @@ -1093,6 +1121,41 @@ data Section a = Section {
, sectionVerbatim :: [Verbatim]
} deriving (Eq, Show, Functor, Foldable, Traversable)

instance HasEmpty a => HasEmpty (Section a) where
empty = Section
{ sectionData = Hpack.Config.empty
, sectionSourceDirs = []
, sectionDependencies = mempty
, sectionPkgConfigDependencies = []
, sectionDefaultExtensions = []
, sectionOtherExtensions = []
, sectionLanguage = Nothing
, sectionGhcOptions = []
, sectionGhcProfOptions = []
, sectionGhcSharedOptions = []
, sectionGhcjsOptions = []
, sectionCppOptions = []
, sectionAsmOptions = []
, sectionAsmSources = []
, sectionCcOptions = []
, sectionCSources = []
, sectionCxxOptions = []
, sectionCxxSources = []
, sectionJsSources = []
, sectionExtraLibDirs = []
, sectionExtraLibraries = []
, sectionExtraFrameworksDirs = []
, sectionFrameworks = []
, sectionIncludeDirs = []
, sectionInstallIncludes = []
, sectionLdOptions = []
, sectionBuildable = Nothing
, sectionConditionals = []
, sectionBuildTools = mempty
, sectionSystemBuildTools = mempty
, sectionVerbatim = []
}

data Conditional a = Conditional {
conditionalCondition :: Cond
, conditionalThen :: a
Expand Down Expand Up @@ -1657,3 +1720,12 @@ pathsModuleFromPackageName name = Module ("Paths_" ++ map f name)
where
f '-' = '_'
f x = x

-- | If the given section contains only a single conditional, yields just that
-- conditional, otherwise 'Nothing'.
maybeSectionAConditional :: (Eq a, HasEmpty a) => Section a -> Maybe (Conditional (Section a))
maybeSectionAConditional s@(Section { sectionConditionals = [c] }) =
if s == Hpack.Config.empty { sectionConditionals = sectionConditionals s }
then Just c
else Nothing
maybeSectionAConditional _ = Nothing
18 changes: 12 additions & 6 deletions src/Hpack/Render.hs
Original file line number Diff line number Diff line change
Expand Up @@ -240,7 +240,7 @@ renderExposed = Field "exposed" . Literal . show
renderVisibility :: String -> Element
renderVisibility = Field "visibility" . Literal

renderSection :: (a -> [Element]) -> [Element] -> Section a -> RenderM [Element]
renderSection :: (Eq a, HasEmpty a) => (a -> [Element]) -> [Element] -> Section a -> RenderM [Element]
renderSection renderSectionData extraFieldsStart Section{..} = do
buildTools <- renderBuildTools sectionBuildTools sectionSystemBuildTools
conditionals <- traverse (renderConditional renderSectionData) sectionConditionals
Expand Down Expand Up @@ -307,12 +307,18 @@ renderVerbatimObject = map renderPair . Map.toList
[x] -> Field key (Literal x)
xs -> Field key (LineSeparatedList xs)

renderConditional :: (a -> [Element]) -> Conditional (Section a) -> RenderM Element
renderConditional renderSectionData (Conditional condition sect mElse) = case mElse of
Nothing -> if_
Just else_ -> Group <$> if_ <*> (Stanza "else" <$> renderSection renderSectionData [] else_)
renderConditional :: (Eq a, HasEmpty a) => (a -> [Element]) -> Conditional (Section a) -> RenderM Element
renderConditional = renderConditional' "if "
where
if_ = Stanza ("if " ++ renderCond condition) <$> renderSection renderSectionData [] sect
renderConditional' stanza renderSectionData (Conditional condition sect mElse) = case mElse of
Nothing -> stanza_
Just else_ -> Group <$> stanza_ <*> case maybeSectionAConditional else_ of
Just conditional ->
renderConditional' "elif " renderSectionData conditional
Nothing ->
Stanza "else" <$> renderSection renderSectionData [] else_
where
stanza_ = Stanza (stanza ++ renderCond condition) <$> renderSection renderSectionData [] sect

renderCond :: Cond -> String
renderCond = \ case
Expand Down
30 changes: 30 additions & 0 deletions test/EndToEndSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -1802,6 +1802,36 @@ spec = around_ (inTempDirectoryNamed "my-package") $ do
build-depends:
unix
|]
it "uses elif when applicable and infers cabal-version 2.2" $ do
[i|
when:
condition: os(windows)
then:
source-dirs: windows
else:
when:
condition: "os(darwin) || os(linux)"
then:
source-dirs: unix-like
else:
source-dirs: unsupported-os
executable: {}
|] `shouldRenderTo` (executable "my-package" [i|
other-modules:
Paths_my_package
autogen-modules:
Paths_my_package
default-language: Haskell2010
if os(windows)
hs-source-dirs:
windows
elif os(darwin) || os(linux)
hs-source-dirs:
unix-like
else
hs-source-dirs:
unsupported-os
|]) {packageCabalVersion = "2.2"}

context "with empty then-branch" $ do
it "provides a hint" $ do
Expand Down
15 changes: 15 additions & 0 deletions test/Hpack/RenderSpec.hs
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,21 @@ spec = do
, " unix"
]

it "renders conditionals with else-branch using elif when applicable" $ do
let conditional = Conditional "os(windows)" (section Empty) {sectionSourceDirs = ["windows"]} $ Just (section Empty) {sectionConditionals = [innerConditional]}
innerConditional = Conditional "os(darwin) || os(linux)" (section Empty) {sectionSourceDirs = ["unix-like"]} $ Just (section Empty) {sectionSourceDirs = ["unsupported-os"]}
render defaultRenderSettings 0 (run $ renderConditional renderEmptySection conditional) `shouldBe` [
"if os(windows)"
, " hs-source-dirs:"
, " windows"
, "elif os(darwin) || os(linux)"
, " hs-source-dirs:"
, " unix-like"
, "else"
, " hs-source-dirs:"
, " unsupported-os"
]

it "renders nested conditionals" $ do
let conditional = Conditional "arch(i386)" (section Empty) {sectionGhcOptions = ["-threaded"], sectionConditionals = [innerConditional]} Nothing
innerConditional = Conditional "os(windows)" (section Empty) {sectionDependencies = deps ["Win32"]} Nothing
Expand Down