Patchwork commit: --edit/-e to force edit of otherwise-supplied commit message

login
register
mail settings
Submitter Bradley M. Kuhn
Date Sept. 8, 2013, 11:15 p.m.
Message ID <874n9ukiri.fsf@ebb.org>
Download mbox | patch
Permalink /patch/2414/
State Accepted
Headers show

Comments

Bradley M. Kuhn - Sept. 8, 2013, 11:15 p.m.
# HG changeset patch
# User "Bradley M. Kuhn" <bkuhn@ebb.org>
# Date 1378681328 14400
# Node ID 3d6eba7684266bacc022dfb3f15ce242fd645aa7
# Parent  1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0
commit: --edit/-e to force edit of otherwise-supplied commit message

The --edit/-e option for the 'commit' command forces editor, even when a
commit message has been provided already by other means, such as by the -m or
-l options.
Augie Fackler - Sept. 9, 2013, 3:15 a.m.
On Sun, Sep 08, 2013 at 07:15:13PM -0400, Bradley M. Kuhn wrote:
> # HG changeset patch
> # User "Bradley M. Kuhn" <bkuhn@ebb.org>
> # Date 1378681328 14400
> # Node ID 3d6eba7684266bacc022dfb3f15ce242fd645aa7
> # Parent  1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0
> commit: --edit/-e to force edit of otherwise-supplied commit message

I'm not quite sure I see the use case - is this so you can edit
something after a script specified a commit message?

Also, I'm hesitant to spend the '-e' short flag right away, I guess
that depends on the use case.

>
> The --edit/-e option for the 'commit' command forces editor, even when a
> commit message has been provided already by other means, such as by the -m or
> -l options.
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -1289,6 +1289,8 @@
>       _('mark a branch as closed, hiding it from the branch list')),
>      ('', 'amend', None, _('amend the parent of the working dir')),
>      ('s', 'secret', None, _('use the secret phase for committing')),
> +    ('e', 'edit', None,
> +     _('Further edit commit message already specified')),
>      ] + walkopts + commitopts + commitopts2 + subrepoopts,
>      _('[OPTION]... [FILE]...'))
>  def commit(ui, repo, *pats, **opts):
> @@ -1327,6 +1329,8 @@
>
>      Returns 0 on success, 1 if nothing changed.
>      """
> +    forceeditor = opts.get('force_editor') or opts.get('edit')
> +
>      if opts.get('subrepos'):
>          if opts.get('amend'):
>              raise util.Abort(_('cannot amend with --subrepos'))
> @@ -1365,7 +1369,7 @@
>              raise util.Abort(_('cannot amend changeset with children'))
>
>          e = cmdutil.commiteditor
> -        if opts.get('force_editor'):
> +        if forceeditor:
>              e = cmdutil.commitforceeditor
>
>          def commitfunc(ui, repo, message, match, opts):
> @@ -1405,7 +1409,7 @@
>              newmarks.write()
>      else:
>          e = cmdutil.commiteditor
> -        if opts.get('force_editor'):
> +        if forceeditor:
>              e = cmdutil.commitforceeditor
>
>          def commitfunc(ui, repo, message, match, opts):
> diff --git a/tests/test-commit.t b/tests/test-commit.t
> --- a/tests/test-commit.t
> +++ b/tests/test-commit.t
> @@ -119,7 +119,18 @@
>    $ hg add
>    adding bar/bar (glob)
>    adding foo/foo (glob)
> -  $ hg ci -m commit-subdir-1 foo
> +  $ HGEDITOR=cat hg ci -e -m commit-subdir-1 foo
> +  commit-subdir-1
> +
> +
> +  HG: Enter commit message.  Lines beginning with 'HG:' are removed.
> +  HG: Leave message empty to abort commit.
> +  HG: --
> +  HG: user: test
> +  HG: branch 'default'
> +  HG: added foo/foo
> +
> +
>    $ hg ci -m commit-subdir-2 bar
>
>  subdir log 1
> @@ -173,11 +184,23 @@
>  dot and subdir commit test
>
>    $ hg init test3
> +  $ echo commit-foo-subdir > commit-log-test
>    $ cd test3
>    $ mkdir foo
>    $ echo foo content > foo/plain-file
>    $ hg add foo/plain-file
> -  $ hg ci -m commit-foo-subdir foo
> +  $ HGEDITOR=cat hg ci --edit -l ../commit-log-test foo
> +  commit-foo-subdir
> +
> +
> +  HG: Enter commit message.  Lines beginning with 'HG:' are removed.
> +  HG: Leave message empty to abort commit.
> +  HG: --
> +  HG: user: test
> +  HG: branch 'default'
> +  HG: added foo/plain-file
> +
> +
>    $ echo modified foo content > foo/plain-file
>    $ hg ci -m commit-foo-dot .
>
> diff --git a/tests/test-completion.t b/tests/test-completion.t
> --- a/tests/test-completion.t
> +++ b/tests/test-completion.t
> @@ -199,7 +199,7 @@
>    add: include, exclude, subrepos, dry-run
>    annotate: rev, follow, no-follow, text, user, file, date, number, changeset, line-number, ignore-all-space, ignore-space-change, ignore-blank-lines, include, exclude
>    clone: noupdate, updaterev, rev, branch, pull, uncompressed, ssh, remotecmd, insecure
> -  commit: addremove, close-branch, amend, secret, include, exclude, message, logfile, date, user, subrepos
> +  commit: addremove, close-branch, amend, secret, edit, include, exclude, message, logfile, date, user, subrepos
>    diff: rev, change, text, git, nodates, show-function, reverse, ignore-all-space, ignore-space-change, ignore-blank-lines, unified, stat, include, exclude, subrepos
>    export: output, switch-parent, rev, text, git, nodates
>    forget: include, exclude
> diff --git a/tests/test-qrecord.t b/tests/test-qrecord.t
> --- a/tests/test-qrecord.t
> +++ b/tests/test-qrecord.t
> @@ -62,6 +62,7 @@
>                              list
>        --amend               amend the parent of the working dir
>     -s --secret              use the secret phase for committing
> +   -e --edit                Further edit commit message already specified
>     -I --include PATTERN [+] include names matching the given patterns
>     -X --exclude PATTERN [+] exclude names matching the given patterns
>     -m --message TEXT        use text as commit message
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Idan Kamara - Sept. 9, 2013, 1:44 p.m.
On Mon, Sep 9, 2013 at 6:15 AM, Augie Fackler <raf@durin42.com> wrote:

> On Sun, Sep 08, 2013 at 07:15:13PM -0400, Bradley M. Kuhn wrote:
> > # HG changeset patch
> > # User "Bradley M. Kuhn" <bkuhn@ebb.org>
> > # Date 1378681328 14400
> > # Node ID 3d6eba7684266bacc022dfb3f15ce242fd645aa7
> > # Parent  1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0
> > commit: --edit/-e to force edit of otherwise-supplied commit message
>
> I'm not quite sure I see the use case - is this so you can edit
> something after a script specified a commit message?
>
> Also, I'm hesitant to spend the '-e' short flag right away, I guess
> that depends on the use case.
>

My guess is he got the idea from git, where at least one use case is
something
like: git commit -m "a commit message that suddenly got too long" -e

And it will open the editor with what you already typed.
Bradley M. Kuhn - Sept. 9, 2013, 4:54 p.m.
Augie Fackler wrote at 23:15 (EDT) on Sunday:
>> I'm not quite sure I see the use case - is this so you can edit
>> something after a script specified a commit message?

Basically, yes.  I got the idea to add it when I was working on the
following patch for etckeeper:
  http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=613278#8

etckeeper prepares a commit message automatically when doing a package
manager install, and feeds it via -m and/or -l.

The patch was I implementing allows the interactive user to then edit
that message before the commit completes.

Idan Kamara wrote at 09:44 (EDT):
> My guess is he got the idea from git, where at least one use case is
> something like: git commit -m "a commit message that suddenly got too
> long" -e

Indeed I did.  The main issue is not for command line typing, but
calling from a script where it's pre-prepping most of the commit message
for you, but you'd like to edit it.

>> Also, I'm hesitant to spend the '-e' short flag right away, I guess
>> that depends on the use case.

I have no objection to not supporting the short flag, and just '--edit'
for now (or even something else entirely, as you prefer.

Would you like me to submit a new patch with a different command line
option and/or no-short option at all?
Augie Fackler - Sept. 9, 2013, 4:56 p.m.
On Mon, Sep 9, 2013 at 12:54 PM, Bradley M. Kuhn <bkuhn@ebb.org> wrote:
>
>>> Also, I'm hesitant to spend the '-e' short flag right away, I guess
>>> that depends on the use case.
>
> I have no objection to not supporting the short flag, and just '--edit'
> for now (or even something else entirely, as you prefer.
>
> Would you like me to submit a new patch with a different command line
> option and/or no-short option at all?


I might (at the risk of excessive verbosity) go with --edit-message,
and no short flag for now. If that works for you (and nobody else
violently objects), just send the patch that way and I'll gladly queue
it.

AF
Pierre-Yves David - Sept. 9, 2013, 5:05 p.m.
On 9 sept. 2013, at 05:15, Augie Fackler wrote:

> On Sun, Sep 08, 2013 at 07:15:13PM -0400, Bradley M. Kuhn wrote:
>> # HG changeset patch
>> # User "Bradley M. Kuhn" <bkuhn@ebb.org>
>> # Date 1378681328 14400
>> # Node ID 3d6eba7684266bacc022dfb3f15ce242fd645aa7
>> # Parent  1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0
>> commit: --edit/-e to force edit of otherwise-supplied commit message
> 
> I'm not quite sure I see the use case - is this so you can edit
> something after a script specified a commit message?

I've been thinking for a way to "queue" stuff for future commit message. adding stuff to a file then using -l <f> -e would work.

This could also help on the --amend front to have people explicitly asking for the editor.

However, I'm not trilled about adding such a flag both of my usecase have more elegant solution and I'm not convinced the `-m "omg this is too long" -e` worst it.
Pierre-Yves David - April 13, 2014, 2:38 p.m.
On 09/08/2013 07:15 PM, Bradley M. Kuhn wrote:
> # HG changeset patch
> # User "Bradley M. Kuhn" <bkuhn@ebb.org>
> # Date 1378681328 14400
> # Node ID 3d6eba7684266bacc022dfb3f15ce242fd645aa7
> # Parent  1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0
> commit: --edit/-e to force edit of otherwise-supplied commit message
>
> The --edit/-e option for the 'commit' command forces editor, even when a
> commit message has been provided already by other means, such as by the -m or
> -l options.

This patch seems to got lost in discussion swamps

At this patch credit, multiple other command (tags, graft, (evolve 
amend)) use the same flags (e, edit) for the same purpose.
Bradley M. Kuhn - April 13, 2014, 5:35 p.m.
On 09/08/2013 07:15 PM, Bradley M. Kuhn wrote:
>> # HG changeset patch
>> # User "Bradley M. Kuhn" <bkuhn@ebb.org>
>> # Date 1378681328 14400
>> # Node ID 3d6eba7684266bacc022dfb3f15ce242fd645aa7
>> # Parent  1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0

Pierre-Yves David wrote today:
> This patch seems to got lost in discussion swamps

Indeed, it was stalled because I was going to rework it to use a long
option only (as Auggie suggested on this thread), and not -e.

However, if, in the meantime, -e/--edit has become a standard for
this purpose with other Mercurial commands...
>  multiple other command (tags, graft, (evolve
> amend)) use the same flags (e, edit) for the same purpose.
... perhaps my original patch now stands on its own without further
modification and is thus resonable to be accepted as it stands.

Auggie had also suggested that the long option be --edit-message which
seemed reasonable at the time, but if other commands are using just --edit
then perhaps consistency is better, which means my original patch is
good to go.

   -- bkuhn
Sean Farley - April 14, 2014, 12:37 a.m.
Bradley M. Kuhn <bkuhn@ebb.org> writes:

> On 09/08/2013 07:15 PM, Bradley M. Kuhn wrote:
>>> # HG changeset patch
>>> # User "Bradley M. Kuhn" <bkuhn@ebb.org>
>>> # Date 1378681328 14400
>>> # Node ID 3d6eba7684266bacc022dfb3f15ce242fd645aa7
>>> # Parent  1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0
>
> Pierre-Yves David wrote today:
>> This patch seems to got lost in discussion swamps
>
> Indeed, it was stalled because I was going to rework it to use a long
> option only (as Auggie suggested on this thread), and not -e.
>
> However, if, in the meantime, -e/--edit has become a standard for
> this purpose with other Mercurial commands...
>>  multiple other command (tags, graft, (evolve
>> amend)) use the same flags (e, edit) for the same purpose.
> ... perhaps my original patch now stands on its own without further
> modification and is thus resonable to be accepted as it stands.
>
> Auggie had also suggested that the long option be --edit-message which
> seemed reasonable at the time, but if other commands are using just --edit
> then perhaps consistency is better, which means my original patch is
> good to go.

I've been using this patch as is and meant to reply that -e/--edit are
in line with other commands so I would prefer to keep it as such.

Side note: I can't live without this patch now. So convenient.
Augie Fackler - April 14, 2014, 2:32 a.m.
On Apr 13, 2014, at 1:35 PM, Bradley M. Kuhn <bkuhn@ebb.org> wrote:

> On 09/08/2013 07:15 PM, Bradley M. Kuhn wrote:
>>> # HG changeset patch
>>> # User "Bradley M. Kuhn" <bkuhn@ebb.org>
>>> # Date 1378681328 14400
>>> # Node ID 3d6eba7684266bacc022dfb3f15ce242fd645aa7
>>> # Parent  1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0
> 
> Pierre-Yves David wrote today:
>> This patch seems to got lost in discussion swamps
> 
> Indeed, it was stalled because I was going to rework it to use a long
> option only (as Auggie suggested on this thread), and not -e.
> 
> However, if, in the meantime, -e/--edit has become a standard for
> this purpose with other Mercurial commands...
>> multiple other command (tags, graft, (evolve
>> amend)) use the same flags (e, edit) for the same purpose.
> ... perhaps my original patch now stands on its own without further
> modification and is thus resonable to be accepted as it stands.
> 
> Auggie had also suggested that the long option be --edit-message which
> seemed reasonable at the time, but if other commands are using just --edit
> then perhaps consistency is better, which means my original patch is
> good to go.

I concur - since we've gone ahead and spent -e in graft et al, let's go ahead and just take this one.

I've queued this (from an airplane, so it may not get pushed when this email goes out ;) )

> 
>   -- bkuhn
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - April 14, 2014, 4:29 a.m.
On 04/13/2014 10:32 PM, Augie Fackler wrote:
>
> On Apr 13, 2014, at 1:35 PM, Bradley M. Kuhn <bkuhn@ebb.org> wrote:
>
>> On 09/08/2013 07:15 PM, Bradley M. Kuhn wrote:
>>>> # HG changeset patch
>>>> # User "Bradley M. Kuhn" <bkuhn@ebb.org>
>>>> # Date 1378681328 14400
>>>> # Node ID 3d6eba7684266bacc022dfb3f15ce242fd645aa7
>>>> # Parent  1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0
>>
>> Pierre-Yves David wrote today:
>>> This patch seems to got lost in discussion swamps
>>
>> Indeed, it was stalled because I was going to rework it to use a long
>> option only (as Auggie suggested on this thread), and not -e.
>>
>> However, if, in the meantime, -e/--edit has become a standard for
>> this purpose with other Mercurial commands...
>>> multiple other command (tags, graft, (evolve
>>> amend)) use the same flags (e, edit) for the same purpose.
>> ... perhaps my original patch now stands on its own without further
>> modification and is thus resonable to be accepted as it stands.
>>
>> Auggie had also suggested that the long option be --edit-message which
>> seemed reasonable at the time, but if other commands are using just --edit
>> then perhaps consistency is better, which means my original patch is
>> good to go.
>
> I concur - since we've gone ahead and spent -e in graft et al, let's go ahead and just take this one.
>
> I've queued this (from an airplane, so it may not get pushed when this email goes out ;) )

I'm currently running the test on it, I'll probably race you at pushing 
it ;-)
Pierre-Yves David - April 14, 2014, 4:33 a.m.
On 09/08/2013 07:15 PM, Bradley M. Kuhn wrote:
> # HG changeset patch
> # User "Bradley M. Kuhn" <bkuhn@ebb.org>
> # Date 1378681328 14400
> # Node ID 3d6eba7684266bacc022dfb3f15ce242fd645aa7
> # Parent  1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0
> commit: --edit/-e to force edit of otherwise-supplied commit message

I'm please to announce that this patch have been successfully 
clowncopterized. Thanks!

I fixed the capitalization of the short help.
Bradley M. Kuhn - April 16, 2014, 12:56 a.m.
> On 09/08/2013 07:15 PM, Bradley M. Kuhn wrote:
> ># HG changeset patch
> ># User "Bradley M. Kuhn" <bkuhn@ebb.org>
> ># Date 1378681328 14400
> ># Node ID 3d6eba7684266bacc022dfb3f15ce242fd645aa7
> ># Parent  1d07bf106c2ad1c7ef5e257e754ca8d858bd04b0
> >commit: --edit/-e to force edit of otherwise-supplied commit message

Pierre-Yves David wrote:
> I'm please to announce that this patch have been successfully
> clowncopterized. Thanks!

Thanks all, I know it's a small one but I'm very proud to now have my name as
on a changeset that found its way into Mercurial (albeit a rather trivial
change ;).

   -- bkuhn

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -1289,6 +1289,8 @@ 
      _('mark a branch as closed, hiding it from the branch list')),
     ('', 'amend', None, _('amend the parent of the working dir')),
     ('s', 'secret', None, _('use the secret phase for committing')),
+    ('e', 'edit', None,
+     _('Further edit commit message already specified')),
     ] + walkopts + commitopts + commitopts2 + subrepoopts,
     _('[OPTION]... [FILE]...'))
 def commit(ui, repo, *pats, **opts):
@@ -1327,6 +1329,8 @@ 
 
     Returns 0 on success, 1 if nothing changed.
     """
+    forceeditor = opts.get('force_editor') or opts.get('edit')
+
     if opts.get('subrepos'):
         if opts.get('amend'):
             raise util.Abort(_('cannot amend with --subrepos'))
@@ -1365,7 +1369,7 @@ 
             raise util.Abort(_('cannot amend changeset with children'))
 
         e = cmdutil.commiteditor
-        if opts.get('force_editor'):
+        if forceeditor:
             e = cmdutil.commitforceeditor
 
         def commitfunc(ui, repo, message, match, opts):
@@ -1405,7 +1409,7 @@ 
             newmarks.write()
     else:
         e = cmdutil.commiteditor
-        if opts.get('force_editor'):
+        if forceeditor:
             e = cmdutil.commitforceeditor
 
         def commitfunc(ui, repo, message, match, opts):
diff --git a/tests/test-commit.t b/tests/test-commit.t
--- a/tests/test-commit.t
+++ b/tests/test-commit.t
@@ -119,7 +119,18 @@ 
   $ hg add
   adding bar/bar (glob)
   adding foo/foo (glob)
-  $ hg ci -m commit-subdir-1 foo
+  $ HGEDITOR=cat hg ci -e -m commit-subdir-1 foo
+  commit-subdir-1
+  
+  
+  HG: Enter commit message.  Lines beginning with 'HG:' are removed.
+  HG: Leave message empty to abort commit.
+  HG: --
+  HG: user: test
+  HG: branch 'default'
+  HG: added foo/foo
+
+
   $ hg ci -m commit-subdir-2 bar
 
 subdir log 1
@@ -173,11 +184,23 @@ 
 dot and subdir commit test
 
   $ hg init test3
+  $ echo commit-foo-subdir > commit-log-test
   $ cd test3
   $ mkdir foo
   $ echo foo content > foo/plain-file
   $ hg add foo/plain-file
-  $ hg ci -m commit-foo-subdir foo
+  $ HGEDITOR=cat hg ci --edit -l ../commit-log-test foo
+  commit-foo-subdir
+  
+  
+  HG: Enter commit message.  Lines beginning with 'HG:' are removed.
+  HG: Leave message empty to abort commit.
+  HG: --
+  HG: user: test
+  HG: branch 'default'
+  HG: added foo/plain-file
+
+
   $ echo modified foo content > foo/plain-file
   $ hg ci -m commit-foo-dot .
 
diff --git a/tests/test-completion.t b/tests/test-completion.t
--- a/tests/test-completion.t
+++ b/tests/test-completion.t
@@ -199,7 +199,7 @@ 
   add: include, exclude, subrepos, dry-run
   annotate: rev, follow, no-follow, text, user, file, date, number, changeset, line-number, ignore-all-space, ignore-space-change, ignore-blank-lines, include, exclude
   clone: noupdate, updaterev, rev, branch, pull, uncompressed, ssh, remotecmd, insecure
-  commit: addremove, close-branch, amend, secret, include, exclude, message, logfile, date, user, subrepos
+  commit: addremove, close-branch, amend, secret, edit, include, exclude, message, logfile, date, user, subrepos
   diff: rev, change, text, git, nodates, show-function, reverse, ignore-all-space, ignore-space-change, ignore-blank-lines, unified, stat, include, exclude, subrepos
   export: output, switch-parent, rev, text, git, nodates
   forget: include, exclude
diff --git a/tests/test-qrecord.t b/tests/test-qrecord.t
--- a/tests/test-qrecord.t
+++ b/tests/test-qrecord.t
@@ -62,6 +62,7 @@ 
                             list
       --amend               amend the parent of the working dir
    -s --secret              use the secret phase for committing
+   -e --edit                Further edit commit message already specified
    -I --include PATTERN [+] include names matching the given patterns
    -X --exclude PATTERN [+] exclude names matching the given patterns
    -m --message TEXT        use text as commit message