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://public-inbox.org/git/878ta8vyqe....@evledraar.gmail.com/ 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.
