From: Ævar Arnfjörð Bjarmason <email@example.com> To: Phillip Wood <firstname.lastname@example.org> Cc: Git Mailing List <email@example.com>, Igor Djordjevic <firstname.lastname@example.org>, Junio C Hamano <email@example.com> Subject: Re: [RFC PATCH v5 0/4] add -p: select individual hunk lines Date: Sat, 28 Jul 2018 14:40:59 +0200 Message-ID: <firstname.lastname@example.org> (raw) In-Reply-To: <email@example.com> On Thu, Jul 26 2018, Phillip Wood wrote: > Unfortuantely v4 had test failures due to a suprious brace from a last > minute edit to a comment that I forgot to test. This version fixes > that, my applogies for the patch churn. > > I've updated this series based on Ævar's feedback on v3 (to paraphrase > stop using '$_' so much and fix staging modified lines.). The first > patch is functionally equivalent to the previous version but with a > reworked implementation. Patch 2 is new, it implements correctly > staging modified lines with a couple of limitations - see the commit > message for more details, I'm keen to get some feedback on it. Patches > 3 and 4 are essentially rebased and tweaked versions of patches 2 and > 3 from the previous version. > > This series is based on pw/add-p-recount (f4d35a6b49 "add -p: fix > counting empty context lines in edited patches") > > The motivation for this series is summed up in the first commit > message: > > "When I end up editing hunks it is almost always because I want to > stage a subset of the lines in the hunk. Doing this by editing the > hunk is inconvenient and error prone (especially so if the patch is > going to be reversed before being applied). Instead offer an option > for add -p to stage individual lines. When the user presses 'l' the > hunk is redrawn with labels by the insertions and deletions and they > are prompted to enter a list of the lines they wish to stage. Ranges > of lines may be specified using 'a-b' where either 'a' or 'b' may be > omitted to mean all lines from 'a' to the end of the hunk or all lines > from 1 upto and including 'b'." I tested this with an eye towards what I pointed out in https://firstname.lastname@example.org/ Using the same workflow (search for "So what I was expecting" in that E-Mail) this now does the right thing in that example: select lines? 4,10 [...] $ git diff --staged -U1 diff --git a/README.md b/README.md index ff990622a3..6d16f7e52b 100644 --- a/README.md +++ b/README.md @@ -20,3 +20,3 @@ See [Documentation/gittutorial.txt] to get started, then see Documentation/git-<commandname>.txt for documentation of each command. -If git has been correctly installed, then the tutorial can also be +If Git has been correctly installed, then the tutorial can also be read with `man gittutorial` or `git help tutorial`, and the u git ((49703a4754...) $) $ Some other comments on this: 1) It needs to be more obvious how to exit this sub-mode, i.e. consider this confused user: Stage this hunk [y,n,q,a,d,/,j,J,g,s,e,l,?]? l select lines? ? invalid hunk line '?' select lines? q invalid hunk line 'q' select lines? exit invalid hunk line 'exit' select lines? quit invalid hunk line 'quit' select lines? :wq invalid hunk line ':wq' select lines? help invalid hunk line 'help' Just doing Ctrl+D or RET exits it. Instead "?" should print some help related to this sub-mode showing what the syntax is, and how to exit the sub-mode. I think it would make sense for "q" to by synonymous with "RET", i.e. you'd need "q<RET>q<RET>" to fully exit, but I don't know... 2) I think it's confusing UI that selecting some of the lines won't re-present the hunk to you again in line mode, but I see this is consistent with how e.g. "e" works, it won't re-present the hunk to you if there's still something to do, you need to exit and run "git add -p" again. I think it makes sense to change that and you'd either "e" or "l" and then "n" to proceed, or continue, but that's per-se unrelated to this feature. Just something I ran into... 3) I don't see any way around this, but we need to carefully explain that selecting a list of things in one session is *not* the same thing as selecting them incrementally in multiple sessions. I.e. consider this diff: @@ -1,3 +1,3 @@ -a -b -c +1 +2 +3 If I select 1,4 I get, as expected: @@ -1,3 +1,3 @@ -a +1 b c And then in the next session: @@ -1,3 +1,3 @@ 1 1 -b 2 -c 3 +2 4 +3 select lines? 1,3 Yields, as expected: @@ -1,3 +1,3 @@ -a -b +1 +2 c But this is not the same as redoing the whole thing as: select lines? 1,4 select lines? 1 select lines? 3 Which instead yields: @@ -1,3 +1,3 @@ -a -b +1 c +3 Now, rummaging through my wetware and that E-Mail from back in March I don't see how it could work differently, and you *also* want to be able to select one line at a time like that. Just something that's not very intuative / hard to explain, and maybe there should be a different syntax (e.g. 1:4) for this "swap 1 for 4" operation, as opposed to selecting lines 1 and 4 as they appear in the diff. 4) With that abc 123 diff noted above, why am I in two sessions allowed to do: @@ -1,3 +1,3 @@ 1 -a 2 -b 3 -c 4 +1 5 +2 6 +3 select lines? 1,4 select lines? 1,4 To end up with this staged: @@ -1,3 +1,3 @@ -a -b +1 +3 c But not allowed to do the same thing in one operation via: @@ -1,3 +1,3 @@ 1 -a 2 -b 3 -c 4 +1 5 +2 6 +3 select lines? 1,4,2,6 unable to pair up insertions and deletions But I am allowed to do e.g.: select lines? 1,4,2,5 To end up with: @@ -1,3 +1,3 @@ -a -b +1 +2 c I can do this in two steps.
next prev parent reply index Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-02-19 11:36 [PATCH v1 0/3] " Phillip Wood 2018-02-19 11:36 ` [PATCH v1 1/3] " Phillip Wood 2018-02-19 11:36 ` [PATCH v1 2/3] add -p: allow line selection to be inverted Phillip Wood 2018-02-19 11:36 ` [PATCH v1 3/3] add -p: optimize line selection for short hunks Phillip Wood 2018-02-19 12:20 ` [PATCH v1 0/3] add -p: select individual hunk lines Gustavo Leite 2018-03-06 10:17 ` [PATCH v2 " Phillip Wood 2018-03-06 10:17 ` [PATCH v2 1/3] " Phillip Wood 2018-03-06 20:29 ` Igor Djordjevic 2018-03-06 21:33 ` Igor Djordjevic 2018-03-06 10:17 ` [PATCH v2 2/3] add -p: allow line selection to be inverted Phillip Wood 2018-03-06 19:57 ` Junio C Hamano 2018-03-08 11:05 ` Phillip Wood 2018-03-08 17:53 ` Junio C Hamano 2018-03-13 12:06 ` Phillip Wood 2018-03-13 16:32 ` Junio C Hamano 2018-03-14 11:02 ` Phillip Wood 2018-03-06 20:41 ` Igor Djordjevic 2018-03-06 10:17 ` [PATCH v2 3/3] add -p: optimize line selection for short hunks Phillip Wood 2018-03-06 20:33 ` Igor Djordjevic 2018-03-06 20:19 ` [PATCH v2 0/3] add -p: select individual hunk lines Igor Djordjevic 2018-03-06 21:03 ` Junio C Hamano 2018-03-06 21:20 ` Igor Djordjevic 2018-03-16 10:13 ` [PATCH v3 " Phillip Wood 2018-03-16 10:13 ` [PATCH v3 1/3] " Phillip Wood 2018-03-16 10:13 ` [PATCH v3 2/3] add -p: allow line selection to be inverted Phillip Wood 2018-03-16 10:13 ` [PATCH v3 3/3] add -p: optimize line selection for short hunks Phillip Wood 2018-03-29 18:32 ` [PATCH v3 0/3] add -p: select individual hunk lines Junio C Hamano 2018-03-30 11:09 ` Phillip Wood 2018-03-31 19:20 ` Ævar Arnfjörð Bjarmason 2018-04-02 10:55 ` Phillip Wood 2018-04-02 11:39 ` Ævar Arnfjörð Bjarmason 2018-07-26 10:22 ` [RFC PATCH v4 0/4] " Phillip Wood 2018-07-26 10:22 ` [PATCH v4 1/4] " Phillip Wood 2018-07-26 10:22 ` [RFC PATCH v4 2/4] add -p: select modified lines correctly Phillip Wood 2018-07-26 10:22 ` [PATCH v4 3/4] add -p: allow line selection to be inverted Phillip Wood 2018-07-26 10:22 ` [PATCH v4 4/4] add -p: optimize line selection for short hunks Phillip Wood 2018-07-26 15:58 ` [RFC PATCH v5 0/4] add -p: select individual hunk lines Phillip Wood 2018-07-26 15:58 ` [PATCH v5 1/4] " Phillip Wood 2018-07-26 19:36 ` Junio C Hamano 2018-07-27 10:05 ` Phillip Wood 2018-07-27 16:09 ` Junio C Hamano 2018-07-26 15:58 ` [RFC PATCH v5 2/4] add -p: select modified lines correctly Phillip Wood 2018-07-26 19:30 ` Junio C Hamano 2018-07-27 10:19 ` Phillip Wood 2018-07-27 16:14 ` Junio C Hamano 2018-07-26 15:58 ` [PATCH v5 3/4] add -p: allow line selection to be inverted Phillip Wood 2018-07-26 15:58 ` [PATCH v5 4/4] add -p: optimize line selection for short hunks Phillip Wood 2018-07-27 18:27 ` [RFC PATCH v5 0/4] add -p: select individual hunk lines Ævar Arnfjörð Bjarmason 2018-07-28 10:08 ` Phillip Wood 2018-07-28 12:40 ` Ævar Arnfjörð Bjarmason [this message] 2018-08-03 10:01 ` Phillip Wood 2018-08-03 16:51 ` Junio C Hamano 2018-08-03 17:59 ` Ævar Arnfjörð Bjarmason
Reply instructions: You may reply publically 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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ /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
email@example.com list mirror (unofficial, one of many) Archives are clonable: git clone --mirror https://public-inbox.org/git git clone --mirror http://ou63pmih66umazou.onion/git git clone --mirror http://czquwvybam4bgbro.onion/git git clone --mirror http://hjrcffqmbrq6wope.onion/git Newsgroups are available over NNTP: nntp://news.public-inbox.org/inbox.comp.version-control.git nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git nntp://news.gmane.org/gmane.comp.version-control.git note: .onion URLs require Tor: https://www.torproject.org/ or Tor2web: https://www.tor2web.org/ AGPL code for this site: git clone https://public-inbox.org/ public-inbox