On 13/04/2018 15:48, Feld Boris wrote:
On 12/04/2018 13:09, Yuya Nishihara wrote:
On Thu, 12 Apr 2018 11:32:23 +0200, Feld Boris wrote:
On 11/04/2018 17:16, Yuya Nishihara wrote:
On Wed, 11 Apr 2018 11:36:05 +0200, Feld Boris wrote:
The proposal here is to define a prefix for which we break backward
compatibility. If we do so, people with a "<set>" label will have to use:

      "<set>":whatever

to get a similar effect.
IIRC x:y was the most important syntax that needed a strong BC guarantee, so
this proposal doesn't sound good.
Indeed, the `x:y` is important and we don't want to break the BC
guarantee of it.

The proposal is less impacting, only people using 'set' as labels and
using it at the beginning of a revsetwould be impacted. This prefix has
the advantage of being concise and coherent with whatfilesetuse.
Doesn't '-r set:foo' look like a range?

I don't like an idea of introducing another ambiguous syntax to resolve
ambiguity, but furthermore "set:foo" seems confusing to humans.

IIUC, we have "set:" for filesets only because that's the syntax to specify
file patterns. If we really want to add something to force revset evaluation,
I think it has to be compatible with the current syntax, such as "(EXPR)" or
"revset(EXPR)".
`(EXPR)` seemtoo likely to introduce a BC breakage, as people are more
likely to have a tag that looks like `(xxx)` than a 'set' tag IMHO.
Ugh, I never assumed that would actually happen, but since you face to more
real users having various backgrounds than me, I might be wrong.

In a previous discussion, you pointed out that `revset(EXPR)` would be
painful because we would have to escape everything within EXPR. What
makes you consider it again? Do you mean that if a revset has this form
`revset((.*))`; we just evaluate the contents inside parenthesis?
My proposal is 'revset(EXPR)', not 'revset("EXPR")'. It's an identity function
similar to present(), but also disables the legacy lookup. I don't wanna add
such magic, but 'revset(EXPR)' seems less bad than 'set:EXPR' or ' EXPR'.

Agreed that `set:foo` looks like a range, maybe we need to use a less
ambiguous operator?

`=foo+bar`
`#foo+bar`
`#revset# foo+bar`

(Nothing really stands out as pretty, but trying to extend our search
area here.)
Yeah, they look weird.

FWIW, I prefer not reviewing this sort of patches just before the freeze.
The patch itself is simple, but we have to carefully think about UX and
syntactic consistency.

We love the 'revset(EXPR)' idea and we have a v4 series that is ready to send.

It is okay to send it before the freeze? If not, would it be possible to take the first two changesets of the V3 series? They will likely bring some speed improvements for big repositories where name lookups are slow and for commands that don't need to load them.

By re-reading your comments on the V3 series, I realized that the second changeset is not necessary since repo is never passed for internal lookup

I will update the series and prepare a new V4.


_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel



_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

_______________________________________________
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Reply via email to