[PATCH 2 of 2 V2] revset: skip old style lookup if external whitespace are detected

[Feld Boris]

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 at mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20180413/e6efc2f2/attachment.html>