-
Notifications
You must be signed in to change notification settings - Fork 696
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
caret operator accepts a set of versions #5906
Conversation
build-depends: network ^>= { 2.6.3.6, 2.7.0.2, 2.8.0.0, 3.0.1.0 }
@tseenshe look at leading-comma tests in https://github.com/haskell/cabal/tree/master/Cabal/tests/ParserTests/errors and https://github.com/haskell/cabal/tree/master/Cabal/tests/ParserTests/regressions (note that only And after you make a first commit, you'll need to |
-- plain version without tags or wildcards | ||
verPlain :: CabalParsing m => m Version | ||
verPlain = mkVersion <$> P.sepBy1 P.integral (P.char '.') | ||
|
||
-- either wildcard or normal version | ||
verOrWild :: CabalParsing m => m (Bool, Version) | ||
verOrWild = do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@hvr if you look at this, this doesn't differentiate between 01.02.03
and 1.2.3
given different cabal-version
.
I'll change that, hopefully nothing slipped on Hackage
http://hackage.haskell.org/package/Cabal-2.4.1.0/docs/src/Distribution.Types.Version.html#line-99
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See also #5138 where we took some measures to prevent denorm versions hitting hackage
base ==1 || ==2, | ||
base ==1.2, | ||
base ==1.2 || ==3.4, | ||
ghc ==8.6.3 || ==8.4.4 || ==8.2.2 || ==8.0.2 || ==7.10.3 || ==7.8.4 || ==7.6.3 || ==7.4.2, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ooh, hmm...
@hvr should we introduce a VersionRange
constructor for set notation? I'd say yes. And can do it after this is merged.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We'd need NonEmpty
to do it, so it's "a bit" tricky
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main benefit would be for syntactical round tripping, wouldn't it? Don't we already have other cases such as -none
where we have a syntactically lossy encoding?
I wonder if we should instead go for having two different datatypes, one for the concrete parsetree (which is better for error reporting and/or pretty printing), and one for the desugared abstract syntax tree (which is more convenient for computations). The current VersionRange
type is a bit inconsistent in this regard IMO (i.e. it has support for representing parens which are imo redundant for an expression tree, but otoh it lacks the ability to represent -none
directly).
In any case, I think we should discuss this in a new separate "design ticket" and we also still have the issue of having to handle pkg-config version-ranges which might be related.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
I've found the
^>=
operator to be quite verbose to use when I am supporting multiple ghcs (or stackage snapshots). I found this cool post from ages ago https://www.reddit.com/r/haskell/comments/7tykqi/should_stackage_ignore_version_bounds/dtgp0v0 which looked good. e.g.I don't know how to write a test. Somebody please help?
Please include the following checklist in your PR:
Please also shortly describe how you tested your change. Bonus points for added tests!