Re: [PATCH] graphlog: draw multiple edges towards null node (issue5440)

[Augie Fackler]

On Mon, Mar 20, 2017 at 11:24:45AM +, Ryan McElroy wrote:
> On 3/20/17 8:22 AM, Yuya Nishihara wrote:
> ># HG changeset patch
> ># User Yuya Nishihara 
> ># Date 1489978255 -32400
> >#  Mon Mar 20 11:50:55 2017 +0900
> ># Node ID db70a30dde5182e704b02b0e47a79d3ecfafdd49
> ># Parent  cede8eaf55e2fe68434c273cfd066512b5ca112e
> >graphlog: draw multiple edges towards null node (issue5440)

queued, thanks

> >
> >Before, edge (r, null) was processed only once by newparents. However what
> >we really need is just stripping the edge (null, null).
> >
>

[...]

> I'd like to re-state for the record my wish that for subtle bugfixes like
> this that we could split the fix into two patches: a patch that adds a test
> exhibiting the broken behavior, and then the fix with the corrections to the
> test. That would make it *so much easier* to review a patch like this.

+1

>
> For the curious (like me), the previous (broken) behavior was this (note the
> missing /):
>
>@  1 bar
>|
>| o  0 foo
> |
>o  -1
>
> The difference in the test output with this patch is thus:
>
>@  1 bar
>|
>| o  0 foo
> -  |
> +  |/
>o  -1
>
> Overall, this change looks good to me.
> ___
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH] graphlog: draw multiple edges towards null node (issue5440)

[Ryan McElroy]

On 3/20/17 8:22 AM, Yuya Nishihara wrote:

# HG changeset patch
# User Yuya Nishihara 
# Date 1489978255 -32400
#  Mon Mar 20 11:50:55 2017 +0900
# Node ID db70a30dde5182e704b02b0e47a79d3ecfafdd49
# Parent  cede8eaf55e2fe68434c273cfd066512b5ca112e
graphlog: draw multiple edges towards null node (issue5440)

Before, edge (r, null) was processed only once by newparents. However what
we really need is just stripping the edge (null, null).

diff --git a/mercurial/graphmod.py b/mercurial/graphmod.py
--- a/mercurial/graphmod.py
+++ b/mercurial/graphmod.py
@@ -182,6 +182,9 @@ def asciiedges(type, char, lines, state,
  knownparents = []
  newparents = []
  for ptype, parent in parents:
+if parent == rev:
+# self reference (should only be seen in null rev)
+continue
  if parent in seen:
  knownparents.append(parent)
  else:
@@ -191,8 +194,7 @@ def asciiedges(type, char, lines, state,
  ncols = len(seen)
  nextseen = seen[:]
  nextseen[nodeidx:nodeidx + 1] = newparents
-edges = [(nodeidx, nextseen.index(p))
- for p in knownparents if p != nullrev]
+edges = [(nodeidx, nextseen.index(p)) for p in knownparents]
  
  seen[:] = nextseen

  while len(newparents) > 2:
diff --git a/tests/test-glog.t b/tests/test-glog.t
--- a/tests/test-glog.t
+++ b/tests/test-glog.t
@@ -3424,3 +3424,39 @@ the right node styles are used (issue517
   summary: 0

  
+  $ cd ..

+
+Multiple roots (issue5440):
+
+  $ hg init multiroots
+  $ cd multiroots
+  $ cat < .hg/hgrc
+  > [ui]
+  > logtemplate = '{rev} {desc}\n\n'
+  > EOF
+
+  $ touch foo
+  $ hg ci -Aqm foo
+  $ hg co -q null
+  $ touch bar
+  $ hg ci -Aqm bar
+
+  $ hg log -Gr null:
+  @  1 bar
+  |
+  | o  0 foo
+  |/
+  o  -1
+
+  $ hg log -Gr null+0
+  o  0 foo
+  |
+  o  -1
+
+  $ hg log -Gr null+1
+  @  1 bar
+  |
+  o  -1
+
+
+  $ cd ..


I'd like to re-state for the record my wish that for subtle bugfixes 
like this that we could split the fix into two patches: a patch that 
adds a test exhibiting the broken behavior, and then the fix with the 
corrections to the test. That would make it *so much easier* to review a 
patch like this.


For the curious (like me), the previous (broken) behavior was this (note 
the missing /):


   @  1 bar
   |
   | o  0 foo
|
   o  -1

The difference in the test output with this patch is thus:

   @  1 bar
   |
   | o  0 foo
-  |
+  |/
   o  -1

Overall, this change looks good to me.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH] graphlog: draw multiple edges towards null node (issue5440)

[Yuya Nishihara]

# HG changeset patch
# User Yuya Nishihara 
# Date 1489978255 -32400
#  Mon Mar 20 11:50:55 2017 +0900
# Node ID db70a30dde5182e704b02b0e47a79d3ecfafdd49
# Parent  cede8eaf55e2fe68434c273cfd066512b5ca112e
graphlog: draw multiple edges towards null node (issue5440)

Before, edge (r, null) was processed only once by newparents. However what
we really need is just stripping the edge (null, null).

diff --git a/mercurial/graphmod.py b/mercurial/graphmod.py
--- a/mercurial/graphmod.py
+++ b/mercurial/graphmod.py
@@ -182,6 +182,9 @@ def asciiedges(type, char, lines, state,
 knownparents = []
 newparents = []
 for ptype, parent in parents:
+if parent == rev:
+# self reference (should only be seen in null rev)
+continue
 if parent in seen:
 knownparents.append(parent)
 else:
@@ -191,8 +194,7 @@ def asciiedges(type, char, lines, state,
 ncols = len(seen)
 nextseen = seen[:]
 nextseen[nodeidx:nodeidx + 1] = newparents
-edges = [(nodeidx, nextseen.index(p))
- for p in knownparents if p != nullrev]
+edges = [(nodeidx, nextseen.index(p)) for p in knownparents]
 
 seen[:] = nextseen
 while len(newparents) > 2:
diff --git a/tests/test-glog.t b/tests/test-glog.t
--- a/tests/test-glog.t
+++ b/tests/test-glog.t
@@ -3424,3 +3424,39 @@ the right node styles are used (issue517
  summary: 0
   
 
+  $ cd ..
+
+Multiple roots (issue5440):
+
+  $ hg init multiroots
+  $ cd multiroots
+  $ cat < .hg/hgrc
+  > [ui]
+  > logtemplate = '{rev} {desc}\n\n'
+  > EOF
+
+  $ touch foo
+  $ hg ci -Aqm foo
+  $ hg co -q null
+  $ touch bar
+  $ hg ci -Aqm bar
+
+  $ hg log -Gr null:
+  @  1 bar
+  |
+  | o  0 foo
+  |/
+  o  -1
+  
+  $ hg log -Gr null+0
+  o  0 foo
+  |
+  o  -1
+  
+  $ hg log -Gr null+1
+  @  1 bar
+  |
+  o  -1
+  
+
+  $ cd ..
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel