Skip to content

Optional commas in dependency lists #1509

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

Closed
hdgarrood opened this issue Sep 17, 2013 · 8 comments
Closed

Optional commas in dependency lists #1509

hdgarrood opened this issue Sep 17, 2013 · 8 comments

Comments

@hdgarrood
Copy link
Contributor

hdgarrood commented Sep 17, 2013

From this page:

Related to above, allow layout for lists of tokens, to make the comma
optional in lists of deps if they're on separate lines. Allow trailing
commas?

I'm interested in having a go at implementing this. Has anything already been done? Should I just get stuck in?

@dcoutts
Copy link
Contributor

dcoutts commented Sep 19, 2013

Yes I think this is ok. What I don't want is for us to end up with a syntax where you need to know the grammar of the list elements just to be able to split the field into elements. That is, I want all of our list fields to be able to be semi-parsed -- just by knowing it is supposed to be a list and not knowing the grammar of the list elements.

The one thing to watch out for is whether in the existing parser, if dependency expressions that span multiple lines are currently allowed, and if there are .cabal files on hackage that make use of that. If so, then we would need to make this format change be dependent on the spec version given in the cabal-version field.

Related to all this: I'm writing a new parser for .cabal files. Currently stalled on needing a dependency on parsec.

@hdgarrood
Copy link
Contributor Author

I suppose it would make sense to wait until this new parser is at least partially implemented, then?

The way I see it, this format should require a minimum cabal-version of the first version to contain this feature, regardless of what's already on Hackage. (Or have I misunderstood you?)

Explanation: assume that this change doesn't require a minimum cabal-version present, and someone writes this .cabal file using a version of cabal which does have this feature:

name: test
version: 0.0.1
build-type: Simple

executable test
    -- note: no comma after "base" here
    build-depends: base
                   random

Anyone trying to parse this with an earlier version of cabal will get a "parse failed" error. A "newer cabal version required" error would be more user-friendly and appropriate.

@nikita-volkov
Copy link

+1!

@osa1
Copy link

osa1 commented Feb 24, 2015

Does anyone have any news on that? Is the work on new parser done?

@phadej
Copy link
Collaborator

phadej commented Jan 31, 2018

There is #4971, so you can write:

executable test
    -- note: no comma after "base" here
    build-depends:
      , base
      , random

to make your build-depends sort and diff friendly.

@23Skidoo
Copy link
Member

Do we have a changelog note for this? Otherwise looks like this ticket can be closed as fixed.

@phadej
Copy link
Collaborator

phadej commented Jan 31, 2018

cabal/Cabal/changelog

Lines 12 to 13 in 6750de6

* Fields with mandatory commas (e.g. build-depends) may now have a
leading or a trailing comma (either one, not both) (#4953)

@23Skidoo
Copy link
Member

Closing as fixed by #4971.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants