[PATCH] hgweb: display difference for a changeset against any parents (issue2810)
Weiwen Liu
weiwen at fb.com
Fri Nov 16 13:28:21 CST 2012
Thanks for feedback.
On URL scheme, I like your suggestions. diff/<node a>:<node b> extends diff command instead of introducing a new one. And it follows more closely pattern in the NewWebInterface (NewWebInterface considers anything beyond the first node as file path). I will try this one out.
Conditional template is interesting.
Two new template variables are used: original node and baseline.
With expansion of diff command, it makes it easier to know which baseline is used by displaying baseline info in the page.
If a diff is against a node's only parent, we can skip the link for changing baseline. If a diff is against a node's ancestor, or sibling from a different branch, it is convenient to have a link to see difference against direct parent.
I prefer to keep these new variables for the purpose of consistency and simplicity in code. Let me know if this makes sense.
Thanks,
Weiwen
________________________________________
From: Matt Mackall [mpm at selenic.com]
Sent: Thursday, November 15, 2012 9:47 PM
To: Weiwen Liu
Cc: mercurial-devel at selenic.com; Bryan O'Sullivan
Subject: Re: [PATCH] hgweb: display difference for a changeset against any parents (issue2810)
On Fri, 2012-11-16 at 00:49 +0000, Weiwen Liu wrote:
> # HG changeset patch
> # User Weiwen <weiwen at fb.com>
> # Date 1352757939 28800
> # Node ID 3502c78b6b53ec6ca3e583019e8704014f9357f8
> # Parent fb14a5dcdc62987512820531fe60719d650491b6
> hgweb: display difference for a changeset against any parents (issue2810)
>
> During merge of branches, it is useful to compare merge results against
> the two parents. This change adds this support to hgweb. To specify
> which parent to compare to, use rev/12345?base=12300 where 12300 is a
> parent changeset number. Two links are added to changeset web page so
> that one can choose which parent to compare to.
I like the idea of this feature, but I'm not sure about the URL scheme.
We spent a fair amount of effort back in 2006 or so to banish this style
of argument from our browser-visible URLs, so it would be a shame to add
them back:
http://mercurial.selenic.com/wiki/NewWebInterface
Instead, we probably want a url format that accepts two args, like
http://selenic.com/hg/diff2/<node a>/<node b>[/path/to/file]
or perhaps even something like:
http://selenic.com/hg/diff/<node a>:<node b>[/path/to/file]
thus creating a precedent for using more interesting specifications in
URLs.
Also, the template engine now supports conditionals, so there's probably
no need to pass new variables to the template or to show multiple links
if there's no p2.
> diff --git a/mercurial/hgweb/webcommands.py b/mercurial/hgweb/webcommands.py
> --- a/mercurial/hgweb/webcommands.py
> +++ b/mercurial/hgweb/webcommands.py
> @@ -255,6 +255,9 @@
>
> def changeset(web, req, tmpl):
> ctx = webutil.changectx(web.repo, req)
> + basectx = webutil.changectx(web.repo, req, 'base')
> + if basectx is None:
> + basectx = ctx.p1()
> showtags = webutil.showtag(web.repo, tmpl, 'changesettag', ctx.node())
> showbookmarks = webutil.showbookmark(web.repo, tmpl, 'changesetbookmark',
> ctx.node())
> @@ -273,10 +276,10 @@
> style = req.form['style'][0]
>
> parity = paritygen(web.stripecount)
> - diffs = webutil.diffs(web.repo, tmpl, ctx, None, parity, style)
> + diffs = webutil.diffs(web.repo, tmpl, ctx, basectx, None, parity, style)
>
> parity = paritygen(web.stripecount)
> - diffstatgen = webutil.diffstatgen(ctx)
> + diffstatgen = webutil.diffstatgen(ctx, basectx)
> diffstat = webutil.diffstat(tmpl, ctx, diffstatgen, parity)
>
> return tmpl('changeset',
> @@ -285,6 +288,8 @@
> node=ctx.hex(),
> parent=webutil.parents(ctx),
> child=webutil.children(ctx),
> + baseline=webutil.parents(ctx),
> + currentbaseline=basectx.hex(),
> changesettag=showtags,
> changesetbookmark=showbookmarks,
> changesetbranch=showbranch,
> @@ -566,7 +571,7 @@
> if 'style' in req.form:
> style = req.form['style'][0]
>
> - diffs = webutil.diffs(web.repo, tmpl, ctx, [path], parity, style)
> + diffs = webutil.diffs(web.repo, tmpl, ctx, None, [path], parity, style)
> rename = fctx and webutil.renamelink(fctx) or []
> ctx = fctx and fctx or ctx
> return tmpl("filediff",
> diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
> --- a/mercurial/hgweb/webutil.py
> +++ b/mercurial/hgweb/webutil.py
> @@ -140,10 +140,13 @@
> path = path.lstrip('/')
> return scmutil.canonpath(repo.root, '', path)
>
> -def changectx(repo, req):
> +def changectx(repo, req, nodename='node'):
> changeid = "tip"
> - if 'node' in req.form:
> - changeid = req.form['node'][0]
> +
> + if nodename in req.form:
> + changeid = req.form[nodename][0]
> + elif nodename == 'base':
> + return None
> elif 'manifest' in req.form:
> changeid = req.form['manifest'][0]
>
> @@ -178,7 +181,7 @@
> if len(files) > max:
> yield tmpl('fileellipses')
>
> -def diffs(repo, tmpl, ctx, files, parity, style):
> +def diffs(repo, tmpl, ctx, basectx, files, parity, style):
>
> def countgen():
> start = 1
> @@ -209,8 +212,11 @@
> m = match.always(repo.root, repo.getcwd())
>
> diffopts = patch.diffopts(repo.ui, untrusted=True)
> - parents = ctx.parents()
> - node1 = parents and parents[0].node() or nullid
> + if basectx is None:
> + parents = ctx.parents()
> + node1 = parents and parents[0].node() or nullid
> + else:
> + node1 = basectx.node()
> node2 = ctx.node()
>
> block = []
> @@ -274,10 +280,10 @@
> for oc in s.get_grouped_opcodes(n=context):
> yield tmpl('comparisonblock', lines=getblock(oc))
>
> -def diffstatgen(ctx):
> +def diffstatgen(ctx, basectx):
> '''Generator function that provides the diffstat data.'''
>
> - stats = patch.diffstatdata(util.iterlines(ctx.diff()))
> + stats = patch.diffstatdata(util.iterlines(ctx.diff(basectx)))
> maxname, maxtotal, addtotal, removetotal, binary = patch.diffstatsum(stats)
> while True:
> yield stats, maxname, maxtotal, addtotal, removetotal, binary
> diff --git a/mercurial/templater.py b/mercurial/templater.py
> --- a/mercurial/templater.py
> +++ b/mercurial/templater.py
> @@ -179,6 +179,7 @@
> for i in d:
> if isinstance(i, dict):
> lm.update(i)
> + lm['originalnode'] = mapping.get('node')
> yield runtemplate(context, lm, ctmpl)
> else:
> # v is not an iterable of dicts, this happen when 'key'
> diff --git a/mercurial/templates/paper/changeset.tmpl b/mercurial/templates/paper/changeset.tmpl
> --- a/mercurial/templates/paper/changeset.tmpl
> +++ b/mercurial/templates/paper/changeset.tmpl
> @@ -74,6 +74,14 @@
> </div>
> </td>
> </tr>
> +<tr>
> + <th class="author">change baseline</th>
> + <td class="author">{baseline%changesetbaseline}</td>
> +</tr>
> +<tr>
> + <th class="author">current baseline</th>
> + <td class="author"><a href="{url}rev/{currentbaseline|short}{sessionvars%urlparameter}">{currentbaseline|short}</a> </td>
> +</tr>
> </table>
>
> <div class="overflow">
> diff --git a/mercurial/templates/paper/map b/mercurial/templates/paper/map
> --- a/mercurial/templates/paper/map
> +++ b/mercurial/templates/paper/map
> @@ -101,6 +101,8 @@
>
> changesetparent = '<a href="{url}rev/{node|short}{sessionvars%urlparameter}">{node|short}</a> '
>
> +changesetbaseline = '<a href="{url}rev/{originalnode|short}?base={node|short}{sessionvars%urlparameter}">{node|short} </a> '
> +
> filerevparent = '<a href="{url}file/{node|short}/{file|urlescape}{sessionvars%urlparameter}">{rename%filerename}{node|short}</a> '
> filerevchild = '<a href="{url}file/{node|short}/{file|urlescape}{sessionvars%urlparameter}">{node|short}</a> '
>
> 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
> @@ -441,6 +441,14 @@
> </div>
> </td>
> </tr>
> + <tr>
> + <th class="author">change baseline</th>
> + <td class="author"></td>
> + </tr>
> + <tr>
> + <th class="author">current baseline</th>
> + <td class="author"><a href="/rev/000000000000">000000000000</a> </td>
> + </tr>
> </table>
>
> <div class="overflow">
> diff --git a/tests/test-hgweb-diffs.t b/tests/test-hgweb-diffs.t
> --- a/tests/test-hgweb-diffs.t
> +++ b/tests/test-hgweb-diffs.t
> @@ -139,6 +139,14 @@
> </div>
> </td>
> </tr>
> + <tr>
> + <th class="author">change baseline</th>
> + <td class="author"></td>
> + </tr>
> + <tr>
> + <th class="author">current baseline</th>
> + <td class="author"><a href="/rev/000000000000">000000000000</a> </td>
> + </tr>
> </table>
>
> <div class="overflow">
> @@ -400,6 +408,14 @@
> </div>
> </td>
> </tr>
> + <tr>
> + <th class="author">change baseline</th>
> + <td class="author"></td>
> + </tr>
> + <tr>
> + <th class="author">current baseline</th>
> + <td class="author"><a href="/rev/000000000000">000000000000</a> </td>
> + </tr>
> </table>
>
> <div class="overflow">
> diff --git a/tests/test-hgweb-removed.t b/tests/test-hgweb-removed.t
> --- a/tests/test-hgweb-removed.t
> +++ b/tests/test-hgweb-removed.t
> @@ -112,6 +112,14 @@
> </div>
> </td>
> </tr>
> + <tr>
> + <th class="author">change baseline</th>
> + <td class="author"><a href="/rev/c78f6c5cbea9?base=cb9a9f314b8b">cb9a9f314b8b </a> </td>
> + </tr>
> + <tr>
> + <th class="author">current baseline</th>
> + <td class="author"><a href="/rev/cb9a9f314b8b">cb9a9f314b8b</a> </td>
> + </tr>
> </table>
>
> <div class="overflow">
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
--
Mathematics is the supreme nostalgia of our time.
More information about the Mercurial-devel
mailing list