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

2017-03-24 Thread Jun Wu
Excerpts from Denis Laxalde's message of 2017-03-23 10:14:14 +0100:
> Ryan McElroy a écrit :
> > On 3/22/17 7:29 AM, Sean Farley wrote:
> >> Yuya Nishihara  writes:
> >>
> >>> On Sun, 12 Mar 2017 21:38:00 -0700, Gregory Szorc wrote:
>  # HG changeset patch
>  # User Gregory Szorc 
>  # Date 1489378362 25200
>  #  Sun Mar 12 21:12:42 2017 -0700
>  # Node ID d30057d358076cbe7d632cd573095af97543f932
>  # Parent  1c3352d7eaf24533ad52d4b8a024211e9189fb0b
>  show: new extension for displaying various repository data
> >>> The idea sounds nice to me. I just checked minor implementation details
> >>> about formatter.
> >> Just a quick reply (as I whittle down my backlog), but a lot of people
> >> (including myself) have a 'show' alias (inspired by 'git show').
> >>
> >> That may or may not be a factor in this.
> >
> > Greg called this out specifically in his excellent summary.
> >
> > FB also has a "show" extension that replaced our "show" alias:
> > https://bitbucket.org/facebook/hg-experimental/src/default/hgext3rd/show.py 
> 
> It seems to me that all "show" extensions/aliases can be replaced by the
> proposed extension. Also if we look to the future, it seems to me that

The problem is top-level command names are rare resources. It's really hard
to name things. I think subcommands is a great choice.

> we'd have hard time to explain that this command is not called "show"
> because there used to be extensions/aliases with this name and that got
> replaced by said command.

Another benefit of the "show" approach is it's supposed to be read-only.
It feels cleaner than for example "hg bookmark" which could do either read
or write.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


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

2017-03-22 Thread Ryan McElroy



On 3/22/17 7:29 AM, Sean Farley wrote:

Yuya Nishihara  writes:


On Sun, 12 Mar 2017 21:38:00 -0700, Gregory Szorc wrote:

# HG changeset patch
# User Gregory Szorc 
# Date 1489378362 25200
#  Sun Mar 12 21:12:42 2017 -0700
# Node ID d30057d358076cbe7d632cd573095af97543f932
# Parent  1c3352d7eaf24533ad52d4b8a024211e9189fb0b
show: new extension for displaying various repository data

The idea sounds nice to me. I just checked minor implementation details
about formatter.

Just a quick reply (as I whittle down my backlog), but a lot of people
(including myself) have a 'show' alias (inspired by 'git show').

That may or may not be a factor in this.


Greg called this out specifically in his excellent summary.

FB also has a "show" extension that replaced our "show" alias: 
https://bitbucket.org/facebook/hg-experimental/src/default/hgext3rd/show.py


Therefore, I would also slightly prefer "view", but I admit I don't care 
about hgk (even though we have it on at FB and it doesn't seem to work 
at all...)


I'll respond to the original as well so I can respond inline to the code 
and summary.

___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


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

2017-03-22 Thread Gregory Szorc
On Mon, Mar 13, 2017 at 7:21 PM, Yuya Nishihara  wrote:

> On Sun, 12 Mar 2017 21:38:00 -0700, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc 
> > # Date 1489378362 25200
> > #  Sun Mar 12 21:12:42 2017 -0700
> > # Node ID d30057d358076cbe7d632cd573095af97543f932
> > # Parent  1c3352d7eaf24533ad52d4b8a024211e9189fb0b
> > show: new extension for displaying various repository data
>
> The idea sounds nice to me. I just checked minor implementation details
> about formatter.
>
> > +@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.
> > +
> > +.. note::
> > +
> > +   The default output from this command is not covered under
> Mercurial's
> > +   default backwards-compatible mechanism (which puts an emphasis on
> > +   not changing behavior). This means output from this command may
> change
> > +   in any future release. However, the values fed to the formatter
> are
> > +   covered under the default backwards-compatible mechanism.
> > +
> > +   Automated consumers of this command should specify an explicit
> template
> > +   via ``-T/--template`` to guarantee output is stable.
> > +
> > +List of available views:
> > +
> > +"""
> > +views = showview._table
> > +
> > +if not view:
> > +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 ` to choose a
> view'))
> > +
> > +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 = views[view]._formatter
> > +formatter = ui.formatter(fmtopic, {'template': template})
> > +
> > +return views[view](ui, repo, formatter)
>
> s/formatter/fm/ seems better for consistency. Also, I prefer calling
> fm.end()
> here as it should be paired with the construction.
>
>   with ui.formatter(...) as fm:
>   return views[view](...)
>
> > +@showview('bookmarks', formatter='bookmarks')
>
> s/formatter=/topic=/ or /fmtopic=/ ?
>
> If this 'bookmarks' template can't be compatible with the default template
> used by "hg bookmarks" command, we'll need another topic name.
>
> > +def showbookmarks(ui, repo, fm):
> > +"""bookmarks and their associated changeset"""
> > +marks = repo._bookmarks
> > +if not len(marks):
> > +ui.write_err('(no bookmarks set)\n')
> > +return 0
>
> If this is an error, it should error out. Otherwise, use fm.plain().
>

fm.plain() no-ops when using templateformatter though.

I'm not sure what to do here. We probably want a message printed to the
user without an error exit code. So I'm going to keep this as ui.write() in
V2 and you can suggest a workaround.


>
> > +active = repo._activebookmark
> > +longest = max(len(b) for b in marks)
> > +
> > +for bm, node in sorted(marks.items()):
> > +fm.startitem()
> > +fm.write('bookmark', '%s', bm)
> > +fm.write('node', fm.hexfunc(node), fm.hexfunc(node))
> > +fm.data(ctx=repo[node],
> > +active=bm == active,
> > +longestlen=longest)
>
> As changectx can't be serialized, it shouldn't be passed to e.g.
> jsonformatter.
> fm.context(ctx=repo[node]) can be used. It's unfortunate that 'longestlen'
> appears in JSON output, but I have no better idea other than abusing
> fm.context().
>

I have an idea. I may test the waters in V2.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel


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

2017-03-14 Thread Yuya Nishihara
On Sun, 12 Mar 2017 21:38:00 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc 
> # Date 1489378362 25200
> #  Sun Mar 12 21:12:42 2017 -0700
> # Node ID d30057d358076cbe7d632cd573095af97543f932
> # Parent  1c3352d7eaf24533ad52d4b8a024211e9189fb0b
> show: new extension for displaying various repository data

The idea sounds nice to me. I just checked minor implementation details
about formatter.

> +@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.
> +
> +.. note::
> +
> +   The default output from this command is not covered under Mercurial's
> +   default backwards-compatible mechanism (which puts an emphasis on
> +   not changing behavior). This means output from this command may change
> +   in any future release. However, the values fed to the formatter are
> +   covered under the default backwards-compatible mechanism.
> +
> +   Automated consumers of this command should specify an explicit 
> template
> +   via ``-T/--template`` to guarantee output is stable.
> +
> +List of available views:
> +
> +"""
> +views = showview._table
> +
> +if not view:
> +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 ` to choose a view'))
> +
> +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 = views[view]._formatter
> +formatter = ui.formatter(fmtopic, {'template': template})
> +
> +return views[view](ui, repo, formatter)

s/formatter/fm/ seems better for consistency. Also, I prefer calling fm.end()
here as it should be paired with the construction.

  with ui.formatter(...) as fm:
  return views[view](...)

> +@showview('bookmarks', formatter='bookmarks')

s/formatter=/topic=/ or /fmtopic=/ ?

If this 'bookmarks' template can't be compatible with the default template
used by "hg bookmarks" command, we'll need another topic name.

> +def showbookmarks(ui, repo, fm):
> +"""bookmarks and their associated changeset"""
> +marks = repo._bookmarks
> +if not len(marks):
> +ui.write_err('(no bookmarks set)\n')
> +return 0

If this is an error, it should error out. Otherwise, use fm.plain().

> +active = repo._activebookmark
> +longest = max(len(b) for b in marks)
> +
> +for bm, node in sorted(marks.items()):
> +fm.startitem()
> +fm.write('bookmark', '%s', bm)
> +fm.write('node', fm.hexfunc(node), fm.hexfunc(node))
> +fm.data(ctx=repo[node],
> +active=bm == active,
> +longestlen=longest)

As changectx can't be serialized, it shouldn't be passed to e.g. jsonformatter.
fm.context(ctx=repo[node]) can be used. It's unfortunate that 'longestlen'
appears in JSON output, but I have no better idea other than abusing
fm.context().
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel