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

Bad conditional formatting #72

Closed
tfausak opened this issue May 14, 2024 · 4 comments · Fixed by #83
Closed

Bad conditional formatting #72

tfausak opened this issue May 14, 2024 · 4 comments · Fixed by #83
Labels
bug Something isn't working

Comments

@tfausak
Copy link
Owner

tfausak commented May 14, 2024

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

There should be a space before &&.

@tfausak
Copy link
Owner Author

tfausak commented May 14, 2024

Discovered here: haskell/haskell-language-server#4229 (comment)

@tfausak
Copy link
Owner Author

tfausak commented May 14, 2024

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
Copy link
Owner Author

tfausak commented May 21, 2024

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
Copy link
Owner Author

tfausak commented May 21, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant