tfausak / cabal-gild

:crown: Format Haskell package descriptions.
https://hackage.haskell.org/package/cabal-gild
MIT License
47 stars 5 forks source link

Bad conditional formatting #72

Closed tfausak closed 3 months ago

tfausak commented 4 months ago
if (impl(ghc >= 9.2)&& impl(ghc < 9.3))

There should be a space before &&.

tfausak commented 4 months ago

Discovered here: https://github.com/haskell/haskell-language-server/pull/4229#discussion_r1599311876

tfausak commented 4 months ago

Rather than my janky Chunk type, I think it makes sense to use the proper Condition ConfVar for this.

https://hackage.haskell.org/package/Cabal-syntax-3.12.0.0/docs/Distribution-Fields-ConfVar.html#v:parseConditionConfVar

It will require a bit of a rework to parse the arguments for if (and elif when cabal-version: >= 2.2), but I think it's worth it.

tfausak commented 4 months ago

Unfortunately the Condition type doesn't have a CParen constructor. That means it can't losslessly round-trip a conditional like this:

if !((x || y) && z)

The AST will be correct, but simply rendering it will give you this:

if !x || y && z

That could potentially be fixed by using a showsPrec-style renderer. However sometimes users insert parentheses that aren't strictly necessary. I think Gild should retain those. So I'll probably need to write my own Condition type, along with a parser and pretty printer.

https://github.com/haskell/cabal/blob/4072eb8d658a250af35ae45c65da77f59eca3a5e/doc/cabal-package-description-file.rst#conditional-blocks

tfausak commented 4 months ago

The various ConfVar types also have problems for me. Both OS and Arch have the concept of "strictness". The parser picks a strictness for me, so I can't make it permissive (unless it happens to already be that way). This is a problem because they have aliases. For example, AArch64 is the actual constructor, but it can allow an arm64 alias. I would prefer to keep whatever the user gave without translation. If that's not possible, I'd prefer to be as lax as possible, meaning allowing all aliases and converting them into their canonical form.

(Also they're case-insensitive, but that's not controlled by the strictness.)

https://hackage.haskell.org/package/Cabal-syntax-3.12.0.0/docs/Distribution-System.html#t:ClassificationStrictness