On 02/03/2018 05:58, Yuya Nishihara wrote:
On Thu, 1 Mar 2018 15:48:47 -0500, Feld Boris wrote:
On 26/02/2018 08:11, Yuya Nishihara wrote:
On Mon, 26 Feb 2018 11:45:03 +0100, Feld Boris wrote:
On 13/02/2018 12:47, Yuya Nishihara wrote:
On Mon, 12 Feb 2018 18:00:52 +0100, Boris Feld wrote:
# HG changeset patch
# User Boris Feld <boris.f...@octobus.net>
# Date 1518448909 -3600
#      Mon Feb 12 16:21:49 2018 +0100
# Node ID b0f45e1376e2d0f32023e197c51802bc21c60490
# Parent  f02fd7ca256d044c4a51c3f3fc0ecaf95d23e03d
# EXP-Topic noname
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 
revset: skip old style lookup if external whitespace are detected

Since label cannot contains leading or trailing whitespace we can skip looking
for them. This is useful in repository with slow labels (eg: special type of
tags). Short command running on a specific revision can benefit from such

eg on a repository where loading tags take 0.4s:

1: hg log --template '{node}\n' --rev 'rev(0)'
      0.560 seconds

2: hg log --template '{node}\n' --rev ' rev(0)'
      0.109 seconds
Seems okay, but isn't it too obscure that prefixing with ' ' is the fast
way of querying?
Yes, it is a bit obscure but it's the best solution we came up with
existing code. I sent a new patch that implements the fast path at the
individual name-space level in a way that seems cleaner and more useful.

Another solution is we could force revset evaluation using a `set:` (or
`revset:`) prefix. This way we could have both a clean and explicit way
of implementing the fast path.
That isn't possible because "set:whatever" can be a range between "set"
and whatever. ;)
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:


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.

Alternatively, we could add
a config knob to switch off the old-style range support.
Having a config knob for this seems weird. We don't expect users to find
it and really understand what the config is about. It would be useful
for large corporate users with centralized config, but they would need
to set the flag on every one of their scripts/servers involving Mercurial.
IMHO, config knob is easier to learn than using the ' ' prefix. I would say
WTF if I taught to use the ' ' to make hg fast. And I think this config can
be switched on by tweakdefaults because lookup() exists only for backward
compatibility. (We might still want to allow dashes in symbols, though.)
Dropping the older lookup methods seems impractical. Right now, `rev(0)`
is a valid tag. So dropping legacy lookup means all these things will
have to be heavily quoted: (eg: hg log -r '"version-4(candidate2)"').
Yes. I assume that "foo(bar)" tags or branches wouldn't be used widely.

Since "foo(bar)" needs quotes in revset query (except for x and x:y), it would
makes some sense to add an option to disable the compatibility hack at all.

We cannot see a way to make the config option both easily discoverable and constrained. There is lot of labels that includes `-`, `+` and other symbols that will be impacted.

Having a revset prefix would allow some revsets to be parsed with the legacy lookup (likely the user inputs) while other could be parsed explicitly as revsets (for exampleconfig options like pushrev) to avoid any collision with potential labels.

A longer terms solution would be to support configuring constraints on
labels. If tags can be configured to only match a specific pattern we
could skip lookup for data that does not match it very quickly. To be
fully enforced it probably need to a part of the .hg/requires so it is a
heavier change. It also needs to be more than just a hook so that the
lookup logic is aware of it and can fast path it. Such approach work in
our use-case but is also more specific and requires more
We could use the same approach as the V2 series proposed here:
Just curious. Can't we make the name lookup instant?

Sadly no.

However, we think it still makes sense to have an easy and standard way
to force something to be evaluated as a revset as it's not possible as
of today.

Here is a summary of the idea we've discussed so far:

1) Using some kind of prefix, breaking BC for people using the prefix as
a label:

    `revs:rev(42)` → directly evaluated as `rev(42)`
    `"revs":rev(42)` → force "revs" as a label

2) Adding a special revset for revset evaluation.

    `revset("rev(42)")` → directly evaluated `rev(42)`

3) adding strange special case: (leading space, parents).

    ` rev(42)` → directly evaluated as `rev(42)`
    `(rev(42))` → directly evaluated as `rev(42)`

4) Config flag to disable legacy parsing (force quote for strange labels).

5) Full-featured label patterns enforcement as described earlier in this

We agree fast-pathing on (3) is a bit too obscure.
We find (4) to be impractical (and likely to break unexpected stuff).

(5) is a good solution but significantly heavier to write and much more

So we would be happy if we could agree on a way to go for (1) or maybe
(2). We prefer (1) because it is closer than what we do for fileset. The
one time BC should be specific enough to not be a real issue. (`revs` is
the best prefix we can think of).
As I said earlier in this reply, I think (1) should be avoided. (2) seems less
bad, but we would need to escape inner quotes (e.g. 'revset("\"foo bar\"")'),
which is horrible. (3) is odd. (4) seems okay for me. (5) might be okay, but
the proposed syntax ' rev(42)' is weird, and enforcing naming style by a VCS
tool might be bad for UX.

Mercurial-devel mailing list

Reply via email to