Proposing compact graph log

[Mads Kiilerich]

On 06/01/2014 10:58 PM, Santiago Payà i Miralta wrote:
>
> Hello,
>
> I would ask for a review of the --graph log option. When using --graph 
> with a --template the resulting DAG chain always shows the changesets 
> connected by a row of vertical edges '|', regardless of using '\n' or 
> not in the --template parameter:
>
> $ hg log -G --template={rev} -l 3
> @  21644
> |
> o  21643
> |
> o  21642
> |
>
> And what is proposed is:
>
> $ hg log -G --template={rev} -l 3
> @  21644
> o  21643
> o  21642
>

The example shows the trivial case. The interesting case is the 
non-trivial one where you also have non-vertical lines - that is what 
ultimately should be in the patch description.

> You can see my point of view detailed in this blog [1].
>
> There are some problems with this change:
>
> * Breaks some (57) tests :(
>

It would also break existing uses of Mercurial. I doubt it can be 
changed now. The default must stay the same.

> * Could depend from an option to the log command, some as --compact or 
> similar, in this way the previous behavior would not be broken.
>

Yes, it could perhaps be added to Mercurial if controlled this way.

> * Works filtering the resulting graph, may be should work in the graph 
> generation (?).
>

Yes. Probably just by looking at coldiff.

> About the code be aware that, in my opinion, the comment in line 
> graphmod:327[2] is not true, the `shift_interline' list contains all 
> non-vertical _and_ vertical edges.
>

The comment doesn't say that there is no other edges. It just says that 
the line is there to handle the non-vertical edges - that is what makes 
it special.

> Well, this is just a proposal that if deemed correct should be 
> developed. Was it discussed at some point when the graphlog extension ?
>

I would be -0.1 for making the output more inconsistent and another -0.1 
for adding another option ... and to that I would add +0.2 because I can 
see how it could be convenient even though it is harder to read.

I suggest looking at 
http://mercurial.selenic.com/wiki/ContributingChanges and contributing a 
full patch so we know exactly what we are discussing.

A second patch could perhaps also skip the extra padding lines in this 
compact mode.

/Mads


> Best regards,
>
> Santiago
>
> [1] http://print--hello--world.blogspot.com.es/2014_05_01_archive.html
> [2] http://selenic.com/hg/file/9c35f3a8cac4/mercurial/graphmod.py#l327
>
> - - - -
>
> diff -r 76a347bcdb33 -r 9c5a8e65386a mercurial/graphmod.py
> --- a/mercurial/graphmod.py Thu May 29 16:01:39 2014 -0700
> +++ b/mercurial/graphmod.py Fri May 30 22:09:31 2014 +0200
> @@ -346,7 +346,14 @@
>      lines = [nodeline]
>      if add_padding_line:
>          lines.append(_getpaddingline(idx, ncols, edges))
> -    lines.append(shift_interline)
> +
> +    # do not add shift_interline if are all vertical edges '|'
> +    add_shift_interline = False
> +    for token in shift_interline:
> +        if token not in ['|', ' ']:
> +            add_shift_interline = True
> +    if add_shift_interline:
> +        lines.append(shift_interline)
>

It could probably just be something like

if util.any(c not in '| ' for c in shift_interline):
     lines.append(shift_interline)

/Mads