Patchwork cmdutil: show diffs in commit message with ui.verbosecommit option

login
register
mail settings
Submitter Jordi Gutiérrez Hermoso
Date Jan. 11, 2014, 7:12 p.m.
Message ID <b40695568db84a43b24c.1389467541@Iris>
Download mbox | patch
Permalink /patch/3293/
State Changes Requested
Headers show

Comments

Jordi Gutiérrez Hermoso - Jan. 11, 2014, 7:12 p.m.
# HG changeset patch
# User Jordi Gutiérrez Hermoso <jordigh@octave.org>
# Date 1373469596 14400
#      Wed Jul 10 11:19:56 2013 -0400
# Node ID b40695568db84a43b24cffb4c32583a759d96b09
# Parent  01bdccfeb9d98ae85388d06c9c694b946f346edf
cmdutil: show diffs in commit message with ui.verbosecommit option

The following adds an option, ui.verbosecommit, which displays the
entire diff about to be committed in the text editor window, prefixed
with "HG:". This is useful as a final reminder of what's about to be
committed while writing the commit message.
Martin Geisler - Jan. 11, 2014, 9:20 p.m.
Jordi Gutiérrez Hermoso <jordigh@octave.org> writes:

> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> # Date 1373469596 14400
> #      Wed Jul 10 11:19:56 2013 -0400
> # Node ID b40695568db84a43b24cffb4c32583a759d96b09
> # Parent  01bdccfeb9d98ae85388d06c9c694b946f346edf
> cmdutil: show diffs in commit message with ui.verbosecommit option

I know people have been asking for this, but I've never really
understood why: I simply run 'hg diff' in another terminal or take a
peek in TortoiseHg. If the diff is substantial, having it dumped into
the commit message buffer wont help me much -- I will want at least to
have color highlighting and probably a real diff tool to see what's
going on.

Also, the recommended way to do this today is to set your editor to a
script which will run 'hg diff' for you before starting your real
editor.

> The following adds an option, ui.verbosecommit, which displays the
> entire diff about to be committed in the text editor window,

If this goes in, then I suggest using a more descriptive name than
ui.verbosecommit. Maybe ui.commitmsgdiff so that the scope of the
setting is more well-defined.

> prefixed with "HG:". This is useful as a final reminder of what's
> about to be committed while writing the commit message.
>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -1864,6 +1864,12 @@
>      edittext.extend([_("HG: added %s") % f for f in added])
>      edittext.extend([_("HG: changed %s") % f for f in modified])
>      edittext.extend([_("HG: removed %s") % f for f in removed])
> +    if repo.ui.config("ui","verbosecommit"):
> +        edittext.append("HG: ")
> +        diff = ctx.diff()
> +        for f in diff:
> +            for l in f.split("\n"):
> +                edittext.extend([_("HG: %s") % l])

You should probably either use

            for l in f.split("\n"):
                edittext.append(_("HG: %s") % l)

or

            edittext.extend([_("HG: %s") % l for l in f.split("\n")])

That is, either append single lines or extend the list with all lines in
one go.
Jordi Gutiérrez Hermoso - Jan. 16, 2014, 2:21 a.m.
On Sat, 2014-01-11 at 15:20 -0600, Martin Geisler wrote:
> Jordi Gutiérrez Hermoso <jordigh@octave.org> writes:
> 
> > # HG changeset patch
> > # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
> > # Date 1373469596 14400
> > #      Wed Jul 10 11:19:56 2013 -0400
> > # Node ID b40695568db84a43b24cffb4c32583a759d96b09
> > # Parent  01bdccfeb9d98ae85388d06c9c694b946f346edf
> > cmdutil: show diffs in commit message with ui.verbosecommit option
> 
> I know people have been asking for this, but I've never really
> understood why: I simply run 'hg diff' in another terminal or take a
> peek in TortoiseHg.

It would be convenient to be able to look at the diff as one is
writing a commit message about it. Indeed, since lines beginning with
HG: are ignored, it is even possible to even annotate the diff inline
in the commit message. It seems useful to me.

Another use case is as a final reminder of what's about to be
committed if using hg (c)record. In that case, you want to see what
you're about to commit, "hg diff" or thg will be no help. I thought
this patch helped with the case of (c)record, but testing it again, I
see that it doesn't. I'll fix this.

> > The following adds an option, ui.verbosecommit, which displays the
> > entire diff about to be committed in the text editor window,
> 
> If this goes in, then I suggest using a more descriptive name than
> ui.verbosecommit. Maybe ui.commitmsgdiff so that the scope of the
> setting is more well-defined.

Originally I was hoping for other kinds of output besides the diff,
hence the generic name. I have forgotten what those other things were,
so I'll revert to your specific option name.

> > prefixed with "HG:". This is useful as a final reminder of what's
> > about to be committed while writing the commit message.
> >
> > diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> > --- a/mercurial/cmdutil.py
> > +++ b/mercurial/cmdutil.py
> > @@ -1864,6 +1864,12 @@
> >      edittext.extend([_("HG: added %s") % f for f in added])
> >      edittext.extend([_("HG: changed %s") % f for f in modified])
> >      edittext.extend([_("HG: removed %s") % f for f in removed])
> > +    if repo.ui.config("ui","verbosecommit"):
> > +        edittext.append("HG: ")
> > +        diff = ctx.diff()
> > +        for f in diff:
> > +            for l in f.split("\n"):
> > +                edittext.extend([_("HG: %s") % l])
> 
> You should probably either use
> 
>             for l in f.split("\n"):
>                 edittext.append(_("HG: %s") % l)
> 
> or
> 
>             edittext.extend([_("HG: %s") % l for l in f.split("\n")])
> 
> That is, either append single lines or extend the list with all lines in
> one go.

I'll make this change.

- Jordi G. H.
Martin Geisler - Jan. 18, 2014, 3:16 p.m.
Jordi Gutiérrez Hermoso <jordigh@octave.org> writes:

> On Sat, 2014-01-11 at 15:20 -0600, Martin Geisler wrote:
>> Jordi Gutiérrez Hermoso <jordigh@octave.org> writes:
>> 
>> > # HG changeset patch
>> > # User Jordi Gutiérrez Hermoso <jordigh@octave.org>
>> > # Date 1373469596 14400
>> > #      Wed Jul 10 11:19:56 2013 -0400
>> > # Node ID b40695568db84a43b24cffb4c32583a759d96b09
>> > # Parent  01bdccfeb9d98ae85388d06c9c694b946f346edf
>> > cmdutil: show diffs in commit message with ui.verbosecommit option
>> 
>> I know people have been asking for this, but I've never really
>> understood why: I simply run 'hg diff' in another terminal or take a
>> peek in TortoiseHg.
>
> It would be convenient to be able to look at the diff as one is
> writing a commit message about it. Indeed, since lines beginning with
> HG: are ignored, it is even possible to even annotate the diff inline
> in the commit message. It seems useful to me.

What I tried to say is that I think we'll be in one of two situations:

a) the diff is small enough that I can understand it when I read it in
   the commit editor

b) the diff is large and complicated (spanning multiple files with
   larger changes in each file)

In the first case I would prefer to refer back to my (colored!) 'hg
diff' output in the terminal -- in the second case I would need to use a
real diff tool anyway to get a sense of what's going on. That is why I
don't see the utility of adding a ton of 'HG:' prefixed lines to the
commit editor message.

Maybe my logic is wrong for you, maybe you like to run 'hg commit'
without having run 'hg diff' first? That would seem odd to me, but maybe
that's how you work :)

Maybe a better approach would be to work on having Mercurial include
some good wrapper scripts that would inject this information into the
commit message template before opening the normal editor.

That way you don't have to change core Mercurial -- you set ui.editor to
"inject-diff-editor" and you're done. You could even create a
third-party project to developer and distribute such editor wrappers. If
they're installed in the user's PATH, enabling a particular editor
wrapper would be just as easy as setting

  [ui]
  show-me-diffs-in-the-commit-msg-template = yes

Just a thought.

> Another use case is as a final reminder of what's about to be
> committed if using hg (c)record. In that case, you want to see what
> you're about to commit, "hg diff" or thg will be no help. I thought
> this patch helped with the case of (c)record, but testing it again, I
> see that it doesn't. I'll fix this.

So me, that sounds like a better motivation.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1864,6 +1864,12 @@ 
     edittext.extend([_("HG: added %s") % f for f in added])
     edittext.extend([_("HG: changed %s") % f for f in modified])
     edittext.extend([_("HG: removed %s") % f for f in removed])
+    if repo.ui.config("ui","verbosecommit"):
+        edittext.append("HG: ")
+        diff = ctx.diff()
+        for f in diff:
+            for l in f.split("\n"):
+                edittext.extend([_("HG: %s") % l])
     if not added and not modified and not removed:
         edittext.append(_("HG: no files changed"))
     edittext.append("")
diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1253,6 +1253,11 @@ 
 ``verbose``
     Increase the amount of output printed. True or False. Default is False.
 
+``verbosecommit``
+    Put more information prefixed with "HG: " in commit messages when
+    editing them, such as the diff that's about to be committed.
+    Default is False.
+
 
 ``web``
 -------