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

Quoting problem when there are several targets #701

Closed
ghost opened this issue Apr 12, 2018 · 8 comments · Fixed by ocaml/opam-repository#12323
Closed

Quoting problem when there are several targets #701

ghost opened this issue Apr 12, 2018 · 8 comments · Fixed by ocaml/opam-repository#12323
Assignees
Milestone

Comments

@ghost
Copy link

ghost commented Apr 12, 2018

Problem exposed by the test added in #699

@ghost
Copy link
Author

ghost commented Apr 17, 2018

I suggest that we use the following model, that I find slightly simpler than the current one:

A variable always expand to a list of values. When a variable expands to exactly 1 value, there is no ambiguity and it can appear anywhere. When it expands to 0 or more than 2 values, it can appear:

  • either inside a quoted string, in which case it expands to the list of values concatenated by spaces
  • either as a whole atom inside a variadic functions, in which case it expands to one argument per value

@Chris00
Copy link
Member

Chris00 commented Apr 17, 2018

I'll try to have a look later today (in the evening CET).

@Chris00
Copy link
Member

Chris00 commented Apr 17, 2018

I'm afraid it'll have to wait for tomorrow.

@ghost
Copy link
Author

ghost commented May 17, 2018

@Chris00 do you think you'll have some time to look at this in the coming weeks? I'm thinking that it'd be nice to sort this out before we release Dune 1.0.0.

@Chris00
Copy link
Member

Chris00 commented May 17, 2018

I'm thinking that it'd be nice to sort this out before we release Dune 1.0.0.

I agree. I'll try to find the time but there are a couple of things I definitely need to finish first...

@ghost ghost mentioned this issue May 31, 2018
@rgrinberg rgrinberg added this to the 1.0.0 milestone May 31, 2018
@rgrinberg
Copy link
Member

@Chris00 I'll take over this if you don't mind. I'd like to have this fixed to support version comparisons.

@Chris00
Copy link
Member

Chris00 commented May 31, 2018

@rgrinberg Thanks — I'm doing too many things ATM...

@rgrinberg rgrinberg self-assigned this Jun 1, 2018
@ghost ghost mentioned this issue Jun 2, 2018
@rgrinberg
Copy link
Member

Fixed in master.

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

Successfully merging a pull request may close this issue.

2 participants