Patchwork chistedit: properly show verbose diffs

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date April 4, 2019, 2:47 p.m.
Message ID <704f79617827ab0c19a7.1554389269@chloe>
Download mbox | patch
Permalink /patch/39477/
State Accepted
Headers show

Comments

Jordi Gutiérrez Hermoso - April 4, 2019, 2:47 p.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1554388915 14400
#      Thu Apr 04 10:41:55 2019 -0400
# Node ID 704f79617827ab0c19a788715b797fcfe8557cea
# Parent  4ee906aa7b60fb6b113e4dc187fbb5a8f42e557c
chistedit: properly show verbose diffs

I'm not sure if that ever worked and it's an internal API breakage,
but `"verbose": True` is not correctly parsed, as most of these
options are parsed by diffopts, whereas verbose is a global option.

Setting the UI to verbose instead does work and does show a verbose
patch, with full commit message.

It also shows all files, which unfortunately are a bit hard to read on
a single line in the default verbose template. Thus, we also change
the default template to use the status template, which shows one file
per line as well as its modification state.
Pulkit Goyal - April 5, 2019, 3:01 p.m.
On Thu, Apr 4, 2019 at 5:50 PM Jordi Gutiérrez Hermoso <jordigh@octave.org>
wrote:

> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1554388915 14400
> #      Thu Apr 04 10:41:55 2019 -0400
> # Node ID 704f79617827ab0c19a788715b797fcfe8557cea
> # Parent  4ee906aa7b60fb6b113e4dc187fbb5a8f42e557c
> chistedit: properly show verbose diffs
>
> I'm not sure if that ever worked and it's an internal API breakage,
> but `"verbose": True` is not correctly parsed, as most of these
> options are parsed by diffopts, whereas verbose is a global option.
>
> Setting the UI to verbose instead does work and does show a verbose
> patch, with full commit message.
>
> It also shows all files, which unfortunately are a bit hard to read on
> a single line in the default verbose template. Thus, we also change
> the default template to use the status template, which shows one file
> per line as well as its modification state.
>

Showing full commit message looks good addition. Queued this, many thanks!
Yuya Nishihara - April 6, 2019, 12:46 a.m.
On Thu, 04 Apr 2019 10:47:49 -0400, Jordi Gutiérrez Hermoso wrote:
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1554388915 14400
> #      Thu Apr 04 10:41:55 2019 -0400
> # Node ID 704f79617827ab0c19a788715b797fcfe8557cea
> # Parent  4ee906aa7b60fb6b113e4dc187fbb5a8f42e557c
> chistedit: properly show verbose diffs
> 
> I'm not sure if that ever worked and it's an internal API breakage,
> but `"verbose": True` is not correctly parsed, as most of these
> options are parsed by diffopts, whereas verbose is a global option.
> 
> Setting the UI to verbose instead does work and does show a verbose
> patch, with full commit message.
> 
> It also shows all files, which unfortunately are a bit hard to read on
> a single line in the default verbose template. Thus, we also change
> the default template to use the status template, which shows one file
> per line as well as its modification state.
> 
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -1230,8 +1230,9 @@ def addln(win, y, x, line, color=None):
>  def patchcontents(state):
>      repo = state['repo']
>      rule = state['rules'][state['pos']]
> +    repo.ui.verbose = True

Perhaps, this has to be ui.configoverride() so the original value can be
restored.
Jordi Gutiérrez Hermoso - April 8, 2019, 2:36 p.m.
On Sat, 2019-04-06 at 09:46 +0900, Yuya Nishihara wrote:
> On Thu, 04 Apr 2019 10:47:49 -0400, Jordi Gutiérrez Hermoso wrote:
> > diff --git a/hgext/histedit.py b/hgext/histedit.py
> > --- a/hgext/histedit.py
> > +++ b/hgext/histedit.py
> > @@ -1230,8 +1230,9 @@ def addln(win, y, x, line, color=None):
> >  def patchcontents(state):
> >      repo = state['repo']
> >      rule = state['rules'][state['pos']]
> > +    repo.ui.verbose = True

> Perhaps, this has to be ui.configoverride() so the original value can be
> restored.

I think we might just even set this "globally" for curses histedit.
I'm not sure where else the ui.verbose value matters here except for
displaying diffs.
Yuya Nishihara - April 8, 2019, 11:10 p.m.
On Mon, 08 Apr 2019 10:36:53 -0400, Jordi Gutiérrez Hermoso wrote:
> On Sat, 2019-04-06 at 09:46 +0900, Yuya Nishihara wrote:
> > On Thu, 04 Apr 2019 10:47:49 -0400, Jordi Gutiérrez Hermoso wrote:
> > > diff --git a/hgext/histedit.py b/hgext/histedit.py
> > > --- a/hgext/histedit.py
> > > +++ b/hgext/histedit.py
> > > @@ -1230,8 +1230,9 @@ def addln(win, y, x, line, color=None):
> > >  def patchcontents(state):
> > >      repo = state['repo']
> > >      rule = state['rules'][state['pos']]
> > > +    repo.ui.verbose = True
> > 
> > Perhaps, this has to be ui.configoverride() so the original value can be
> > restored.
> 
> I think we might just even set this "globally" for curses histedit.
> I'm not sure where else the ui.verbose value matters here except for
> displaying diffs.

I don't know whether or not we want ui.verbose globally. I'm just saying
it looks incorrect to set a temporary value to semi-global object.

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -1230,8 +1230,9 @@  def addln(win, y, x, line, color=None):
 def patchcontents(state):
     repo = state['repo']
     rule = state['rules'][state['pos']]
+    repo.ui.verbose = True
     displayer = logcmdutil.changesetdisplayer(repo.ui, repo, {
-        'patch': True, 'verbose': True
+        "patch": True,  "template": "status"
     }, buffered=True)
     displayer.show(rule.ctx)
     displayer.close()