Re: [PATCH] histedit: make actions toggleables
On Sun, 2019-05-12 at 20:17 -0700, Feng Yu wrote: > To clarify, the change I propose is in the ncurses based _chistedit UI, which > does display the graph. > It does not change the behaviour of TUI version of _histedit. "TUI" and "curses" mean the same thing, generally. Maybe we should call the other histedit interface editor-based? > I will look into adding a config option for this and file a new > patch. Wait, no, not more configs. That seems like unnecessary complication. Let's come to a consensus first. I'm not sure which way makes most sense. Both ways seem to make sense to me. Do you have examples of users who were confused by the lack of toggleability? ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: make actions toggleables
Hi Anton, To clarify, the change I propose is in the ncurses based _chistedit UI, which does display the graph. It does not change the behaviour of TUI version of _histedit. The behaviour change is to consistently redefine the keypress actions from set-a-rule to toggle-a-rule. I can imagine in scenarios, one (even myself) may smash the keyboard repeatedly in order to set the rule list, so I do agree with you the definition of 'responsive' is subjective in this situation. I will look into adding a config option for this and file a new patch. Thanks! On Sat, May 11, 2019 at 1:56 AM Anton Shestakov wrote: > On Fri, 10 May 2019 10:15:31 -0700 > Yu Feng wrote: > > > # HG changeset patch > > # User f...@google.com > > # Date 1557508115 25200 > > # Fri May 10 10:08:35 2019 -0700 > > # Node ID 23bc04dc2d149829133db571f6a922e95843c9f9 > > # Parent 458dc948aff9f1217718b7679f890fea510d54f7 > > histedit: make actions toggleables. > > > > If the same action is applied twice, revert to pick. > > > > For example, the current state > > #0 pick 104:x Collect progress > > > > pressing e once > > #0 edit 104:x Collect progress > > > > pressing e again toggles it back to pick. > > Instead of keeping at e (behavior before this patch) > > #0 pick 104:x Collect progress > > > > The rationale is that giving a response when a key is pressed > > makes happy users. Toggling seems to be a reasonable response in > > this scenario. > > This rationale may be good for GUI, but text-based interfaces don't > traditionally react to every possible key press. And one way to see > this patch is that it makes histedit do pretty much the opposite to what > it's told to do: I press e and it goes back to "pick". That's > surprising because action field is not a toggle, it's a choice. > > Also, users don't have to change histedit plan in one go, they can > leave it waiting for input for days on end, or open another window to > see the graph and mentally think up the list of actions to mechanically > punch it in on the keyboard without looking at the current state of > histedit plan. If they know that commits A+B+C need to be edited, it > makes sense to let them press (e, down)x3 on A without making sure that > any of the actions wasn't already on "edit" because of their previous > input. > > This could be a config option, that would make people knowingly opt-in > and not be surprised by the behavior. > ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Re: [PATCH] histedit: make actions toggleables
On Fri, 10 May 2019 10:15:31 -0700 Yu Feng wrote: > # HG changeset patch > # User f...@google.com > # Date 1557508115 25200 > # Fri May 10 10:08:35 2019 -0700 > # Node ID 23bc04dc2d149829133db571f6a922e95843c9f9 > # Parent 458dc948aff9f1217718b7679f890fea510d54f7 > histedit: make actions toggleables. > > If the same action is applied twice, revert to pick. > > For example, the current state > #0 pick 104:x Collect progress > > pressing e once > #0 edit 104:x Collect progress > > pressing e again toggles it back to pick. > Instead of keeping at e (behavior before this patch) > #0 pick 104:x Collect progress > > The rationale is that giving a response when a key is pressed > makes happy users. Toggling seems to be a reasonable response in > this scenario. This rationale may be good for GUI, but text-based interfaces don't traditionally react to every possible key press. And one way to see this patch is that it makes histedit do pretty much the opposite to what it's told to do: I press e and it goes back to "pick". That's surprising because action field is not a toggle, it's a choice. Also, users don't have to change histedit plan in one go, they can leave it waiting for input for days on end, or open another window to see the graph and mentally think up the list of actions to mechanically punch it in on the keyboard without looking at the current state of histedit plan. If they know that commits A+B+C need to be edited, it makes sense to let them press (e, down)x3 on A without making sure that any of the actions wasn't already on "edit" because of their previous input. This could be a config option, that would make people knowingly opt-in and not be surprised by the behavior. ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
[PATCH] histedit: make actions toggleables
# HG changeset patch # User f...@google.com # Date 1557508115 25200 # Fri May 10 10:08:35 2019 -0700 # Node ID 23bc04dc2d149829133db571f6a922e95843c9f9 # Parent 458dc948aff9f1217718b7679f890fea510d54f7 histedit: make actions toggleables. If the same action is applied twice, revert to pick. For example, the current state #0 pick 104:x Collect progress pressing e once #0 edit 104:x Collect progress pressing e again toggles it back to pick. Instead of keeping at e (behavior before this patch) #0 pick 104:x Collect progress The rationale is that giving a response when a key is pressed makes happy users. Toggling seems to be a reasonable response in this scenario. diff --git a/hgext/histedit.py b/hgext/histedit.py --- a/hgext/histedit.py +++ b/hgext/histedit.py @@ -,7 +,10 @@ def changeaction(state, pos, action): """Change the action state on the given position to the new action""" rules = state['rules'] assert 0 <= pos < len(rules) -rules[pos].action = action +if rules[pos].action != action: +rules[pos].action = action +else: # applying the same action twice reverts to the default +rules[pos].action = KEY_LIST[0] def cycleaction(state, pos, next=False): """Changes the action state the next or the previous action from ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
