Parse error on "mixins" field

[pikajude]

The mixins field doesn't parse if the provided renaming clause has whitespace after the opening parenthesis.

mixins: example-mixin ( Mixin1 as Foo.Mixin1
                      , Mixin2 as Foo.Mixin2
                      )

$ readGenericPackageDescription normal "test.txt"
Warning: test.txt:6:26:
unexpected " "
expecting ")"

example-mixin ( Mixin1 as Foo.Mixin1
, Mixin2 as Foo.Mixin2
)
*** Exception: dieVerbatim: user error (<interactive>: Failed parsing "test.txt".

If I delete the space in ( Mixin1 it parses fine. I feel like the given example should parse correctly since it would make vertically formatted long renaming clauses nicer-looking.

If this isn't intended behavior, I can PR the patch I've made locally.

[phadej]

Can you test what cabal-install-2.0 does, (e.g. `cabal check`).

…

Sent from my iPhone

On 15 Feb 2018, at 21.08, Jude Taylor ***@***.***> wrote: The mixins field doesn't parse if the provided renaming clause has whitespace after the opening parenthesis. mixins: example-mixin ( Mixin1 as Foo.Mixin1 , Mixin2 as Foo.Mixin2 ) $ readGenericPackageDescription normal "test.txt" Warning: test.txt:6:26: unexpected " " expecting ")" example-mixin ( Mixin1 as Foo.Mixin1 , Mixin2 as Foo.Mixin2 ) *** Exception: dieVerbatim: user error (<interactive>: Failed parsing "test.txt". If I delete the space in ( Mixin1 it parses fine. I feel like the given example should parse correctly since it would make vertically formatted long renaming clauses nicer-looking. If this isn't intended behavior, I can PR the patch I've made locally. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[pikajude]

$ cabal --version && cabal check
cabal-install version 2.0.0.1
compiled using version 2.0.1.0 of the Cabal library
cabal: ./test.cabal:6: Parse of field 'mixins' failed.

[phadej]

As old cabal-install fails too, new lenient parser have to be guarded with `cabal-version` check. We have to be bug compatible :/

…

Sent from my iPhone

On 15 Feb 2018, at 21.18, Jude Taylor ***@***.***> wrote: $ cabal-install version 2.0.0.1 compiled using version 2.0.1.0 of the Cabal library cabal: ./test.cabal:6: Parse of field 'mixins' failed. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[pikajude]

Which version of Cabal is supposed to be able to parse my example? If I specify cabal-version: 2.2 at the top of the file, the parse error still happens, and 2.3 isn't a supported version.

[ezyang]

Sorry, what @phadej was saying that it's not enough to just fix the parser, we also have to make sure the fix only applies when you have a sufficiently new cabal-version (probably 2.4 since we've missed this merge Window.)

@phadej, is this really necessary? It's certainly not a BC-breaking change, which is what I thought cabal-version was for.

[phadej]

@ezyang It's breaking change. If there will be a .cabal file requiring relaxed parser and cabal-version: 2.0, it will break cabal-install-2.0 as example above shows.

Similar "small" thing as #5092 #5093 #5138 (EDIT removed wrong issue from the list)

[fgaz]

Was this fixed in #5293 ?