[PATCH V3] show: new extension for displaying various repository data

[Yuya Nishihara]

On Fri, 24 Mar 2017 19:31:20 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc at gmail.com>
> # Date 1490408340 25200
> #      Fri Mar 24 19:19:00 2017 -0700
> # Node ID 42b0afe0f6ebea285569e0061e1eab2fcffb686e
> # Parent  aea8ec3f7dd1967a05ecce8f779e16f7ad14fdee
> show: new extension for displaying various repository data

Looks good, and there seems no negative comment on this, so I'll queue it
tomorrow or next Monday.

A couple of comments in line.

> I chose to implement the "bookmarks" view first because it is simple
> and shouldn't invite too much bikeshedding that detracts from the
> evaluation of `hg show` itself. But there is an important point
> to consider: we now have 2 ways to show a list of bookmarks. I'm not
> a fan of introducing multiple ways to do very similar things. So it
> might be worth discussing how we wish to tackle this issue for
> bookmarks, tags, branches, MQ series, etc.

This is real concern, but can be settled later since "hg show" is still
experimental.

> + at command('show', commands.formatteropts, _('VIEW'))
> +def show(ui, repo, view=None, template=None):
> +    """show various repository information
> +
> +    A requested view of repository data is displayed.
> +
> +    If no view is requested, the list of available views is shown and the
> +    command aborts.
> +
> +    .. note::
> +
> +       There are no backwards compatibility guarantees for the output of this
> +       command. Output may change in any future Mercurial release.
> +
> +       Consumers wanting stable command output should specify a template via
> +       ``-T/--template``.
> +
> +    List of available views:
> +
> +    """

Is "hg show VIEW" supposed to not take any view-specific arguments?

> +    if ui.plain() and not template:
> +        raise error.Abort(_('"hg show" cannot be used in plain mode because '
> +                            'output is not stable'),
> +                          hint=_('unset HGPLAIN and invoke with -T/--template '
> +                                 'to control output'))
> +
> +    ui.pager('show')
> +    views = showview._table
> +
> +    if not view:
> +        # TODO consider using formatter here so available views can be
> +        # rendered to custom format.

I'll move ui.pager() here, ...

> +        ui.write(_('available views:\n'))
> +        ui.write('\n')
> +
> +        for name, func in sorted(views.items()):
> +            ui.write(('%s\n') % func.__doc__)
> +
> +        ui.write('\n')
> +        raise error.Abort(_('no view requested'),
> +                          hint=_('use "hg show VIEW" to choose a view'))
> +
> +    # TODO use same logic as dispatch to perform prefix matching.
> +    if view not in views:
> +        raise error.Abort(_('unknown view: %s') % view,
> +                          hint=_('run "hg show" to see available views'))
> +
> +    template = template or 'show'
> +    fmtopic = 'show%s' % views[view]._fmtopic

and ui.pager() here.

> +    with ui.formatter(fmtopic, {'template': template}) as fm:
> +        return views[view](ui, repo, fm)
> +
> + at showview('bookmarks', fmtopic='bookmarks')
> +def showbookmarks(ui, repo, fm):
> +    """bookmarks and their associated changeset"""
> +    marks = repo._bookmarks
> +    if not len(marks):

I would add "TODO json output is corrupted". Can be updated inflight.

> +        ui.write(_('(no bookmarks set)\n'))
> +        return
> +
> +    active = repo._activebookmark
> +    longestname = max(len(b) for b in marks)
> +    # TODO consider exposing longest shortest(node).
> +
> +    for bm, node in sorted(marks.items()):
> +        fm.startitem()
> +        fm.context(ctx=repo[node])
> +        fm.write('bookmark', '%s', bm)
> +        fm.write('node', fm.hexfunc(node), fm.hexfunc(node))
> +        fm.data(active=bm == active,
> +                longestbookmarklen=longestname)