[PATCH 4 of 8] revset: enforce "%d" to be interpreted as literal revision number (API)

[Boris FELD]

On 13/01/2019 10:12, Yuya Nishihara wrote:
> On Sun, 13 Jan 2019 08:37:47 +0100, Boris FELD wrote:
>> On 12/01/2019 05:04, Yuya Nishihara wrote:
>>> On Fri, 11 Jan 2019 12:29:06 +0100, Boris Feld wrote:
>>>> # HG changeset patch
>>>> # User Boris Feld <boris.feld at octobus.net>
>>>> # Date 1547130238 -3600
>>>> #      Thu Jan 10 15:23:58 2019 +0100
>>>> # Node ID 38733dd85595782676175141111a42f253efabb6
>>>> # Parent  427247e84e29c144321d21a825d371458b5d3e1a
>>>> # EXP-Topic revs-efficiency
>>>> # Available At https://bitbucket.org/octobus/mercurial-devel/
>>>> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 38733dd85595
>>>> revset: enforce "%d" to be interpreted as literal revision number (API)
>>> New behavior looks saner. Please also flag this as (BC). It's exposed as
>>> revset() template function.
>>>
>>>>      %r = revset expression, parenthesized
>>>> -    %d = int(arg), no quoting
>>>> +    %d = rev(int(arg)), no quoting
>>> 'rev(n)' returns an empty set if n is out of range, whereas 'n' aborts.
>>> Suppose it's pretty much a coding error to pass in an invalid revision to
>>> repo.revs(), we'll probably want aborts.
>>>
>>> Maybe we'll need an internal '_rev()' function?
>> %ld silently pass on these too, so I would rather have %ld and %d
>> consistent here (and align on the silence %ld behavior).
> Indeed. I never thought %ld would behave in such way. Consistent behavior
> should be better.

Through some buggy revset usage in evolve we discovered that the
optimization to passthrough input directly for %ld changed this
behavior. In these cases, the revisions are passed as is and
filtered/out-of-bound revision raise their usual error.

Reintroducing the silent filtering behavior for %ld seems possible (it
will have a performance impact, but still faster than the previous
serialization).

However, it is probably more efficient and saner to have these errors
raised. In this case, we should align all %ld case and %d behaviors on
the error raising version. What do you think?

I'll try to get a patch out tomorrow to align things in a direction or
the other.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel