Submitter | Jordi Gutiérrez Hermoso |
---|---|
Date | Feb. 26, 2015, 6:58 p.m. |
Message ID | <be1e1c998407c4b35c9d.1424977094@Iris> |
Download | mbox | patch |
Permalink | /patch/7840/ |
State | Accepted |
Headers | show |
Comments
On Thu, 26 Feb 2015 13:58:14 -0500, Jordi Gutiérrez Hermoso wrote: > # HG changeset patch > # User Jordi Gutiérrez Hermoso <jordigh@octave.org> > # Date 1424905921 18000 > # Wed Feb 25 18:12:01 2015 -0500 > # Node ID be1e1c998407c4b35c9df52be7ce8f4c87ef417c > # Parent f15891e7f658fbc0351589887760548cab2c41f8 > revrange: don't parse revset aliases as hash prefixes (issue4553) > > If a user has a revsetalias defined, it is their explicit wish for > this alias to be parsed as a revset and nothing else. Although the > case of the alias being short enough and only contain the letters a-f > is probably kind of rare, it may still happen. > > diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py > --- a/mercurial/scmutil.py > +++ b/mercurial/scmutil.py > @@ -628,12 +628,22 @@ def revrange(repo, revs): > return repo[val].rev() > > seen, l = set(), revset.baseset([]) > + > + revsetaliases = [alias for (alias, _) in > + repo.ui.configitems("revsetalias")] "_" shadows _() function and could cause problem in future. > for spec in revs: > if l and not seen: > seen = set(l) > # attempt to parse old-style ranges first to deal with > # things like old-tag which contain query metacharacters > try: > + # ... except for revset aliases without arguments. These > + # should be parsed as soon as possible, because they might > + # clash with a hash prefix. > + if spec in revsetaliases: > + raise error.RepoLookupError > + > if isinstance(spec, int): > seen.add(spec) > l = l + revset.baseset([spec]) > @@ -641,6 +651,9 @@ def revrange(repo, revs): > > if _revrangesep in spec: > start, end = spec.split(_revrangesep, 1) > + if start in revsetaliases or end in revsetaliases: > + raise error.RepoLookupError > + > start = revfix(repo, start, 0) > end = revfix(repo, end, len(repo) - 1) > if end == nullrev and start < 0: I don't know which is better, but both "x in revsetliases" checks can be done by revfix(). Regards,
On Sat, 28 Feb 2015 12:37:19 +0900, Yuya Nishihara wrote: > On Thu, 26 Feb 2015 13:58:14 -0500, Jordi Gutiérrez Hermoso wrote: > > for spec in revs: > > if l and not seen: > > seen = set(l) > > # attempt to parse old-style ranges first to deal with > > # things like old-tag which contain query metacharacters > > try: > > + # ... except for revset aliases without arguments. These > > + # should be parsed as soon as possible, because they might > > + # clash with a hash prefix. > > + if spec in revsetaliases: > > + raise error.RepoLookupError > > + > > if isinstance(spec, int): > > seen.add(spec) > > l = l + revset.baseset([spec]) > > @@ -641,6 +651,9 @@ def revrange(repo, revs): > > > > if _revrangesep in spec: > > start, end = spec.split(_revrangesep, 1) > > + if start in revsetaliases or end in revsetaliases: > > + raise error.RepoLookupError > > + > > start = revfix(repo, start, 0) > > end = revfix(repo, end, len(repo) - 1) > > if end == nullrev and start < 0: > > I don't know which is better, but both "x in revsetliases" checks can be done > by revfix(). Oops, my suggestion didn't work because "spec in repo" may raise LookupError. It seems "spec in repo" is unnecessary, though.
On Sat, 2015-02-28 at 18:04 +0900, Yuya Nishihara wrote: > On Sat, 28 Feb 2015 12:37:19 +0900, Yuya Nishihara wrote: > > I don't know which is better, but both "x in revsetliases" checks can be done > > by revfix(). > > Oops, my suggestion didn't work because "spec in repo" may raise LookupError. > It seems "spec in repo" is unnecessary, though. I don't understand what you're suggesting with revfix. Do you think I can ignore your suggestion? :-)
On 02/26/2015 06:58 PM, Jordi Gutiérrez Hermoso wrote: > # HG changeset patch > # User Jordi Gutiérrez Hermoso <jordigh@octave.org> > # Date 1424905921 18000 > # Wed Feb 25 18:12:01 2015 -0500 > # Node ID be1e1c998407c4b35c9df52be7ce8f4c87ef417c > # Parent f15891e7f658fbc0351589887760548cab2c41f8 > revrange: don't parse revset aliases as hash prefixes (issue4553) Looks like an interesting things to have. Had to look the code a bit to convince me that the LookupError raising was not an abomination. Pushed to the clowncopter.
On Mon, 02 Mar 2015 13:28:32 -0500, Jordi Gutiérrez Hermoso wrote: > On Sat, 2015-02-28 at 18:04 +0900, Yuya Nishihara wrote: > > On Sat, 28 Feb 2015 12:37:19 +0900, Yuya Nishihara wrote: > > > I don't know which is better, but both "x in revsetliases" checks can be done > > > by revfix(). > > > > Oops, my suggestion didn't work because "spec in repo" may raise LookupError. > > It seems "spec in repo" is unnecessary, though. > > I don't understand what you're suggesting with revfix. Do you think I > can ignore your suggestion? :-) Yes, it's not important. Just it could be written as follows. + revsetaliases = [alias for (alias, _definition) in + repo.ui.configitems("revsetalias")] + def revfix(repo, val, defval): if not val and val != 0 and defval is not None: return defval + if val in revsetaliases: + raise error.RepoLookupError return repo[val].rev() @@ -659,7 +665,7 @@ def revrange(repo, revs): seen = newrevs l = l + revset.baseset(sorted(newrevs, reverse=start > end)) continue - elif spec and spec in repo: # single unquoted rev + elif spec: # single unquoted rev rev = revfix(repo, spec, None) if rev in seen: continue
Patch
diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py --- a/mercurial/scmutil.py +++ b/mercurial/scmutil.py @@ -628,12 +628,22 @@ def revrange(repo, revs): return repo[val].rev() seen, l = set(), revset.baseset([]) + + revsetaliases = [alias for (alias, _) in + repo.ui.configitems("revsetalias")] + for spec in revs: if l and not seen: seen = set(l) # attempt to parse old-style ranges first to deal with # things like old-tag which contain query metacharacters try: + # ... except for revset aliases without arguments. These + # should be parsed as soon as possible, because they might + # clash with a hash prefix. + if spec in revsetaliases: + raise error.RepoLookupError + if isinstance(spec, int): seen.add(spec) l = l + revset.baseset([spec]) @@ -641,6 +651,9 @@ def revrange(repo, revs): if _revrangesep in spec: start, end = spec.split(_revrangesep, 1) + if start in revsetaliases or end in revsetaliases: + raise error.RepoLookupError + start = revfix(repo, start, 0) end = revfix(repo, end, len(repo) - 1) if end == nullrev and start < 0: diff --git a/tests/test-revset.t b/tests/test-revset.t --- a/tests/test-revset.t +++ b/tests/test-revset.t @@ -1128,6 +1128,62 @@ far away. 3 2 +issue4553: check that revset aliases override existing hash prefix + + $ hg log -qr e + 6:e0cc66ef77e8 + + $ hg log -qr e --config revsetalias.e="all()" + 0:2785f51eece5 + 1:d75937da8da0 + 2:5ed5505e9f1c + 3:8528aa5637f2 + 4:2326846efdab + 5:904fa392b941 + 6:e0cc66ef77e8 + 7:013af1973af4 + 8:d5d0dcbdc4d9 + 9:24286f4ae135 + + $ hg log -qr e: --config revsetalias.e="0" + 0:2785f51eece5 + 1:d75937da8da0 + 2:5ed5505e9f1c + 3:8528aa5637f2 + 4:2326846efdab + 5:904fa392b941 + 6:e0cc66ef77e8 + 7:013af1973af4 + 8:d5d0dcbdc4d9 + 9:24286f4ae135 + + $ hg log -qr :e --config revsetalias.e="9" + 0:2785f51eece5 + 1:d75937da8da0 + 2:5ed5505e9f1c + 3:8528aa5637f2 + 4:2326846efdab + 5:904fa392b941 + 6:e0cc66ef77e8 + 7:013af1973af4 + 8:d5d0dcbdc4d9 + 9:24286f4ae135 + + $ hg log -qr e: + 6:e0cc66ef77e8 + 7:013af1973af4 + 8:d5d0dcbdc4d9 + 9:24286f4ae135 + + $ hg log -qr :e + 0:2785f51eece5 + 1:d75937da8da0 + 2:5ed5505e9f1c + 3:8528aa5637f2 + 4:2326846efdab + 5:904fa392b941 + 6:e0cc66ef77e8 + issue2549 - correct optimizations $ log 'limit(1 or 2 or 3, 2) and not 2'