-
Notifications
You must be signed in to change notification settings - Fork 5
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
else/elif seem unsupported? #109
Comments
Thanks for reporting this issue! I think there's some confusion about how conditionals work in package descriptions. Prior to Cabal 2.2, if flag(a)
constraints: tar +static
else
if flag(b)
constraints: tar -static
else
if flag(c)
constraints: tar +static +win32 Note that nothing else can go on the line with Starting with Cabal 2.2, the new cabal-version: 2.2
if flag(a)
constraints: tar +static
elif flag(b)
constraints: tar -static
elif flag(c)
constraints: tar +static +win32 |
On Thu, Sep 12, 2024 at 06:40:39AM GMT, Taylor Fausak wrote:
Note that nothing else can go on the line with `else`.
Ah, right!
```cabal
if os(mingw32)
constraints: tar +static
else
if os(mingw32)
constraints: tar -static
```
indeed correctly gives
```cabal
if os(windows)
constraints: tar +static
else
if os(windows)
constraints: tar -static
```
So `cabal-gild` is correctly not accepting an `else` section with anything on
its line (though it'd be nice if it either warned about this or would
automatically fix it).
Starting with Cabal 2.2, the new `elif` section was allowed.
Fine. These are the possibilities I see for this:
- cabal-gild pretty-prints the `elif`, but warns if `cabal-version` is not
specified to be at least 2.2
- cabal-gild automatically bumps the `cabal-version` to be at least 2.2 when
an `elif` section is encountered
- cabal-gild desugars the `elif` if the in-file `cabal-version < 2.2` and
pretty-prints it otherwise
- the same, but controlled by a command-line argument of what cabal version to
target
Given the project goals, and given we probably want to avoid being too clever,
the last two options don't look very promising. Either of the first two is good
by me.
|
Gild is very intentionally only a formatter. It does not aim to provide any validation or linting. Cabal itself will emit an error if you provide any arguments to cabal-version: 2.0
name: example
version: 0
build-type: Simple
library
if False
x-whatever: 0
else if False
x-whatever: 1
And it will warn about using cabal-version: 2.0
name: example
version: 0
build-type: Simple
library
if False
x-whatever: 0
elif False
x-whatever: 1
|
On Thu, Sep 12, 2024 at 09:46:23AM GMT, Taylor Fausak wrote:
Gild is very intentionally only a formatter. It does not aim to provide any validation or linting. `else foo` is syntactically valid, so Gild formats it. Similarly, `elif` is syntactically valid even for older Cabal versions, so Gild formats it.
OK. However, the formatting for `elif` as a directive starting a conditional
section, versus a generic section-starting directive, *is* something where Gild
could err on the side of newer cabal formats on, correct?
My other musings on emitting the correct cabal-version aside, we could pretend
`elif` is a supported directive, and have cabal catch if the usage was
incorrect.
(This would remove `else if -> elif` from the table, which is a compromise I'm
willing to make)
|
Are you asking for Gild to always format Once the user notices the warning from Cabal, presumably they'll edit their package description to use a recent-enough Cabal version. Then all they have to do is re-run Gild and it should be formatted correctly. It's also worth noting that |
I'm saying that |
But -- input
cabal-version: 2.2
elif flag ( BLAH )
x-blah: true -- output
cabal-version: 2.2
elif flag(blah)
x-blah: true There's also a test for this case: cabal-gild/source/test-suite/Main.hs Lines 660 to 663 in f30f35e
|
I could've sworn I could reproduce the issue last week, but looking back at my
scrollback and the initial file, it seems I wasn't noticing the absence of a
`cabal-version` header. My bad, sorry for the noise!
|
No problem! Sorry for the confusion. |
Consider the following MWE:
It is currently formatted as
where I would expect it to be formatted as
(note the spacing!)
The text was updated successfully, but these errors were encountered: