git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: David Aguilar <davvid@gmail.com>
To: Pratyush Yadav <me@yadavpratyush.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
	Johannes Sixt <j6t@kdbg.org>,
	Bert Wesarg <bert.wesarg@googlemail.com>
Subject: Re: [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines
Date: Fri, 23 Aug 2019 16:43:39 -0700	[thread overview]
Message-ID: <20190823234339.GA17088@gmail.com> (raw)
In-Reply-To: <20190822220107.4153-1-me@yadavpratyush.com>

On Fri, Aug 23, 2019 at 03:31:03AM +0530, Pratyush Yadav wrote:
> Hi,
> 
> This series adds the ability to revert selected lines and hunks in
> git-gui. Partially based on the patch by Bert Wesarg [0].
> 
> The commits can be found in the topic branch 'py/revert-hunks-lines'
> at https://github.com/prati0100/git-gui/tree/py/revert-hunks-lines
> 
> Once reviewed, pull the commits from
> 415ce3f8582769d1d454b3796dc6c9c847cefa87 till
> 0a1f4ea92b97e673fda40918dae68deead43bb27, or just munge the patches and
> apply them locally, whichever you prefer.
> 
> Changes in v2:
> - Add an option to disable the revert confirmation prompt as suggested
>   by Johannes Sixt.
> - Base the patches on Pat's git-gui tree instead of git.git.


We've had these features for years in git-cola.

Please copy our keyboard shortcuts.
IMO we should not re-invent the user interactions.

http://git-cola.github.io/share/doc/git-cola/hotkeys.html

Ctrl-u is our revert-unstaged-edits hotkeys.  "s" is for
staging/unstaging (or Ctrl-s if the file list is focused).

The same hotkey is used for operating at the line level.
If no lines are selected, the hunk surrounding the current cursor
position is used.

Please make keyboard interaction a first-class design consideration.


I have a very strong opinion about the confirmation dialog, so I'll just
mention that here since Hannes is on this thread.

In cola we do have a confirmation dialog, and I strongly believe this is
the correct behavior because it's an operation that drops data that
cannot be recovered.

In the other thread, it was mentioned that this dialog would be a
nuisance.  Perhaps that is true -- for the dialog that may have been
implemented in this series (I haven't run it to verify).

Let's dive into that concern.

In git-cola we have a confirmation dialog and it is by no way a
detriment to the workflow, and I use that feature all the time.
Why?  The reason is that we focused on the keyboard interaction.

The workflow is as follows:

	Ctrl-u to initiate the revert action
	The prompt appears immediately.
		- Hitting any of "enter", "y", or "spacebar" will
		  confirm the confirmation, and proceed.
		- Hitting any of "escape" or "n" will cancel the action.

So essentially the workflow for the power user becomes "ctrl-u, enter"
and that is such a tiny overhead that it really is not a bother at all.

On the other hand, if I had to actually move my hand over to a mouse or
trackpad and actually "click" on something then I would be super
annoyed.  That would be simply horrible with RSI in mind.

OTOH having to hit "enter" or "spacebar" (which is the largest key on
your keyboard, and your thumbs have good hefty muscles) is totally
acceptable in my book because it strikes the right balance between
safety for a destructive operation and convenience.

Now, let's consider the alternative -- adding an option to disable the
prompt.  I don't like that.

Why?  It's yet another option.  It's yet another thing to document, yet
another code path, and yet another pitfall for a user who might run
git-gui in a different configuration (and becomes surprised when revert
doesn't prompt and suddenly loses their work).

Do we really need an option, or do we need better usability instead?
My opinion is that the latter is the real need.


That's my $.02 from having used this feature in practice since 2013.
-- 
David

  parent reply	other threads:[~2019-08-23 23:43 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-19 21:41 [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav
2019-08-19 21:41 ` [PATCH 1/3] git-gui: Move revert confirmation dialog creation to separate function Pratyush Yadav
2019-08-19 21:41 ` [PATCH 2/3] git-gui: Add the ability to revert selected lines Pratyush Yadav
2019-08-20 19:21   ` Johannes Sixt
2019-08-20 19:29     ` Pratyush Yadav
2019-08-20 21:19       ` Johannes Sixt
2019-08-21 21:48         ` Pratyush Yadav
2019-08-23 13:01           ` Johannes Schindelin
2019-08-23 16:28             ` Junio C Hamano
2019-08-23 17:03               ` Pratyush Yadav
2019-08-23 19:17                 ` Johannes Sixt
2019-08-19 21:41 ` [PATCH 3/3] git-gui: Add the ability to revert selected hunk Pratyush Yadav
2019-08-20 18:47 ` [PATCH 0/3] git-gui: Add ability to revert selected hunks and lines Junio C Hamano
2019-08-20 19:49   ` Pratyush Yadav
2019-08-21  7:06 ` Bert Wesarg
2019-08-21 21:30   ` Pratyush Yadav
2019-08-22 22:01 ` [PATCH v2 0/4] " Pratyush Yadav
2019-08-22 22:01   ` [PATCH v2 1/4] git-gui: Move revert confirmation dialog creation to separate function Pratyush Yadav
2019-08-22 22:01   ` [PATCH v2 2/4] git-gui: Add option to disable the revert confirmation prompt Pratyush Yadav
2019-08-22 22:01   ` [PATCH v2 3/4] git-gui: Add the ability to revert selected lines Pratyush Yadav
2019-08-23  6:29     ` Bert Wesarg
2019-08-23 16:51       ` Pratyush Yadav
2019-08-22 22:01   ` [PATCH v2 4/4] git-gui: Add the ability to revert selected hunk Pratyush Yadav
2019-08-22 22:34   ` [PATCH v2 0/4] git-gui: Add ability to revert selected hunks and lines Junio C Hamano
2019-08-22 22:51     ` Pratyush Yadav
2019-08-23  6:04       ` Bert Wesarg
2019-08-23 16:04         ` Junio C Hamano
2019-08-23 16:44         ` Pratyush Yadav
     [not found]           ` <CAKPyHN0QbCDzcG8=zPP4-WHKqMk3R0sJ0BjXDR=zak3OfEa2bg@mail.gmail.com>
2019-08-25 11:57             ` Pratyush Yadav
2019-08-23 23:43   ` David Aguilar [this message]
2019-08-24  6:54     ` Bert Wesarg
2019-08-24  6:57     ` Bert Wesarg
2019-08-24  7:26       ` David Aguilar
2019-08-24  8:09       ` Johannes Sixt
2019-08-28 21:57 ` [PATCH v3 " Pratyush Yadav
2019-08-28 21:57   ` [PATCH v3 1/4] git-gui: allow reverting selected lines Pratyush Yadav
2019-08-28 21:57   ` [PATCH v3 2/4] git-gui: allow reverting selected hunk Pratyush Yadav
2019-08-28 21:57   ` [PATCH v3 3/4] git-gui: return early when patch fails to apply Pratyush Yadav
2019-08-28 21:57   ` [PATCH v3 4/4] git-gui: allow undoing last revert Pratyush Yadav
2019-10-21  9:16     ` Bert Wesarg
2019-10-21 19:04       ` Pratyush Yadav
2019-10-22  8:17         ` Bert Wesarg
2019-10-23 18:57           ` Pratyush Yadav
2019-10-21 19:35       ` Johannes Sixt
2019-10-22  7:46         ` Bert Wesarg
2019-10-23 19:00           ` Pratyush Yadav
2019-09-10 19:21   ` [PATCH v3 0/4] git-gui: Add ability to revert selected hunks and lines Pratyush Yadav
2019-09-10 20:26     ` Johannes Sixt
2019-09-12 18:59       ` Bert Wesarg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190823234339.GA17088@gmail.com \
    --to=davvid@gmail.com \
    --cc=bert.wesarg@googlemail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=j6t@kdbg.org \
    --cc=me@yadavpratyush.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).