This is an archive of the discontinued Mercurial Phabricator instance.

Differential D557

show: use consistent node lengths
AbandonedPublic

Authored by indygreg on Aug 28 2017, 11:55 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Summary

hg show makes heavy use of shortest() to limit the length of the node
hash.

For the "stack" and "work" views, you are often looking at multiple
lines of similar output for "lines" of work. It is visually appeasing
for things to vertically align. A naive use of {shortest(node, N)}
could result in variable length nodes and for the first character of
the description to vary by a column or two.

We implement a function to determine the longest shortest prefix for
a set of revisions. The new function is used to determine the printed
node length for all hg show views.

Testing this will be deferred to a subsequent commit.

.. feature::

show: use consistent node length in views

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

indygreg created this revision.Aug 28 2017, 11:55 PM
quark added a subscriber: quark.Aug 29 2017, 12:06 AM
quark added inline comments.
hgext/show.py
428

How about minlen=4? It could be useful in small repos.

440

I'd call low level function without template and maintain a current minlen to reduce calculation.

yuja added a subscriber: yuja.Aug 29 2017, 10:15 AM
yuja added inline comments.
hgext/show.py
428

minlen=4 seems better than 3. _partialmatch() falls through to
slow path if nodelen < 4.

indygreg added inline comments.Sep 14 2017, 12:14 AM
hgext/show.py
440

The current minlen code is semi-tightly integrated with the templating layer.

I agree with you that a generic function should be established. But I considered that scope bloat and figured it was territory for a follow-up.

indygreg abandoned this revision.Sep 14 2017, 12:20 AM

This got rolled into D558. hg phabsend didn't appear smart enough to discard it from Phabricator when I pushed the new stack.

Revision Contents
Changeset List

Diff 1384

hgext/show.py

if not len(marks): if not len(marks):
# This is a bit hacky. Ideally, templates would have a way to # This is a bit hacky. Ideally, templates would have a way to
# specify an empty output, but we shouldn't corrupt JSON while # specify an empty output, but we shouldn't corrupt JSON while
# waiting for this functionality. # waiting for this functionality.
if not isinstance(fm, formatter.jsonformatter): if not isinstance(fm, formatter.jsonformatter):
ui.write(_('(no bookmarks set)\n')) ui.write(_('(no bookmarks set)\n'))
return return
revs = [repo[node].rev() for node in marks.values()]
active = repo._activebookmark active = repo._activebookmark
longestname = max(len(b) for b in marks) longestname = max(len(b) for b in marks)
# TODO consider exposing longest shortest(node). nodelen = longestshortest(repo, revs)
for bm, node in sorted(marks.items()): for bm, node in sorted(marks.items()):
fm.startitem() fm.startitem()
fm.context(ctx=repo[node]) fm.context(ctx=repo[node])
fm.write('bookmark', '%s', bm) fm.write('bookmark', '%s', bm)
fm.write('node', fm.hexfunc(node), fm.hexfunc(node)) fm.write('node', fm.hexfunc(node), fm.hexfunc(node))
fm.data(active=bm == active, fm.data(active=bm == active,
longestbookmarklen=longestname, longestbookmarklen=longestname,
nodelen=5) nodelen=nodelen)
@showview('stack', csettopic='stack') @showview('stack', csettopic='stack')
def showstack(ui, repo, displayer): def showstack(ui, repo, displayer):
"""current line of work""" """current line of work"""
wdirctx = repo['.'] wdirctx = repo['.']
if wdirctx.rev() == nullrev: if wdirctx.rev() == nullrev:
raise error.Abort(_('stack view only available when there is a ' raise error.Abort(_('stack view only available when there is a '
'working directory')) 'working directory'))
# merge or rebase targets. # merge or rebase targets.
if basectx: if basectx:
# TODO make this customizable? # TODO make this customizable?
newheads = set(repo.revs('heads(%d::) - %ld - not public()', newheads = set(repo.revs('heads(%d::) - %ld - not public()',
basectx.rev(), stackrevs)) basectx.rev(), stackrevs))
else: else:
newheads = set() newheads = set()
allrevs = set(stackrevs) | newheads | set([baserev])
nodelen = longestshortest(repo, allrevs)
try: try:
cmdutil.findcmd('rebase', commands.table) cmdutil.findcmd('rebase', commands.table)
haverebase = True haverebase = True
except (error.AmbiguousCommand, error.UnknownCommand): except (error.AmbiguousCommand, error.UnknownCommand):
haverebase = False haverebase = False
# TODO use templating. # TODO use templating.
# TODO consider using graphmod. But it may not be necessary given # TODO consider using graphmod. But it may not be necessary given
# our simplicity and the customizations required. # our simplicity and the customizations required.
# TODO use proper graph symbols from graphmod # TODO use proper graph symbols from graphmod
shortesttmpl = formatter.maketemplater(ui, '{shortest(node, 5)}') shortesttmpl = formatter.maketemplater(ui, '{shortest(node, %d)}' % nodelen)
def shortest(ctx): def shortest(ctx):
return shortesttmpl.render({'ctx': ctx, 'node': ctx.hex()}) return shortesttmpl.render({'ctx': ctx, 'node': ctx.hex()})
# We write out new heads to aid in DAG awareness and to help with decision # We write out new heads to aid in DAG awareness and to help with decision
# making on how the stack should be reconciled with commits made since the # making on how the stack should be reconciled with commits made since the
# branch point. # branch point.
if newheads: if newheads:
# Calculate distance from base so we can render the count and so we can # Calculate distance from base so we can render the count and so we can
ctx = repo[rev] ctx = repo[rev]
if i: if i:
ui.write(': ') ui.write(': ')
else: else:
ui.write(' ') ui.write(' ')
ui.write(('o ')) ui.write(('o '))
displayer.show(ctx, nodelen=5) displayer.show(ctx, nodelen=nodelen)
displayer.flush(ctx) displayer.flush(ctx)
ui.write('\n') ui.write('\n')
if i: if i:
ui.write(':/') ui.write(':/')
else: else:
ui.write(' /') ui.write(' /')
symbol = '@' if rev == wdirctx.rev() else 'o' symbol = '@' if rev == wdirctx.rev() else 'o'
if newheads: if newheads:
ui.write(': ') ui.write(': ')
else: else:
ui.write(' ') ui.write(' ')
ui.write(symbol, ' ') ui.write(symbol, ' ')
displayer.show(ctx, nodelen=5) displayer.show(ctx, nodelen=nodelen)
displayer.flush(ctx) displayer.flush(ctx)
ui.write('\n') ui.write('\n')
# TODO display histedit hint? # TODO display histedit hint?
if basectx: if basectx:
# Vertically and horizontally separate stack base from parent # Vertically and horizontally separate stack base from parent
# to reinforce stack boundary. # to reinforce stack boundary.
if newheads: if newheads:
ui.write(':/ ') ui.write(':/ ')
else: else:
ui.write(' / ') ui.write(' / ')
ui.write(_('(stack base)'), '\n', label='stack.label') ui.write(_('(stack base)'), '\n', label='stack.label')
ui.write(('o ')) ui.write(('o '))
displayer.show(basectx, nodelen=5) displayer.show(basectx, nodelen=nodelen)
displayer.flush(basectx) displayer.flush(basectx)
ui.write('\n') ui.write('\n')
@revsetpredicate('_underway([commitage[, headage]])') @revsetpredicate('_underway([commitage[, headage]])')
def underwayrevset(repo, subset, x): def underwayrevset(repo, subset, x):
args = revset.getargsdict(x, 'underway', 'commitage headage') args = revset.getargsdict(x, 'underway', 'commitage headage')
if 'commitage' not in args: if 'commitage' not in args:
args['commitage'] = None args['commitage'] = None
return subset & relevant return subset & relevant
@showview('work', csettopic='work') @showview('work', csettopic='work')
def showwork(ui, repo, displayer): def showwork(ui, repo, displayer):
"""changesets that aren't finished""" """changesets that aren't finished"""
# TODO support date-based limiting when calling revset. # TODO support date-based limiting when calling revset.
revs = repo.revs('sort(_underway(), topo)') revs = repo.revs('sort(_underway(), topo)')
nodelen = longestshortest(repo, revs)
revdag = graphmod.dagwalker(repo, revs) revdag = graphmod.dagwalker(repo, revs)
ui.setconfig('experimental', 'graphshorten', True) ui.setconfig('experimental', 'graphshorten', True)
cmdutil.displaygraph(ui, repo, revdag, displayer, graphmod.asciiedges, cmdutil.displaygraph(ui, repo, revdag, displayer, graphmod.asciiedges,
props={'nodelen': 5}) props={'nodelen': nodelen})
def extsetup(ui): def extsetup(ui):
# Alias `hg <prefix><view>` to `hg show <view>`. # Alias `hg <prefix><view>` to `hg show <view>`.
for prefix in ui.configlist('commands', 'show.aliasprefix'): for prefix in ui.configlist('commands', 'show.aliasprefix'):
for view in showview._table: for view in showview._table:
name = '%s%s' % (prefix, view) name = '%s%s' % (prefix, view)
choice, allcommands = cmdutil.findpossible(name, commands.table, choice, allcommands = cmdutil.findpossible(name, commands.table,
strict=True) strict=True)
# This alias is already a command name. Don't set it. # This alias is already a command name. Don't set it.
if name in choice: if name in choice:
continue continue
# Same for aliases. # Same for aliases.
if ui.config('alias', name): if ui.config('alias', name):
continue continue
ui.setconfig('alias', name, 'show %s' % view, source='show') ui.setconfig('alias', name, 'show %s' % view, source='show')
def longestshortest(repo, revs, minlen=5):
quarkUnsubmitted
Not Done

How about minlen=4? It could be useful in small repos.

quark: How about minlen=4? It could be useful in small repos.
yujaUnsubmitted
Not Done

minlen=4 seems better than 3. _partialmatch() falls through to
slow path if nodelen < 4.

yuja: minlen=4 seems better than 3. `_partialmatch()` falls through to slow path if `nodelen < 4`.
"""Return the length of the longest shortest node to identify revisions.
The result of this function can be used with the ``shortest()`` template
function to ensure that a value is unique and unambiguous for a given
set of nodes.
The number of revisions in the repo is taken into account to prevent
a numeric node prefix from conflicting with an integer revision number.
If we fail to do this, a value of e.g. ``10023`` could mean either
revision 10023 or node ``10023abc...``.
"""
tmpl = formatter.maketemplater(repo.ui, '{shortest(node, %d)}' % minlen)
quarkUnsubmitted
Not Done

I'd call low level function without template and maintain a current minlen to reduce calculation.

quark: I'd call low level function without template and maintain a current minlen to reduce…
indygregAuthorUnsubmitted
Not Done

The current minlen code is semi-tightly integrated with the templating layer.

I agree with you that a generic function should be established. But I considered that scope bloat and figured it was territory for a follow-up.

indygreg: The current minlen code is semi-tightly integrated with the templating layer. I agree with you…
lens = [minlen]
for rev in revs:
ctx = repo[rev]
shortest = tmpl.render({'ctx': ctx, 'node': ctx.hex()})
lens.append(len(shortest))
return max(lens)
# Adjust the docstring of the show command so it shows all registered views. # Adjust the docstring of the show command so it shows all registered views.
# This is a bit hacky because it runs at the end of module load. When moved # This is a bit hacky because it runs at the end of module load. When moved
# into core or when another extension wants to provide a view, we'll need # into core or when another extension wants to provide a view, we'll need
# to do this more robustly. # to do this more robustly.
# TODO make this more robust. # TODO make this more robust.
def _updatedocstring(): def _updatedocstring():
longest = max(map(len, showview._table.keys())) longest = max(map(len, showview._table.keys()))
entries = [] entries = []