[PATCH 1 of 2] crecord: repurpose "a" key to toggle all selections (BC)

Denis Laxalde denis at laxalde.org
Wed Dec 11 04:41:53 EST 2019


Jordi Gutiérrez Hermoso a écrit :
> # HG changeset patch
> # User Jordi Gutiérrez Hermoso <jordigh at octave.org>
> # Date 1576015329 18000
> #      Tue Dec 10 17:02:09 2019 -0500
> # Node ID bd10bc4bdb994b875040c0c58f1c341df66ddd46
> # Parent  ce088b38f92b6d480df2072dc45fb068443758dd
> crecord: repurpose "a" key to toggle all selections (BC)
> 
> I really don't like "a". I keep accidentally hitting it when I
> actually want "A", and then I'm suddenly in a state I don't want to be
> in. There's a big wall of text telling me that I've turned amend mode
> on or off (which one was I orginally in?), and this seems very
> useless.

Sounds reasonable to me. Also this 'amend mode' seems only useful for
'commit --interactive'; but it's currently also available for other
interactive commands (e.g. 'revert --interactive', which makes no
sense) so it's already weird.

> If I wanted to amend or not, I would have chosen that from the
> command-line, not change my mind after I've already started picking
> hunks apart.

Agreed as well on this. IMHO, interactive diff hunks selection module
should only be concerned about selecting hunks.

> It seems much better to repurpose this key to be a "weaker" version of
> "A". It toggles all selections. This is pretty harmless if hit
> accidentally, (can just hit "a" again to undo it), and has immediate
> visual feedback that something happened: all the x's and blank spaces
> get switched around. And unlike with amend, the current flipped state
> is also immediately visible without having to read a wall of text.

Does "pretty harmless" mean that you can actually toggle back to the
previous selection? (If not, I wouldn't say this is harmless since the
user may have spend a fair amount of time carefully selecting hunks.)

Also, this breaks tests in test-commit-interactive-curses.t (and it'd be
nice to have some new test for the new feature).

> I'm calling this a BC, however, because somewhere, someone out there
> has probably really fallen in love with the old use of "a" and will
> get angry that we took it away.
> 
> diff --git a/mercurial/crecord.py b/mercurial/crecord.py
> --- a/mercurial/crecord.py
> +++ b/mercurial/crecord.py
> @@ -990,6 +990,13 @@ class curseschunkselector(object):
>                      self.toggleapply(item)
>          self.waslasttoggleallapplied = not self.waslasttoggleallapplied
>  
> +    def flipselections(self):
> +        b"flip all selections"

Triple " without 'b' would be nicer.

> +        for header in self.headerlist:
> +            for hunk in header.allchildren():
> +                for line in hunk.allchildren():
> +                    self.toggleapply(line)
> +
>      def toggleallbetween(self):
>          """toggle applied on or off for all items in range [lastapplied,
>          current]. """
> @@ -1637,7 +1644,7 @@ the following are valid keystrokes:
>                   ctrl-l : scroll the selected line to the top of the screen
>                        m : edit / resume editing the commit message
>                        e : edit the currently selected hunk
> -                      a : toggle amend mode, only with commit -i
> +                      a : toggle all selections
>                        c : confirm selected changes
>                        r : review/edit and confirm selected changes
>                        q : quit without confirming (no changes will be made)
> @@ -1913,7 +1920,7 @@ are you sure you want to review/edit and
>          elif keypressed in ["q"]:
>              raise error.Abort(_(b'user quit'))
>          elif keypressed in ['a']:
> -            self.toggleamend(self.opts, test)
> +            self.flipselections()
>          elif keypressed in ["c"]:
>              return True
>          elif keypressed in ["r"]:


More information about the Mercurial-devel mailing list