Patchwork hgweb: make different kinds of commits look differently on /graph (RFC)

login
register
mail settings
Submitter Anton Shestakov
Date Dec. 19, 2017, 1:52 p.m.
Message ID <c7259f4340b50c8b1863.1513691535@neuro>
Download mbox | patch
Permalink /patch/26363/
State RFC, archived
Headers show

Comments

Anton Shestakov - Dec. 19, 2017, 1:52 p.m.
# HG changeset patch
# User Anton Shestakov <av6@dwimlabs.net>
# Date 1513687285 -28800
#      Tue Dec 19 20:41:25 2017 +0800
# Node ID c7259f4340b50c8b1863ac9a1379ab8412b27b87
# Parent  e28dedf4ff4368820bbb085c668c3e6ea2088842
hgweb: make different kinds of commits look differently on /graph (RFC)

This is RFC for two reasons:

I'm more or less fine with how things look visually (screenshots are in a
follow-up email), but there's still room for improvement. Feel free to
criticise or point me to good-looking graphs of this kind for inspiration.

The second reason is a bit more interesting: currently this patch simply adds
more items to "vertex" tuple in jsdata.

I'd like to add to jsdata something that would determine how a graph node
should look, but the problem is that there are already "node" and "vertex" keys
in those dicts. This patch has one solution to this problem, but "vertex" is
actually a tuple of (column, color), and so probably could be split into those,
freeing up "vertex" for this use case. Or we could have "graphnode" and not
touch "vertex" at all, but then it might be confused with "node", I don't know.

Also, that string includes the style of a particular node and also if it's
currently checked out or not, both at the same time. In hg log -G it's not done
like this, there it's only one character (see templatekw.showgraphnode), but
hgweb doesn't need to have this limitation, since it uses <canvas> and not
plain text. I'm using one string of 1 or 2 characters in this patch, but it
could also be done differently.

Opinions?
Anton Shestakov - Dec. 19, 2017, 1:54 p.m.
On Tue, 19 Dec 2017 21:52:15 +0800
Anton Shestakov <av6@dwimlabs.net> wrote:

> # HG changeset patch
> # User Anton Shestakov <av6@dwimlabs.net>
> # Date 1513687285 -28800
> #      Tue Dec 19 20:41:25 2017 +0800
> # Node ID c7259f4340b50c8b1863ac9a1379ab8412b27b87
> # Parent  e28dedf4ff4368820bbb085c668c3e6ea2088842
> hgweb: make different kinds of commits look differently on /graph (RFC)
> 
> This is RFC for two reasons:
> 
> I'm more or less fine with how things look visually (screenshots are in a
> follow-up email), but there's still room for improvement. Feel free to
> criticise or point me to good-looking graphs of this kind for inspiration.

Closed (and reopened) branch, branch-closing commit checked out:
https://snaps.dwimlabs.net/graph-closed.png

Obsolete changes:
https://snaps.dwimlabs.net/graph-obsolete.png

Custom color+width for default branch with obsolete changes:
https://snaps.dwimlabs.net/graph-pink-default.png
Yuya Nishihara - Jan. 5, 2018, 6:47 a.m.
On Tue, 19 Dec 2017 21:52:15 +0800, Anton Shestakov wrote:
> # HG changeset patch
> # User Anton Shestakov <av6@dwimlabs.net>
> # Date 1513687285 -28800
> #      Tue Dec 19 20:41:25 2017 +0800
> # Node ID c7259f4340b50c8b1863ac9a1379ab8412b27b87
> # Parent  e28dedf4ff4368820bbb085c668c3e6ea2088842
> hgweb: make different kinds of commits look differently on /graph (RFC)
> 
> This is RFC for two reasons:
> 
> I'm more or less fine with how things look visually (screenshots are in a
> follow-up email), but there's still room for improvement. Feel free to
> criticise or point me to good-looking graphs of this kind for inspiration.

I have no preference, and this patch is floating more than 2 weeks, so let's
make it happen to get a feedback. Can you send V2?

> The second reason is a bit more interesting: currently this patch simply adds
> more items to "vertex" tuple in jsdata.
> 
> I'd like to add to jsdata something that would determine how a graph node
> should look, but the problem is that there are already "node" and "vertex" keys
> in those dicts. This patch has one solution to this problem, but "vertex" is
> actually a tuple of (column, color), and so probably could be split into those,
> freeing up "vertex" for this use case. Or we could have "graphnode" and not
> touch "vertex" at all, but then it might be confused with "node", I don't know.

I slightly prefer a set of named attributes e.g. "obsolete": true, but that
wouldn't be important.
Augie Fackler - Jan. 6, 2018, 8:22 p.m.
> On Jan 5, 2018, at 1:47 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> On Tue, 19 Dec 2017 21:52:15 +0800, Anton Shestakov wrote:
>> # HG changeset patch
>> # User Anton Shestakov <av6@dwimlabs.net>
>> # Date 1513687285 -28800
>> #      Tue Dec 19 20:41:25 2017 +0800
>> # Node ID c7259f4340b50c8b1863ac9a1379ab8412b27b87
>> # Parent  e28dedf4ff4368820bbb085c668c3e6ea2088842
>> hgweb: make different kinds of commits look differently on /graph (RFC)
>> 
>> This is RFC for two reasons:
>> 
>> I'm more or less fine with how things look visually (screenshots are in a
>> follow-up email), but there's still room for improvement. Feel free to
>> criticise or point me to good-looking graphs of this kind for inspiration.
> 
> I have no preference, and this patch is floating more than 2 weeks, so let's
> make it happen to get a feedback. Can you send V2?

+1: let’s go ahead and land this and we can back it out before the final release if we decide we don’t care for it.
Anton Shestakov - Jan. 7, 2018, 9:55 a.m.
On Fri, 5 Jan 2018 15:47:23 +0900
Yuya Nishihara <yuya@tcha.org> wrote:

> On Tue, 19 Dec 2017 21:52:15 +0800, Anton Shestakov wrote:
> > # HG changeset patch
> > # User Anton Shestakov <av6@dwimlabs.net>
> > # Date 1513687285 -28800
> > #      Tue Dec 19 20:41:25 2017 +0800
> > # Node ID c7259f4340b50c8b1863ac9a1379ab8412b27b87
> > # Parent  e28dedf4ff4368820bbb085c668c3e6ea2088842
> > hgweb: make different kinds of commits look differently on /graph (RFC)
> > 
> > This is RFC for two reasons:
> > 
> > I'm more or less fine with how things look visually (screenshots are in a
> > follow-up email), but there's still room for improvement. Feel free to
> > criticise or point me to good-looking graphs of this kind for inspiration.  
> 
> I have no preference, and this patch is floating more than 2 weeks, so let's
> make it happen to get a feedback. Can you send V2?

I can send a V2, but I'd like to take a stab at making graph use SVG
first. Let's see if I can do it before the freeze, in the next couple of
days, and if not, then I'll resend this patch. As it is now, this patch
would only be half useful for SVG, because SVG would render graph nodes
in a different way.
Anton Shestakov - Jan. 9, 2018, 11:52 a.m.
On Sun, 7 Jan 2018 17:55:49 +0800
Anton Shestakov <av6@dwimlabs.net> wrote:

> On Fri, 5 Jan 2018 15:47:23 +0900
> Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > On Tue, 19 Dec 2017 21:52:15 +0800, Anton Shestakov wrote:  
> > > # HG changeset patch
> > > # User Anton Shestakov <av6@dwimlabs.net>
> > > # Date 1513687285 -28800
> > > #      Tue Dec 19 20:41:25 2017 +0800
> > > # Node ID c7259f4340b50c8b1863ac9a1379ab8412b27b87
> > > # Parent  e28dedf4ff4368820bbb085c668c3e6ea2088842
> > > hgweb: make different kinds of commits look differently on /graph (RFC)
> > > 
> > > This is RFC for two reasons:
> > > 
> > > I'm more or less fine with how things look visually (screenshots are in a
> > > follow-up email), but there's still room for improvement. Feel free to
> > > criticise or point me to good-looking graphs of this kind for inspiration.    
> > 
> > I have no preference, and this patch is floating more than 2 weeks, so let's
> > make it happen to get a feedback. Can you send V2?  
> 
> I can send a V2, but I'd like to take a stab at making graph use SVG
> first. Let's see if I can do it before the freeze, in the next couple of
> days, and if not, then I'll resend this patch. As it is now, this patch
> would only be half useful for SVG, because SVG would render graph nodes
> in a different way.

Turns out making SVG graph look the same way across browsers is
difficult. For example, Chromium has about 0.5px vertical offset
compared to Firefox, which makes nodes look blurred (because of
anti-aliasing). I'll try and figure out if there's a way to make it
behave, but for now I've sent a V2 of this patch.

Patch

diff --git a/mercurial/hgweb/webcommands.py b/mercurial/hgweb/webcommands.py
--- a/mercurial/hgweb/webcommands.py
+++ b/mercurial/hgweb/webcommands.py
@@ -13,7 +13,7 @@  import os
 import re
 
 from ..i18n import _
-from ..node import hex, short
+from ..node import hex, nullid, short
 
 from .common import (
     ErrorResponse,
@@ -1248,6 +1248,20 @@  def graph(web, req, tmpl):
         tree = list(item for item in graphmod.colored(dag, web.repo)
                     if item[1] == graphmod.CHANGESET)
 
+    def graphnode(ctx):
+        current = ''
+        wpnodes = web.repo.dirstate.parents()
+        if wpnodes[1] == nullid:
+            wpnodes = wpnodes[:1]
+        if ctx.node() in wpnodes:
+            current = '@'
+        if ctx.obsolete():
+            return current + 'x'
+        elif ctx.closesbranch():
+            return current + '_'
+        else:
+            return current + 'o'
+
     def fulltree():
         pos = web.repo[graphtop].rev()
         tree = []
@@ -1260,7 +1274,7 @@  def graph(web, req, tmpl):
 
     def jsdata():
         return [{'node': pycompat.bytestr(ctx),
-                 'vertex': vtx,
+                 'vertex': vtx + (graphnode(ctx),),
                  'edges': edges}
                 for (id, type, ctx, vtx, edges) in fulltree()]
 
diff --git a/mercurial/templates/static/mercurial.js b/mercurial/templates/static/mercurial.js
--- a/mercurial/templates/static/mercurial.js
+++ b/mercurial/templates/static/mercurial.js
@@ -91,11 +91,53 @@  Graph.prototype = {
 
 	},
 
-	vertex: function(x, y, radius, color, parity, cur) {
+	vertexCurrent: function(x, y, radius) {
+		this.ctx.lineWidth = 2;
 		this.ctx.beginPath();
-		this.setColor(color, 0.25, 0.75);
+		this.ctx.arc(x, y, radius * 1.75, 0, Math.PI * 2, true);
+		this.ctx.stroke();
+	},
+
+	vertexClosing: function(x, y, radius) {
+		this.ctx.fillRect(x - radius, y - 1.5, radius * 2, 3);
+	},
+
+	vertexObsolete: function(x, y, radius) {
+		var p45 = radius * Math.cos(Math.PI / 4);
+		this.ctx.lineWidth = 3;
+		this.ctx.beginPath();
+		this.ctx.moveTo(x - p45, y - p45);
+		this.ctx.lineTo(x + p45, y + p45);
+		this.ctx.moveTo(x - p45, y + p45);
+		this.ctx.lineTo(x + p45, y - p45);
+		this.ctx.stroke();
+	},
+
+	vertexNormal: function(x, y, radius) {
+		this.ctx.beginPath();
 		this.ctx.arc(x, y, radius, 0, Math.PI * 2, true);
 		this.ctx.fill();
+	},
+
+	vertex: function(x, y, radius, color, parity, cur) {
+		this.ctx.save();
+		this.setColor(color, 0.25, 0.75);
+		var vtx = cur.vertex[2];
+		if (vtx[0] === '@') {
+			this.vertexCurrent(x, y, radius);
+			vtx = vtx.substr(1);
+		}
+		switch (vtx) {
+			case '_':
+				this.vertexClosing(x, y, radius);
+				break;
+			case 'x':
+				this.vertexObsolete(x, y, radius);
+				break;
+			default:
+				this.vertexNormal(x, y, radius);
+		}
+		this.ctx.restore();
 
 		var left = (this.bg_height - this.box_size) + (this.columns + 1) * this.box_size;
 		var item = document.querySelector('[data-node="' + cur.node + '"]');
diff --git a/tests/test-hgweb-commands.t b/tests/test-hgweb-commands.t
--- a/tests/test-hgweb-commands.t
+++ b/tests/test-hgweb-commands.t
@@ -1814,7 +1814,7 @@  Overviews
   </div>
   
   <script>
-  var data = [{"edges": [[0, 0, 1, 3, "FF0000"]], "node": "cad8025a2e87", "vertex": [0, 1]}, {"edges": [[0, 0, 1, 3, ""]], "node": "1d22e65f027e", "vertex": [0, 1]}, {"edges": [[0, 0, 1, 3, ""]], "node": "a4f92ed23982", "vertex": [0, 1]}, {"edges": [], "node": "2ef0ac749a14", "vertex": [0, 1]}];
+  var data = [{"edges": [[0, 0, 1, 3, "FF0000"]], "node": "cad8025a2e87", "vertex": [0, 1, "@o"]}, {"edges": [[0, 0, 1, 3, ""]], "node": "1d22e65f027e", "vertex": [0, 1, "o"]}, {"edges": [[0, 0, 1, 3, ""]], "node": "a4f92ed23982", "vertex": [0, 1, "o"]}, {"edges": [], "node": "2ef0ac749a14", "vertex": [0, 1, "o"]}];
   var graph = new Graph();
   graph.scale(39);