This is an archive of the discontinued Mercurial Phabricator instance.

revisions: when using prefixhexnode, ensure we prefix "0"
ClosedPublic

Authored by spectral on Oct 16 2018, 10:21 AM.

Details

Summary

Previously, if using experimental.revisions.disambiguatewithin (and it didn't
include rev0), and '0' was the shortest identifier in that disambiguation set,
we printed it as the shortest *without* a prefix. This was because we had logic
to determine "if the prefix is a pure integer, but starts with 0, we don't need
to prefix with 'x': 01 is not a synonym for revision #1", but didn't handle the
case where prefix == 0 (which is a pure integer, and starts with 0... but it
*is* "rev0").

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

spectral created this revision.Oct 16 2018, 10:21 AM
martinvonz accepted this revision.Oct 17 2018, 7:43 PM
martinvonz added a subscriber: martinvonz.

I'll also note that this bug was there all along, but it got way more visible with with the disambiguatewithin stuff. It used to fail only when calculating the shortest prefix for nullid in a repo without any other revisions starting with hex digit 0. The following patch shows that (it failed back on 4.6 too). I'm folding it in in flight. I'm also fixing test-revisions.t (which already showed the bug, but I had not realized it was a bug :P).

diff --git a/tests/test-template-functions.t b/tests/test-template-functions.t

  • a/tests/test-template-functions.t

+++ b/tests/test-template-functions.t
@@ -804,6 +804,8 @@ Test shortest(node) function:

e777603221
bcc7ff960b
f7769ec2ab

+ $ hg log --template '{shortest(node, 1)}\n' -r null
+ 00

$ hg log --template '{node|shortest}\n' -l1
e777
 
This revision is now accepted and ready to land.Oct 17 2018, 7:43 PM
martinvonz added inline comments.Oct 17 2018, 7:49 PM
tests/test-template-functions.t
936–937

"We *do* identify the collision if..." is a little misleading because it's a different collision that's identified, so I've changed this to "We identify the collision with nullid if..." in flight.

This revision was automatically updated to reflect the committed changes.