Patchwork [RFC] debugobsolete: add formatter support (issue5134)

# HG changeset patch
# User liscju <piotr.listkiewicz@gmail.com>
# Date 1467710115 -7200
#      Tue Jul 05 11:15:15 2016 +0200
# Node ID 75dcd3200e5eb25cea41e39a26bcde12f50de376
# Parent  6a98f9408a504be455d4382801610daceac429e6
debugobsolete: add formatter support (issue5134)

This solution keeps the same output debugobsolete had
before formatter support, but there are few issues
I didnt manage to resolve:

1) succnodes, parents were space separated strings, in
formatter they should be passed as list to be printed
by jsonformatter output as for example:
succnodes: [succnode1, succnode2, etc]

Problem is how to pass it as list, but at the same time
keep the same output it had before.

2) metadata should be printed as dictionary, but when
it is passed as one it changes plain old formatting.

3) flags formatting, fm.write('flags', '%X', marker.flags())
behaves differently for plain formatting(it prints some flags
as C) but for json formatting it always prints number.

On Tue, 05 Jul 2016 11:43:00 +0200, liscju wrote:
> # HG changeset patch
> # User liscju <piotr.listkiewicz@gmail.com>
> # Date 1467710115 -7200
> #      Tue Jul 05 11:15:15 2016 +0200
> # Node ID 75dcd3200e5eb25cea41e39a26bcde12f50de376
> # Parent  6a98f9408a504be455d4382801610daceac429e6
> debugobsolete: add formatter support (issue5134)
> 
> This solution keeps the same output debugobsolete had
> before formatter support, but there are few issues
> I didnt manage to resolve:
> 
> 1) succnodes, parents were space separated strings, in
> formatter they should be passed as list to be printed
> by jsonformatter output as for example:
> succnodes: [succnode1, succnode2, etc]
> 
> Problem is how to pass it as list, but at the same time
> keep the same output it had before.

Yeah, it's known issue.

https://bz.mercurial-scm.org/show_bug.cgi?id=5217#c2

The idea I have in mind is to add new API for lists.

  # writelist(field: str, elemname: str, elemfmt: str, data: iterable,
  #           sep='': str)
  fm.writelist('succnodes', 'node', '%s ',
               (hex(repl) for repl in marker.succnodes()))

The point of the formatter API is to avoid divergence between plain and
template/machine outputs.  I think writelist() can handle common cases well.

> 2) metadata should be printed as dictionary, but when
> it is passed as one it changes plain old formatting.

For this particular case, you can use fm.data() and fix _jsonifyobj() to
support dicts. There are more complicated examples in evolve.py and journal.py,
but "{metadata}" can be a plain dict since it isn't a nested template structure.

> 3) flags formatting, fm.write('flags', '%X', marker.flags())
> behaves differently for plain formatting(it prints some flags
> as C) but for json formatting it always prints number.

"C" is hex value. It should be decimal integer in JSON.

> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -1611,25 +1611,42 @@ def show_changeset(ui, repo, opts, buffe
>  
>      return changeset_templater(ui, repo, matchfn, opts, tmpl, mapfile, buffered)
>  
> -def showmarker(ui, marker, index=None):
> +def showmarker(fm, marker, index=None):
>      """utility function to display obsolescence marker in a readable way
>  
>      To be used by debug function."""
> +    fm.startitem()

Better to move fm.startitem() to the caller because this function doesn't
control the loop.

> +    fm.condwrite(index is not None, 'index', '%i', index)
>      if index is not None:
> -        ui.write("%i " % index)
> +        fm.plain(' ')

JSON and template outputs should always have index, i.e.

  fm.condwrite(showindex, 'index', '%i ', index)

> +    fm.write('date', '%s', util.datestr(marker.date()))

It should be a date tuple in template/machine format.

On Sat, 9 Jul 2016 20:12:38 +0900, Yuya Nishihara wrote:
> On Tue, 05 Jul 2016 11:43:00 +0200, liscju wrote:
> > Problem is how to pass it as list, but at the same time
> > keep the same output it had before.  
> 
> Yeah, it's known issue.
> 
> https://bz.mercurial-scm.org/show_bug.cgi?id=5217#c2
> 
> The idea I have in mind is to add new API for lists.
> 
>   # writelist(field: str, elemname: str, elemfmt: str, data: iterable,
>   #           sep='': str)
>   fm.writelist('succnodes', 'node', '%s ',
>                (hex(repl) for repl in marker.succnodes()))
> 
> The point of the formatter API is to avoid divergence between plain and
> template/machine outputs.  I think writelist() can handle common cases well.

I came up with a better idea.

  # formatlist(name: str, data: iterable, fmt='%s': str, sep='': str)
  #            -> str|list|templatekw._hybrid
  hexnodes = [hex(repl) for repl in marker.succnodes()]
  fm.write('succnodes', '%s', fm.formatlist('node', hexnodes, fmt='%s '))

It can avoid copying the functionality of (cond)?write(), and has fewer
arguments.

> > +    fm.write('date', '%s', util.datestr(marker.date()))  
> 
> It should be a date tuple in template/machine format.

Maybe we could handle datestr() in the same way?

  fm.formatdate(date: (int, int), format=...) -> str|(int, int)

>
> For this particular case, you can use fm.data() and fix _jsonifyobj() to
> support dicts. There are more complicated examples in evolve.py and
> journal.py,
> but "{metadata}" can be a plain dict since it isn't a nested template
> structure.


It wouldn't be a problem that fm.data(..) for plainformatter prints
nothing? It means we would need two invocation of writing metadata:

fm.plain( expression_to_stringify_metadata) # only for plain
fm.data(metadata) # only for templates != plain


> JSON and template outputs should always have index, i.e.
>   fm.condwrite(showindex, 'index', '%i ', index)


As far as i checked only a plainformatter care about the condition in the
condwrite, other templates prints value no matter what condition is. Or i
just don't get the point.

I came up with a better idea.
>   # formatlist(name: str, data: iterable, fmt='%s': str, sep='': str)
>   #            -> str|list|templatekw._hybrid
>   hexnodes = [hex(repl) for repl in marker.succnodes()]
>   fm.write('succnodes', '%s', fm.formatlist('node', hexnodes, fmt='%s '))
> It can avoid copying the functionality of (cond)?write(), and has fewer
> arguments.


I have a problem what formatlist should return for jsonformatter:
1) list - it crash because json doesnt support jsonifying list
2) string - it won't work because json fm.write escapes characters like '['
3) templatekw._hybrid - i have no idea how to do this, i see this returns
generator, i have no idea how to use it

Maybe we could handle datestr() in the same way?
>   fm.formatdate(date: (int, int), format=...) -> str|(int, int)


Plainformatter in this case should probably  return util.datestr(date) but
what about jsonformatter?. JSON spec does not specify how date should look
like, should it return tuple of int and as a result print date as tuple?

2016-07-10 14:31 GMT+02:00 Yuya Nishihara <yuya@tcha.org>:

> On Sat, 9 Jul 2016 20:12:38 +0900, Yuya Nishihara wrote:
> > On Tue, 05 Jul 2016 11:43:00 +0200, liscju wrote:
> > > Problem is how to pass it as list, but at the same time
> > > keep the same output it had before.
> >
> > Yeah, it's known issue.
> >
> > https://bz.mercurial-scm.org/show_bug.cgi?id=5217#c2
> >
> > The idea I have in mind is to add new API for lists.
> >
> >   # writelist(field: str, elemname: str, elemfmt: str, data: iterable,
> >   #           sep='': str)
> >   fm.writelist('succnodes', 'node', '%s ',
> >                (hex(repl) for repl in marker.succnodes()))
> >
> > The point of the formatter API is to avoid divergence between plain and
> > template/machine outputs.  I think writelist() can handle common cases
> well.
>
> I came up with a better idea.
>
>   # formatlist(name: str, data: iterable, fmt='%s': str, sep='': str)
>   #            -> str|list|templatekw._hybrid
>   hexnodes = [hex(repl) for repl in marker.succnodes()]
>   fm.write('succnodes', '%s', fm.formatlist('node', hexnodes, fmt='%s '))
>
> It can avoid copying the functionality of (cond)?write(), and has fewer
> arguments.
>
> > > +    fm.write('date', '%s', util.datestr(marker.date()))
> >
> > It should be a date tuple in template/machine format.
>
> Maybe we could handle datestr() in the same way?
>
>   fm.formatdate(date: (int, int), format=...) -> str|(int, int)
>

On Tue, 12 Jul 2016 11:57:17 +0200, Piotr Listkiewicz wrote:
> > For this particular case, you can use fm.data() and fix _jsonifyobj() to
> > support dicts. There are more complicated examples in evolve.py and
> > journal.py,
> > but "{metadata}" can be a plain dict since it isn't a nested template
> > structure.
> 
> It wouldn't be a problem that fm.data(..) for plainformatter prints
> nothing? It means we would need two invocation of writing metadata:
> 
> fm.plain( expression_to_stringify_metadata) # only for plain
> fm.data(metadata) # only for templates != plain

Yes. If we want to avoid two separate calls, we could add fm.formatdict().

> > JSON and template outputs should always have index, i.e.
> >   fm.condwrite(showindex, 'index', '%i ', index)
> 
> As far as i checked only a plainformatter care about the condition in the
> condwrite, other templates prints value no matter what condition is. Or i
> just don't get the point.

Printing index=None is wrong. It needs a real index value plus a condition
whether or not index is printed in plain output.

> I came up with a better idea.
> >   # formatlist(name: str, data: iterable, fmt='%s': str, sep='': str)
> >   #            -> str|list|templatekw._hybrid
> >   hexnodes = [hex(repl) for repl in marker.succnodes()]
> >   fm.write('succnodes', '%s', fm.formatlist('node', hexnodes, fmt='%s '))
> > It can avoid copying the functionality of (cond)?write(), and has fewer
> > arguments.
> 
> I have a problem what formatlist should return for jsonformatter:
> 1) list - it crash because json doesnt support jsonifying list
> 2) string - it won't work because json fm.write escapes characters like '['
> 3) templatekw._hybrid - i have no idea how to do this, i see this returns
> generator, i have no idea how to use it

plainformatter.formatlist() returns a formatted string,
templateformatter.formatlist() returns a list object designed for templater,
otherwise returns a list.

I wrote a draft patch:
https://bitbucket.org/yuja/hg-work/commits/05abf1351e40

> Maybe we could handle datestr() in the same way?
> >   fm.formatdate(date: (int, int), format=...) -> str|(int, int)
> 
> Plainformatter in this case should probably  return util.datestr(date) but
> what about jsonformatter?. JSON spec does not specify how date should look
> like, should it return tuple of int and as a result print date as tuple?

[timestamp, tzoffset]

See "hg log -Tjson" for how date should be rendered.