cabal-gild icon indicating copy to clipboard operation
cabal-gild copied to clipboard

Bad conditional formatting

Open tfausak opened this issue 1 year ago • 4 comments
trafficstars

if (impl(ghc >= 9.2)&& impl(ghc < 9.3))

There should be a space before &&.

tfausak avatar May 14 '24 02:05 tfausak

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

tfausak avatar May 14 '24 03:05 tfausak

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 avatar May 14 '24 12:05 tfausak

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 avatar May 21 '24 13:05 tfausak

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

tfausak avatar May 21 '24 13:05 tfausak