cabalism / hpack-dhall

Use hpack's phrasing in dhall to write cabal files
BSD 3-Clause "New" or "Revised" License
31 stars 3 forks source link

remove `null` from generated JSON #39

Open sellout opened 1 month ago

sellout commented 1 month ago

I‘m trying to conditionalize some non-list values, like

  if impl(ghc >= 9.10.1)
    main-is: skipped.hs
  else
    main-is: doctests.hs

Encoding this in Dhall, I end up with

when =
  [ { condition = "impl(ghc >= 9.10.1"
    , then = hpack.emptyBuildOptions // { main = Some "skipped.hs" }
    , else = hpack.emptyBuildOptions // { main = Some "doctests.hs" }
    }
  ]

and emptyBuildOptions (which allows me to have a list of conditions that don’t all have to specify every field) looks like

      { cpp-options = [] : List Text
      , default-extensions = [] : List Text
      , dependencies = [] : List Text
      , ghc-options = [] : List Text
      , main = None Text
      }

The problem is that when there are conditions that don’t specify main, it ends up with main = None Text, which becomes "main": null in the generated JSON, and then hpack (reasonably) complains that it doesn’t know what to do with a null.

I think the solution here is to change dhallToJSON expr in https://github.com/cabalism/hpack-dhall/blob/a1b799e770fb6dffd47a29324617462ab86f368e/library/Hpack/Dhall.hs#L129-L135 to Dhall.JSON.omitNull (dhallToJSON expr).

It might make sense to use Dhall.JSON.omitEmpty instead of omitNull, but I haven’t thought through the ramifications of that.

sellout commented 1 month ago

omitEmpty is the wrong thing. E.g.,

{ condition = "impl(ghc >= 9.10.1)"
, `then` = { ghc-options = "-Wall" }
, `else` = { }
}

produces

if impl(ghc >= 9.10.1)
  ghc-options = "-Wall"
else

which is valid Cabal. But with omitEmpty, the else is dropped, and then hpack complains about an invalid conditional.

The conditional could alternatively be written

{ condition = "impl(ghc >= 9.10.1)"
, ghc-options = "-Wall"
}

but I don’t think there’s any reason to force the user to do that.