Patchwork [2,of,2] log: display the current node as "*" instead of "@" in the graph (BC)

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date Nov. 24, 2014, 9:49 p.m.
Message ID <b0cfd48c238f446dfb09.1416865740@Iris>
Download mbox | patch
Permalink /patch/6843/
State Rejected
Headers show

Comments

Jordi Gutiérrez Hermoso - Nov. 24, 2014, 9:49 p.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1416865655 18000
#      Mon Nov 24 16:47:35 2014 -0500
# Node ID b0cfd48c238f446dfb0983e81a2af314f9347bdd
# Parent  eefc121a32a3e1ed848f603d062f20f2aff0cdbd
log: display the current node as "*" instead of "@" in the graph (BC)

There is an unfortunate concept clash between the special "@" bookmark
and the graphlog: they both use "@" to represent unrelated concepts.
While it's obviously impossible to change the name of the "@"
bookmark, I am hoping that parsing the graphlog is rare enough to
warrant this change.

The asterisk as a node character has already been used in `git log
--graph`, so it appears to already be in common use in ASCII DAGs
Martin von Zweigbergk - Nov. 25, 2014, 12:53 a.m.
I considered changing it to '.' as that is what it's called on the command
line. I didn't send a patch due to BC concerns, but I'd personally be happy
if this were accepted. What do you think about '.'?
Jordi Gutiérrez Hermoso - Nov. 25, 2014, 5:17 p.m.
On Tue, 2014-11-25 at 00:53 +0000, Martin von Zweigbergk wrote:
> I considered changing it to '.' as that is what it's called on the
> command line. I didn't send a patch due to BC concerns, but I'd
> personally be happy if this were accepted. What do you think about
> '.'?

That might be a good idea, but it seems that the character is a bit
too small to see on the DAG. Do you like how it looks? I think I
prefer the asterisk version.
Martin von Zweigbergk - Nov. 25, 2014, 5:30 p.m.
Yes, it is a little small. I can see how it might be mistaken for just a
turn in the graph (i.e. a drawing artifact). It's just logically what I'd
expect. So I'm undecided.

On Tue Nov 25 2014 at 9:17:36 AM Jordi Gutiérrez Hermoso <jordigh@octave.org>
wrote:

> On Tue, 2014-11-25 at 00:53 +0000, Martin von Zweigbergk wrote:
> > I considered changing it to '.' as that is what it's called on the
> > command line. I didn't send a patch due to BC concerns, but I'd
> > personally be happy if this were accepted. What do you think about
> > '.'?
>
> That might be a good idea, but it seems that the character is a bit
> too small to see on the DAG. Do you like how it looks? I think I
> prefer the asterisk version.
>
>
Pierre-Yves David - Nov. 25, 2014, 6:27 p.m.
On 11/25/2014 09:30 AM, Martin von Zweigbergk wrote:
> Yes, it is a little small. I can see how it might be mistaken for just a
> turn in the graph (i.e. a drawing artifact). It's just logically what
> I'd expect. So I'm undecided.

We'll need dot like char to display ellipsis, so using . for a node 
seems a bad idea.
Matt Mackall - Nov. 26, 2014, 9:23 p.m.
On Mon, 2014-11-24 at 16:49 -0500, Jordi Gutiérrez Hermoso wrote:
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1416865655 18000
> #      Mon Nov 24 16:47:35 2014 -0500
> # Node ID b0cfd48c238f446dfb0983e81a2af314f9347bdd
> # Parent  eefc121a32a3e1ed848f603d062f20f2aff0cdbd
> log: display the current node as "*" instead of "@" in the graph (BC)

Patch was pre-rejected. From IRC last week:

(14:22:23) mpm: 
Not going to change graphlog '@'. Might consider allowing customizing of
the characters used in some fashion though.
(14:23:05) JordiGH: 
mpm: Meh, hiding better features under obscure configs is pointless.
(14:23:14) JordiGH: 
So it can't change, how sad.
Jordi Gutiérrez Hermoso - Nov. 27, 2014, 5:36 p.m.
On Wed, 2014-11-26 at 15:23 -0600, Matt Mackall wrote:
> On Mon, 2014-11-24 at 16:49 -0500, Jordi Gutiérrez Hermoso wrote:
> > # HG changeset patch
> > # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> > # Date 1416865655 18000
> > #      Mon Nov 24 16:47:35 2014 -0500
> > # Node ID b0cfd48c238f446dfb0983e81a2af314f9347bdd
> > # Parent  eefc121a32a3e1ed848f603d062f20f2aff0cdbd
> > log: display the current node as "*" instead of "@" in the graph (BC)
> 
> Patch was pre-rejected. From IRC last week:

I figured it wasn't an official rejection until it was on the mailing
list.

> (14:22:23) mpm: 
> Not going to change graphlog '@'.

So you do consider graphlog part of our stdout which is thus subject
to our API promise? Does this also mean you'll reject _ for
closing-branch commits?
Augie Fackler - Nov. 28, 2014, 4:12 p.m.
On Nov 27, 2014, at 12:36 PM, Jordi Gutiérrez Hermoso <jordigh@octave.org> wrote:

> On Wed, 2014-11-26 at 15:23 -0600, Matt Mackall wrote:
>> On Mon, 2014-11-24 at 16:49 -0500, Jordi Gutiérrez Hermoso wrote:
>>> # HG changeset patch
>>> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
>>> # Date 1416865655 18000
>>> #      Mon Nov 24 16:47:35 2014 -0500
>>> # Node ID b0cfd48c238f446dfb0983e81a2af314f9347bdd
>>> # Parent  eefc121a32a3e1ed848f603d062f20f2aff0cdbd
>>> log: display the current node as "*" instead of "@" in the graph (BC)
>> 
>> Patch was pre-rejected. From IRC last week:
> 
> I figured it wasn't an official rejection until it was on the mailing
> list.

You could probably have a config knob for this though?

(I know you're strongly opposed to things behind knobs and see them as useless, but it would be better than nothing for your use case?)

> 
>> (14:22:23) mpm: 
>> Not going to change graphlog '@'.
> 
> So you do consider graphlog part of our stdout which is thus subject
> to our API promise? Does this also mean you'll reject _ for
> closing-branch commits?
> 
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Jordi Gutiérrez Hermoso - Nov. 28, 2014, 5:10 p.m.
On Fri, 2014-11-28 at 11:12 -0500, Augie Fackler wrote:
> On Nov 27, 2014, at 12:36 PM, Jordi Gutiérrez Hermoso <jordigh@octave.org> wrote:
> 
> > On Wed, 2014-11-26 at 15:23 -0600, Matt Mackall wrote:
> >> On Mon, 2014-11-24 at 16:49 -0500, Jordi Gutiérrez Hermoso wrote:
> >>> # HG changeset patch
> >>> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> >>> # Date 1416865655 18000
> >>> #      Mon Nov 24 16:47:35 2014 -0500
> >>> # Node ID b0cfd48c238f446dfb0983e81a2af314f9347bdd
> >>> # Parent  eefc121a32a3e1ed848f603d062f20f2aff0cdbd
> >>> log: display the current node as "*" instead of "@" in the graph (BC)
> >> 
> >> Patch was pre-rejected. From IRC last week:
> > 
> > I figured it wasn't an official rejection until it was on the mailing
> > list.
> 
> You could probably have a config knob for this though?
> 
> (I know you're strongly opposed to things behind knobs and see them
> as useless, but it would be better than nothing for your use case?)

My use case is making it easy to teach hg to people. If I have to
first explain the config knob, this doesn't make it any easier than
explaining how "@" has different meanings in different contexts. I
also don't like to give people a magic config file without explaining
what it does.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1919,7 +1919,7 @@  def displaygraph(ui, dag, displayer, sho
     for rev, type, ctx, parents in dag:
         char = 'o'
         if ctx.node() in showparents:
-            char = '@'
+            char = '*'
         elif ctx.obsolete():
             char = 'x'
         elif ctx.closesbranch():