{"thread":{"id":"47873","subject":"[PATCH v1 1/3] add -p: select individual hunk lines","startedAt":"2018-02-19T11:36:36Z","lastAt":"2018-08-03T18:00:00Z","messageCount":53,"participants":["Phillip Wood","Gustavo Leite","Junio C Hamano","Igor Djordjevic","Ævar Arnfjörð Bjarmason"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"339609","messageId":"20180219113619.26566-2-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180219113619.26566-1-phillip.wood@talktalk.net","subject":"[PATCH v1 1/3] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:36:17Z","receivedAt":"2018-02-19T11:36:36Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen I end up editing hunks it is almost always because I want to\nstage a subset of the lines in the hunk. Doing this by editing the\nhunk is inconvenient and error prone (especially so if the patch is\ngoing to be reversed before being applied). Instead offer an option\nfor add -p to stage individual lines. When the user presses 'l' the\nhunk is redrawn with labels by the insertions and deletions and they\nare prompted to enter a list of the lines they wish to stage. Ranges\nof lines may be specified using 'a-b' where either 'a' or 'b' may be\nomitted to mean all lines from 'a' to the end of the hunk or all lines\nfrom 1 upto and including 'b'.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 136 +++++++++++++++++++++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 63 +++++++++++++++++++++\n 2 files changed, 199 insertions(+)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex a64c0db57d62ab02ef718b8c8f821105132d9920..8a33796e1f6a564d0a27ba06c216dbb9848827b9 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1003,6 +1003,126 @@ sub color_diff {\n \t} @_;\n }\n \n+sub label_hunk_lines {\n+\tlocal $_;\n+\tmy $hunk = shift;\n+\tmy $i = 0;\n+\tmy $labels = [ map { /^[-+]/ ? ++$i : 0 } @{$hunk->{TEXT}} ];\n+\t$i > 1 and @{$hunk}{qw(LABELS MAX_LABEL)} = ($labels, $i);\n+\treturn $i > 1;\n+}\n+\n+sub select_hunk_lines {\n+\tmy ($hunk, $selected) = @_;\n+\tmy ($text, $labels) = @{$hunk}{qw(TEXT LABELS)};\n+\tmy ($i, $o_cnt, $n_cnt) = (0, 0, 0);\n+\tmy ($push_eol, @newtext);\n+\t# Lines with this mode will become context lines if they are\n+\t# not selected\n+\tmy $context_mode = $patch_mode_flavour{IS_REVERSE} ? '+' : '-';\n+\tfor $i (1..$#{$text}) {\n+\t\tmy $mode = substr($text->[$i], 0, 1);\n+\t\tif ($mode eq '\\\\') {\n+\t\t\tpush @newtext, $text->[$i] if ($push_eol);\n+\t\t\tundef $push_eol;\n+\t\t} elsif ($labels->[$i] and $selected->[$labels->[$i]]) {\n+\t\t\tpush @newtext, $text->[$i];\n+\t\t\tif ($mode eq '+') {\n+\t\t\t\t$n_cnt++;\n+\t\t\t} else {\n+\t\t\t\t$o_cnt++;\n+\t\t\t}\n+\t\t\t$push_eol = 1;\n+\t\t} elsif ($mode eq ' ' or $mode eq $context_mode) {\n+\t\t\tpush @newtext, ' ' . substr($text->[$i], 1);\n+\t\t\t$o_cnt++; $n_cnt++;\n+\t\t\t$push_eol = 1;\n+\t\t} else {\n+\t\t\tundef $push_eol;\n+\t\t}\n+\t}\n+\tmy ($o_ofs, $orig_o_cnt, $n_ofs, $orig_n_cnt) =\n+\t\t\t\t\tparse_hunk_header($text->[0]);\n+\tunshift @newtext, format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n+\tmy $newhunk = {\n+\t\tTEXT => \\@newtext,\n+\t\tDISPLAY => [ color_diff(@newtext) ],\n+\t\tOFS_DELTA => $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt,\n+\t\tTYPE => $hunk->{TYPE},\n+\t\tUSE => 1,\n+\t};\n+\t# If this hunk has previously been edited add the offset delta\n+\t# of the old hunk to get the real delta from the original\n+\t# hunk.\n+\t$hunk->{OFS_DELTA} and $newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n+\treturn $newhunk;\n+}\n+\n+sub check_hunk_label {\n+\tmy ($max_label, $label) = ($_[0]->{MAX_LABEL}, $_[1]);\n+\tif ($label < 1 or $label > $max_label) {\n+\t\terror_msg sprintf(__(\"invalid hunk line '%d'\\n\"), $label);\n+\t\treturn 0;\n+\t}\n+\treturn 1;\n+}\n+\n+sub parse_hunk_selection {\n+\tlocal $_;\n+\tmy ($hunk, $line) = @_;\n+\tmy $max_label = $hunk->{MAX_LABEL};\n+\tmy @selected = (0) x ($max_label + 1);\n+\tmy @fields = split(/[,\\s]+/, $line);\n+\tfor (@fields) {\n+\t\tif (/^([0-9]*)-([0-9]*)$/) {\n+\t\t\tif ($1 eq '' and $2 eq '') {\n+\t\t\t\terror_msg __(\"range '-' missing upper or lower bound\\n\");\n+\t\t\t\treturn undef;\n+\t\t\t}\n+\t\t\tmy $lo = $1 eq '' ? 1 : $1;\n+\t\t\tmy $hi = $2 eq '' ? $max_label : $2;\n+\t\t\tcheck_hunk_label($hunk, $lo) or return undef;\n+\t\t\tcheck_hunk_label($hunk, $hi) or return undef;\n+\t\t\t$hi < $lo and ($lo, $hi) = ($hi, $lo);\n+\t\t\t@selected[$lo..$hi] = (1) x (1 + $hi - $lo);\n+\t\t} elsif (/^([0-9]+)$/) {\n+\t\t\tcheck_hunk_label($hunk, $1) or return undef;\n+\t\t\t$selected[$1] = 1;\n+\t\t} else {\n+\t\t\terror_msg sprintf(__(\"invalid hunk line '%s'\\n\"), $_);\n+\t\t\treturn undef;\n+\t\t}\n+\t}\n+\treturn \\@selected;\n+}\n+\n+sub display_hunk_lines {\n+\tmy ($display, $labels, $max_label) =\n+\t\t\t\t@{$_[0]}{qw(DISPLAY LABELS MAX_LABEL)};\n+\tmy $width = int(log($max_label) / log(10)) + 1;\n+\tmy $padding = ' ' x ($width + 1);\n+\tfor my $i (0..$#{$display}) {\n+\t\tif ($labels->[$i]) {\n+\t\t\tprintf '%*d %s', $width, $labels->[$i], $display->[$i];\n+\t\t} else {\n+\t\t\tprint $padding . $display->[$i];\n+\t\t}\n+\t}\n+}\n+\n+sub select_lines_loop {\n+\tmy $hunk = shift;\n+\tdisplay_hunk_lines($hunk);\n+\tmy $selection = undef;\n+\tuntil (defined $selection) {\n+\t\tprint colored $prompt_color, __(\"select lines? \");\n+\t\tmy $text = ;\n+\t\tdefined $text and $text =~ /\\S/ or return undef;\n+\t\t$selection = parse_hunk_selection($hunk, $text);\n+\t}\n+\treturn select_hunk_lines($hunk, $selection);\n+}\n+\n my %edit_hunk_manually_modes = (\n \tstage => N__(\n \"If the patch applies cleanly, the edited hunk will immediately be\n@@ -1245,6 +1365,7 @@ j - leave this hunk undecided, see next undecided hunk\n J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n+l - select hunk lines to use\n s - split the current hunk into smaller hunks\n e - manually edit the current hunk\n ? - print help\n@@ -1461,6 +1582,9 @@ sub patch_update_file {\n \t\tif ($hunk[$ix]{TYPE} eq 'hunk') {\n \t\t\t$other .= ',e';\n \t\t}\n+\t\tif (label_hunk_lines($hunk[$ix])) {\n+\t\t\t$other .= ',l';\n+\t\t}\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -1600,6 +1724,18 @@ sub patch_update_file {\n \t\t\t\t\tnext;\n \t\t\t\t}\n \t\t\t}\n+\t\t\telsif ($line =~ /^l/) {\n+\t\t\t\tunless ($other =~ /l/) {\n+\t\t\t\t\terror_msg __(\"Cannot select line by line\\n\");\n+\t\t\t\t\tnext;\n+\t\t\t\t}\n+\t\t\t\tmy $newhunk = select_lines_loop($hunk[$ix]);\n+\t\t\t\tif ($newhunk) {\n+\t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n+\t\t\t\t} else {\n+\t\t\t\t\tnext;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\telsif ($other =~ /s/ && $line =~ /^s/) {\n \t\t\t\tmy @split = split_hunk($hunk[$ix]{TEXT}, $hunk[$ix]{DISPLAY});\n \t\t\t\tif (1 < @split) {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 0fb9c0e0f140e21ef7ad467c40b9211d29f53db6..caa80327c461785949eb2b9c919c253f4bef72cc 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -342,6 +342,61 @@ test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n \t! grep \"^+31\" actual\n '\n \n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,9 @@\n+\t+5\n+\t 10\n+\t 20\n+\t+21\n+\t 30\n+\t 40\n+\t 50\n+\t 60\n+\t+61\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can stage individual lines of patch' '\n+\tgit reset &&\n+\tprintf 61 >>test &&\n+\tprintf \"%s\\n\" l \"-2 4\" |\n+\tEDITOR=: git add -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD | sed /^index/d >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,8 @@\n+\t+5\n+\t 10\n+\t 20\n+\t 30\n+\t 40\n+\t 50\n+\t 60\n+\t+61\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can reset individual lines of patch' '\n+\tprintf \"%s\\n\" l 2 |\n+\tEDITOR=: git reset -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD | sed /^index/d >actual &&\n+\ttest_cmp expected actual\n+'\n+\n test_expect_success 'patch mode ignores unmerged entries' '\n \tgit reset --hard &&\n \ttest_commit conflict &&\n@@ -558,4 +613,12 @@ test_expect_success 'add -p patch editing works with pathological context lines'\n \ttest_cmp expected-2 actual\n '\n \n+test_expect_success 'add -p selecting lines works with pathological context lines' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l 2 y |\n+\tGIT_EDITOR=./editor git add -p &&\n+\tgit cat-file blob :a > actual &&\n+\ttest_cmp expected-2 actual\n+'\n+\n test_done\n-- \n2.16.1\n\n"},{"id":"339610","messageId":"20180219113619.26566-1-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":null,"subject":"[PATCH v1 0/3] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:36:16Z","receivedAt":"2018-02-19T11:36:40Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nI need to update the add -i documentation but otherwise I think these\npatches are OK so I thought I'd try and get some feedback. They build\non top of the recount fixes in [1]. The commit message for the first\npatch describes the motivation:\n\n\"When I end up editing hunks it is almost always because I want to\nstage a subset of the lines in the hunk. Doing this by editing the\nhunk is inconvenient and error prone (especially so if the patch is\ngoing to be reversed before being applied). Instead offer an option\nfor add -p to stage individual lines. When the user presses 'l' the\nhunk is redrawn with labels by the insertions and deletions and they\nare prompted to enter a list of the lines they wish to stage. Ranges\nof lines may be specified using 'a-b' where either 'a' or 'b' may be\nomitted to mean all lines from 'a' to the end of the hunk or all lines\nfrom 1 upto and including 'b'.\"\n\n[1] https://public-inbox.org/git/20180219112910.24471-1-phillip.wood@talktalk.net/T/#u\n\nPhillip Wood (3):\n add -p: select individual hunk lines\n add -p: allow line selection to be inverted\n add -p: optimize line selection for short hunks\n\n git-add--interactive.perl | 179 +++++++++++++++++++++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 63 ++++++++++++++++\n 2 files changed, 242 insertions(+)\n\n-- \n2.16.1\n\n"},{"id":"339611","messageId":"20180219113619.26566-3-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180219113619.26566-1-phillip.wood@talktalk.net","subject":"[PATCH v1 2/3] add -p: allow line selection to be inverted","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:36:18Z","receivedAt":"2018-02-19T11:36:43Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf the list of lines to be selected begins with '^' select all the\nlines except the ones listed.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 15 ++++++++++++++-\n t/t3701-add-interactive.sh | 2 +-\n 2 files changed, 15 insertions(+), 2 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 8a33796e1f6a564d0a27ba06c216dbb9848827b9..0e3960b1ecf004bff51d28d540f685a5dc91fad1 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1070,9 +1070,21 @@ sub check_hunk_label {\n sub parse_hunk_selection {\n \tlocal $_;\n \tmy ($hunk, $line) = @_;\n-\tmy $max_label = $hunk->{MAX_LABEL};\n+\tmy ($max_label, $invert) = ($hunk->{MAX_LABEL}, undef);\n \tmy @selected = (0) x ($max_label + 1);\n \tmy @fields = split(/[,\\s]+/, $line);\n+\tif ($fields[0] =~ /^\\^(.*)/) {\n+\t\t$invert = 1;\n+\t\tif ($1 ne '') {\n+\t\t\t$fields[0] = $1;\n+\t\t} else {\n+\t\t\tshift @fields;\n+\t\t\tunless (@fields) {\n+\t\t\t\terror_msg __(\"no lines to invert\\n\");\n+\t\t\t\treturn undef;\n+\t\t\t}\n+\t\t}\n+\t}\n \tfor (@fields) {\n \t\tif (/^([0-9]*)-([0-9]*)$/) {\n \t\t\tif ($1 eq '' and $2 eq '') {\n@@ -1093,6 +1105,7 @@ sub parse_hunk_selection {\n \t\t\treturn undef;\n \t\t}\n \t}\n+\t$invert and @selected = map { !$_ } @selected;\n \treturn \\@selected;\n }\n \ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex caa80327c461785949eb2b9c919c253f4bef72cc..4ae706fd121f157e9cbd93ec293f45ce2a3a53b5 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -390,7 +390,7 @@ test_expect_success 'setup expected diff' '\n '\n \n test_expect_success 'can reset individual lines of patch' '\n-\tprintf \"%s\\n\" l 2 |\n+\tprintf \"%s\\n\" l \"^1 3\" |\n \tEDITOR=: git reset -p 2>error &&\n \ttest_must_be_empty error &&\n \tgit diff --cached HEAD | sed /^index/d >actual &&\n-- \n2.16.1\n\n"},{"id":"339612","messageId":"20180219113619.26566-4-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180219113619.26566-1-phillip.wood@talktalk.net","subject":"[PATCH v1 3/3] add -p: optimize line selection for short hunks","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-02-19T11:36:19Z","receivedAt":"2018-02-19T11:36:45Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf there are fewer than ten changes in a hunk then make spaces\noptional when selecting individual lines. This means that for short\nhunks one can just type -357 to stage lines 1, 2, 3, 5 & 7.\n\nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 30 ++++++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 2 +-\n 2 files changed, 31 insertions(+), 1 deletion(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 0e3960b1ecf004bff51d28d540f685a5dc91fad1..3d9720af03eb113c7f3d2b73f17b4b51a7685bf3 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1067,6 +1067,33 @@ sub check_hunk_label {\n \treturn 1;\n }\n \n+sub split_hunk_selection {\n+\tlocal $_;\n+\tmy @fields = @_;\n+\tmy @ret;\n+\tfor (@fields) {\n+\t\tif (/^(-[0-9])(.*)/) {\n+\t\t\tpush @ret, $1;\n+\t\t\t$_ = $2;\n+\t\t}\n+\t\twhile ($_ ne '') {\n+\t\t\tif (/^[0-9]-$/) {\n+\t\t\t\tpush @ret, $_;\n+\t\t\t\tlast;\n+\t\t\t} elsif (/^([0-9](?:-[0-9])?)(.*)/) {\n+\t\t\t\tpush @ret, $1;\n+\t\t\t\t$_ = $2;\n+\t\t\t} else {\n+\t\t\t\terror_msg sprintf\n+\t\t\t\t __(\"invalid hunk line '%s'\\n\"),\n+\t\t\t\t substr($_, 0, 1);\n+\t\t\t\treturn ();\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn @ret;\n+}\n+\n sub parse_hunk_selection {\n \tlocal $_;\n \tmy ($hunk, $line) = @_;\n@@ -1085,6 +1112,9 @@ sub parse_hunk_selection {\n \t\t\t}\n \t\t}\n \t}\n+\tif ($max_label < 10) {\n+\t\t@fields = split_hunk_selection(@fields) or return undef;\n+\t}\n \tfor (@fields) {\n \t\tif (/^([0-9]*)-([0-9]*)$/) {\n \t\t\tif ($1 eq '' and $2 eq '') {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 4ae706fd121f157e9cbd93ec293f45ce2a3a53b5..c6d847dc495c92782e37ef7b0e2800d7936aabd7 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -390,7 +390,7 @@ test_expect_success 'setup expected diff' '\n '\n \n test_expect_success 'can reset individual lines of patch' '\n-\tprintf \"%s\\n\" l \"^1 3\" |\n+\tprintf \"%s\\n\" l ^13 |\n \tEDITOR=: git reset -p 2>error &&\n \ttest_must_be_empty error &&\n \tgit diff --cached HEAD | sed /^index/d >actual &&\n-- \n2.16.1\n\n"},{"id":"339613","messageId":"CAEGv8HiRsS9N2nHBu8uwm+35GS+HRVc6Q0e1R+=-tZn7JcZ8BQ@mail.gmail.com","threadId":"47873","inReplyTo":"20180219113619.26566-1-phillip.wood@talktalk.net","subject":"Re: [PATCH v1 0/3] add -p: select individual hunk lines","fromName":"Gustavo Leite","fromEmail":"gustavoleite.ti@gmail.com","sentAt":"2018-02-19T12:20:26Z","receivedAt":"2018-02-19T12:20:33Z","isPatch":true,"sender":{"key":"gustavoleite.ti@gmail.com","avatar":null},"body":"2018-02-19 8:36 GMT-03:00 Phillip Wood :\n>\n> \"When I end up editing hunks it is almost always because I want to\n> stage a subset of the lines in the hunk. Doing this by editing the\n> hunk is inconvenient and error prone (especially so if the patch is\n> going to be reversed before being applied). Instead offer an option\n> for add -p to stage individual lines. When the user presses 'l' the\n> hunk is redrawn with labels by the insertions and deletions and they\n> are prompted to enter a list of the lines they wish to stage. Ranges\n> of lines may be specified using 'a-b' where either 'a' or 'b' may be\n> omitted to mean all lines from 'a' to the end of the hunk or all lines\n> from 1 upto and including 'b'.\"\n\nThis is an interesting (and needed feature). Would be nice to see it merged.\n\n--\nGustavo Leite\n"},{"id":"341111","messageId":"20180306101750.18794-3-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180306101750.18794-1-phillip.wood@talktalk.net","subject":"[PATCH v2 2/3] add -p: allow line selection to be inverted","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-06T10:17:49Z","receivedAt":"2018-03-06T10:18:06Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf the list of lines to be selected begins with '^' select all the\nlines except the ones listed.\n\nSigned-off-by: Phillip Wood \n---\n Documentation/git-add.txt | 3 ++-\n git-add--interactive.perl | 17 ++++++++++++++++-\n t/t3701-add-interactive.sh | 2 +-\n 3 files changed, 19 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex ad33fda9a2..0e2c11e97b 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -341,7 +341,8 @@ If you press \"l\" then the hunk will be reprinted with each insertion\n or deletion labelled with a number and you will be prompted to enter\n which lines you wish to select. Individual line numbers should be\n separated by a space or comma, to specify a range of lines use a dash\n-between them.\n+between them. To invert the selection prefix it with \"\\^\" so \"^3-5,8\"\n+will select everything except lines 3, 4, 5 and 8.\n +\n After deciding the fate for all hunks, if there is any hunk\n that was chosen, the index is updated with the selected hunks.\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex a273b41e95..6fa3d0a87c 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1085,9 +1085,21 @@ sub check_hunk_label {\n sub parse_hunk_selection {\n \tlocal $_;\n \tmy ($hunk, $line) = @_;\n-\tmy $max_label = $hunk->{MAX_LABEL};\n+\tmy ($max_label, $invert) = ($hunk->{MAX_LABEL}, undef);\n \tmy @selected = (0) x ($max_label + 1);\n \tmy @fields = split(/[,\\s]+/, $line);\n+\tif ($fields[0] =~ /^\\^(.*)/) {\n+\t\t$invert = 1;\n+\t\tif ($1 ne '') {\n+\t\t\t$fields[0] = $1;\n+\t\t} else {\n+\t\t\tshift @fields;\n+\t\t\tunless (@fields) {\n+\t\t\t\terror_msg __(\"no lines to invert\\n\");\n+\t\t\t\treturn undef;\n+\t\t\t}\n+\t\t}\n+\t}\n \tfor (@fields) {\n \t\tif (/^([0-9]*)-([0-9]*)$/) {\n \t\t\tif ($1 eq '' and $2 eq '') {\n@@ -1110,6 +1122,9 @@ sub parse_hunk_selection {\n \t\t\treturn undef;\n \t\t}\n \t}\n+\tif ($invert) {\n+\t\t@selected = map { !$_ } @selected;\n+\t}\n \treturn \\@selected;\n }\n \ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 65c8c3354b..89c0e73f2b 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -410,7 +410,7 @@ test_expect_success 'setup expected diff' '\n '\n \n test_expect_success 'can reset individual lines of patch' '\n-\tprintf \"%s\\n\" l 2 |\n+\tprintf \"%s\\n\" l \"^1 3\" |\n \tEDITOR=: git reset -p 2>error &&\n \ttest_must_be_empty error &&\n \tgit diff --cached HEAD >actual &&\n-- \n2.16.2\n\n"},{"id":"341112","messageId":"20180306101750.18794-2-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180306101750.18794-1-phillip.wood@talktalk.net","subject":"[PATCH v2 1/3] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-06T10:17:48Z","receivedAt":"2018-03-06T10:18:09Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen I end up editing hunks it is almost always because I want to\nstage a subset of the lines in the hunk. Doing this by editing the\nhunk is inconvenient and error prone (especially so if the patch is\ngoing to be reversed before being applied). Instead offer an option\nfor add -p to stage individual lines. When the user presses 'l' the\nhunk is redrawn with labels by the insertions and deletions and they\nare prompted to enter a list of the lines they wish to stage. Ranges\nof lines may be specified using 'a-b' where either 'a' or 'b' may be\nomitted to mean all lines from 'a' to the end of the hunk or all lines\nfrom 1 upto and including 'b'.\n\nSigned-off-by: Phillip Wood \n---\n Documentation/git-add.txt | 7 +++\n git-add--interactive.perl | 143 +++++++++++++++++++++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 65 +++++++++++++++++++++\n 3 files changed, 215 insertions(+)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex d50fa339dc..ad33fda9a2 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -332,10 +332,17 @@ patch::\n J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n+ l - select hunk lines to use\n s - split the current hunk into smaller hunks\n e - manually edit the current hunk\n ? - print help\n +\n+If you press \"l\" then the hunk will be reprinted with each insertion\n+or deletion labelled with a number and you will be prompted to enter\n+which lines you wish to select. Individual line numbers should be\n+separated by a space or comma, to specify a range of lines use a dash\n+between them.\n++\n After deciding the fate for all hunks, if there is any hunk\n that was chosen, the index is updated with the selected hunks.\n +\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex f83e7450ad..a273b41e95 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1013,6 +1013,133 @@ sub color_diff {\n \t} @_;\n }\n \n+sub label_hunk_lines {\n+\tlocal $_;\n+\tmy $hunk = shift;\n+\tmy $i = 0;\n+\tmy $labels = [ map { /^[-+]/ ? ++$i : 0 } @{$hunk->{TEXT}} ];\n+\tif ($i > 1) {\n+\t\t@{$hunk}{qw(LABELS MAX_LABEL)} = ($labels, $i);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+sub select_hunk_lines {\n+\tmy ($hunk, $selected) = @_;\n+\tmy ($text, $labels) = @{$hunk}{qw(TEXT LABELS)};\n+\tmy ($i, $o_cnt, $n_cnt) = (0, 0, 0);\n+\tmy ($push_eol, @newtext);\n+\t# Lines with this mode will become context lines if they are\n+\t# not selected\n+\tmy $context_mode = $patch_mode_flavour{IS_REVERSE} ? '+' : '-';\n+\tfor $i (1..$#{$text}) {\n+\t\tmy $mode = substr($text->[$i], 0, 1);\n+\t\tif ($mode eq '\\\\') {\n+\t\t\tpush @newtext, $text->[$i] if ($push_eol);\n+\t\t\tundef $push_eol;\n+\t\t} elsif ($labels->[$i] and $selected->[$labels->[$i]]) {\n+\t\t\tpush @newtext, $text->[$i];\n+\t\t\tif ($mode eq '+') {\n+\t\t\t\t$n_cnt++;\n+\t\t\t} else {\n+\t\t\t\t$o_cnt++;\n+\t\t\t}\n+\t\t\t$push_eol = 1;\n+\t\t} elsif ($mode eq ' ' or $mode eq $context_mode) {\n+\t\t\tpush @newtext, ' ' . substr($text->[$i], 1);\n+\t\t\t$o_cnt++; $n_cnt++;\n+\t\t\t$push_eol = 1;\n+\t\t} else {\n+\t\t\tundef $push_eol;\n+\t\t}\n+\t}\n+\tmy ($o_ofs, $orig_o_cnt, $n_ofs, $orig_n_cnt) =\n+\t\t\t\t\tparse_hunk_header($text->[0]);\n+\tunshift @newtext, format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n+\tmy $newhunk = {\n+\t\tTEXT => \\@newtext,\n+\t\tDISPLAY => [ color_diff(@newtext) ],\n+\t\tOFS_DELTA => $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt,\n+\t\tTYPE => $hunk->{TYPE},\n+\t\tUSE => 1,\n+\t};\n+\t# If this hunk has previously been edited add the offset delta\n+\t# of the old hunk to get the real delta from the original\n+\t# hunk.\n+\tif ($hunk->{OFS_DELTA}) {\n+\t\t$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n+\t}\n+\treturn $newhunk;\n+}\n+\n+sub check_hunk_label {\n+\tmy ($max_label, $label) = ($_[0]->{MAX_LABEL}, $_[1]);\n+\tif ($label < 1 or $label > $max_label) {\n+\t\terror_msg sprintf(__(\"invalid hunk line '%d'\\n\"), $label);\n+\t\treturn 0;\n+\t}\n+\treturn 1;\n+}\n+\n+sub parse_hunk_selection {\n+\tlocal $_;\n+\tmy ($hunk, $line) = @_;\n+\tmy $max_label = $hunk->{MAX_LABEL};\n+\tmy @selected = (0) x ($max_label + 1);\n+\tmy @fields = split(/[,\\s]+/, $line);\n+\tfor (@fields) {\n+\t\tif (/^([0-9]*)-([0-9]*)$/) {\n+\t\t\tif ($1 eq '' and $2 eq '') {\n+\t\t\t\terror_msg __(\"range '-' missing upper or lower bound\\n\");\n+\t\t\t\treturn undef;\n+\t\t\t}\n+\t\t\tmy $lo = $1 eq '' ? 1 : $1;\n+\t\t\tmy $hi = $2 eq '' ? $max_label : $2;\n+\t\t\tcheck_hunk_label($hunk, $lo) or return undef;\n+\t\t\tcheck_hunk_label($hunk, $hi) or return undef;\n+\t\t\tif ($hi < $lo) {\n+\t\t\t\t($lo, $hi) = ($hi, $lo);\n+\t\t\t}\n+\t\t\t@selected[$lo..$hi] = (1) x (1 + $hi - $lo);\n+\t\t} elsif (/^([0-9]+)$/) {\n+\t\t\tcheck_hunk_label($hunk, $1) or return undef;\n+\t\t\t$selected[$1] = 1;\n+\t\t} else {\n+\t\t\terror_msg sprintf(__(\"invalid hunk line '%s'\\n\"), $_);\n+\t\t\treturn undef;\n+\t\t}\n+\t}\n+\treturn \\@selected;\n+}\n+\n+sub display_hunk_lines {\n+\tmy ($display, $labels, $max_label) =\n+\t\t\t\t@{$_[0]}{qw(DISPLAY LABELS MAX_LABEL)};\n+\tmy $width = int(log($max_label) / log(10)) + 1;\n+\tmy $padding = ' ' x ($width + 1);\n+\tfor my $i (0..$#{$display}) {\n+\t\tif ($labels->[$i]) {\n+\t\t\tprintf '%*d %s', $width, $labels->[$i], $display->[$i];\n+\t\t} else {\n+\t\t\tprint $padding . $display->[$i];\n+\t\t}\n+\t}\n+}\n+\n+sub select_lines_loop {\n+\tmy $hunk = shift;\n+\tdisplay_hunk_lines($hunk);\n+\tmy $selection = undef;\n+\tuntil (defined $selection) {\n+\t\tprint colored $prompt_color, __(\"select lines? \");\n+\t\tmy $text = ;\n+\t\tdefined $text and $text =~ /\\S/ or return undef;\n+\t\t$selection = parse_hunk_selection($hunk, $text);\n+\t}\n+\treturn select_hunk_lines($hunk, $selection);\n+}\n+\n my %edit_hunk_manually_modes = (\n \tstage => N__(\n \"If the patch applies cleanly, the edited hunk will immediately be\n@@ -1255,6 +1382,7 @@ j - leave this hunk undecided, see next undecided hunk\n J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n+l - select hunk lines to use\n s - split the current hunk into smaller hunks\n e - manually edit the current hunk\n ? - print help\n@@ -1471,6 +1599,9 @@ sub patch_update_file {\n \t\tif ($hunk[$ix]{TYPE} eq 'hunk') {\n \t\t\t$other .= ',e';\n \t\t}\n+\t\tif (label_hunk_lines($hunk[$ix])) {\n+\t\t\t$other .= ',l';\n+\t\t}\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -1610,6 +1741,18 @@ sub patch_update_file {\n \t\t\t\t\tnext;\n \t\t\t\t}\n \t\t\t}\n+\t\t\telsif ($line =~ /^l/) {\n+\t\t\t\tunless ($other =~ /l/) {\n+\t\t\t\t\terror_msg __(\"Cannot select line by line\\n\");\n+\t\t\t\t\tnext;\n+\t\t\t\t}\n+\t\t\t\tmy $newhunk = select_lines_loop($hunk[$ix]);\n+\t\t\t\tif ($newhunk) {\n+\t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n+\t\t\t\t} else {\n+\t\t\t\t\tnext;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\telsif ($other =~ /s/ && $line =~ /^s/) {\n \t\t\t\tmy @split = split_hunk($hunk[$ix]{TEXT}, $hunk[$ix]{DISPLAY});\n \t\t\t\tif (1 < @split) {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex a9a9478a29..65c8c3354b 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -360,6 +360,63 @@ test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n \t! grep \"^+31\" actual\n '\n \n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..341cc6b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,9 @@\n+\t+5\n+\t 10\n+\t 20\n+\t+21\n+\t 30\n+\t 40\n+\t 50\n+\t 60\n+\t+61\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can stage individual lines of patch' '\n+\tgit reset &&\n+\tprintf 61 >>test &&\n+\tprintf \"%s\\n\" l \"-2 4\" |\n+\tEDITOR=: git add -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..cc6163b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,8 @@\n+\t+5\n+\t 10\n+\t 20\n+\t 30\n+\t 40\n+\t 50\n+\t 60\n+\t+61\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can reset individual lines of patch' '\n+\tprintf \"%s\\n\" l 2 |\n+\tEDITOR=: git reset -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n test_expect_success 'patch mode ignores unmerged entries' '\n \tgit reset --hard &&\n \ttest_commit conflict &&\n@@ -576,4 +633,12 @@ test_expect_success 'add -p patch editing works with pathological context lines'\n \ttest_cmp expected-2 actual\n '\n \n+test_expect_success 'add -p selecting lines works with pathological context lines' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l 2 y |\n+\tGIT_EDITOR=./editor git add -p &&\n+\tgit cat-file blob :a >actual &&\n+\ttest_cmp expected-2 actual\n+'\n+\n test_done\n-- \n2.16.2\n\n"},{"id":"341113","messageId":"20180306101750.18794-1-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180219113619.26566-1-phillip.wood@talktalk.net","subject":"[PATCH v2 0/3] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-06T10:17:47Z","receivedAt":"2018-03-06T10:18:12Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nI've added some documentation to git-add.txt for the new selection\nmode and cleaned up some style issues, otherwise these are unchanged\nsince v1. These patches build on top of the recount fixes in [1]. The\ncommit message for the first patch describes the motivation:\n\n\"When I end up editing hunks it is almost always because I want to\nstage a subset of the lines in the hunk. Doing this by editing the\nhunk is inconvenient and error prone (especially so if the patch is\ngoing to be reversed before being applied). Instead offer an option\nfor add -p to stage individual lines. When the user presses 'l' the\nhunk is redrawn with labels by the insertions and deletions and they\nare prompted to enter a list of the lines they wish to stage. Ranges\nof lines may be specified using 'a-b' where either 'a' or 'b' may be\nomitted to mean all lines from 'a' to the end of the hunk or all lines\nfrom 1 upto and including 'b'.\"\n\n[1] https://public-inbox.org/git/xmqqbmg29x1n.fsf@gitster-ct.c.googlers.com/T/#m01d0f1af90f32b698e583b56f8e53b986bcec7c6\n\nInterdiff from v1:\n\n Documentation/git-add.txt | 9 +++++++++\n git-add--interactive.perl | 19 ++++++++++++++-----\n t/t3701-add-interactive.sh | 12 +++++++-----\n 3 files changed, 30 insertions(+), 10 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex d50fa339dc..d52acfc722 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -332,10 +332,19 @@ patch::\n J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n+ l - select hunk lines to use\n s - split the current hunk into smaller hunks\n e - manually edit the current hunk\n ? - print help\n +\n+If you press \"l\" then the hunk will be reprinted with each insertion\n+or deletion labelled with a number and you will be prompted to enter\n+which lines you wish to select. Individual line numbers should be\n+separated by a space or comma (these can be omitted if there are fewer\n+than ten labelled lines), to specify a range of lines use a dash\n+between them. To invert the selection prefix it with \"\\^\" so \"^3-5,8\"\n+will select everything except lines 3, 4, 5 and 8.\n++\n After deciding the fate for all hunks, if there is any hunk\n that was chosen, the index is updated with the selected hunks.\n +\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex aa474c5149..9a6bcd5085 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1018,8 +1018,11 @@ sub label_hunk_lines {\n \tmy $hunk = shift;\n \tmy $i = 0;\n \tmy $labels = [ map { /^[-+]/ ? ++$i : 0 } @{$hunk->{TEXT}} ];\n-\t$i > 1 and @{$hunk}{qw(LABELS MAX_LABEL)} = ($labels, $i);\n-\treturn $i > 1;\n+\tif ($i > 1) {\n+\t\t@{$hunk}{qw(LABELS MAX_LABEL)} = ($labels, $i);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n }\n \n sub select_hunk_lines {\n@@ -1064,7 +1067,9 @@ sub select_hunk_lines {\n \t# If this hunk has previously been edited add the offset delta\n \t# of the old hunk to get the real delta from the original\n \t# hunk.\n-\t$hunk->{OFS_DELTA} and $newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n+\tif ($hunk->{OFS_DELTA}) {\n+\t\t$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n+\t}\n \treturn $newhunk;\n }\n \n@@ -1135,7 +1140,9 @@ sub parse_hunk_selection {\n \t\t\tmy $hi = $2 eq '' ? $max_label : $2;\n \t\t\tcheck_hunk_label($hunk, $lo) or return undef;\n \t\t\tcheck_hunk_label($hunk, $hi) or return undef;\n-\t\t\t$hi < $lo and ($lo, $hi) = ($hi, $lo);\n+\t\t\tif ($hi < $lo) {\n+\t\t\t\t($lo, $hi) = ($hi, $lo);\n+\t\t\t}\n \t\t\t@selected[$lo..$hi] = (1) x (1 + $hi - $lo);\n \t\t} elsif (/^([0-9]+)$/) {\n \t\t\tcheck_hunk_label($hunk, $1) or return undef;\n@@ -1145,7 +1152,9 @@ sub parse_hunk_selection {\n \t\t\treturn undef;\n \t\t}\n \t}\n-\t$invert and @selected = map { !$_ } @selected;\n+\tif ($invert) {\n+\t\t@selected = map { !$_ } @selected;\n+\t}\n \treturn \\@selected;\n }\n \ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 7bea4a2341..d3bce154da 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -363,6 +363,7 @@ test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n test_expect_success 'setup expected diff' '\n \tcat >expected <<-\\EOF\n \tdiff --git a/test b/test\n+\tindex 0889435..341cc6b 100644\n \t--- a/test\n \t+++ b/test\n \t@@ -1,6 +1,9 @@\n@@ -385,13 +386,14 @@ test_expect_success 'can stage individual lines of patch' '\n \tprintf \"%s\\n\" l \"-2 4\" |\n \tEDITOR=: git add -p 2>error &&\n \ttest_must_be_empty error &&\n-\tgit diff --cached HEAD | sed /^index/d >actual &&\n-\ttest_cmp expected actual\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n '\n \n test_expect_success 'setup expected diff' '\n \tcat >expected <<-\\EOF\n \tdiff --git a/test b/test\n+\tindex 0889435..cc6163b 100644\n \t--- a/test\n \t+++ b/test\n \t@@ -1,6 +1,8 @@\n@@ -411,8 +413,8 @@ test_expect_success 'can reset individual lines of patch' '\n \tprintf \"%s\\n\" l ^13 |\n \tEDITOR=: git reset -p 2>error &&\n \ttest_must_be_empty error &&\n-\tgit diff --cached HEAD | sed /^index/d >actual &&\n-\ttest_cmp expected actual\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n '\n \n test_expect_success 'patch mode ignores unmerged entries' '\n@@ -635,7 +637,7 @@ test_expect_success 'add -p selecting lines works with pathological context line\n \tgit reset &&\n \tprintf \"%s\\n\" l 2 y |\n \tGIT_EDITOR=./editor git add -p &&\n-\tgit cat-file blob :a > actual &&\n+\tgit cat-file blob :a >actual &&\n \ttest_cmp expected-2 actual\n '\n \n\nPhillip Wood (3):\n add -p: select individual hunk lines\n add -p: allow line selection to be inverted\n add -p: optimize line selection for short hunks\n\n Documentation/git-add.txt | 9 +++\n git-add--interactive.perl | 188 +++++++++++++++++++++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 65 ++++++++++++++++\n 3 files changed, 262 insertions(+)\n\n-- \n2.16.2\n\n"},{"id":"341114","messageId":"20180306101750.18794-4-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180306101750.18794-1-phillip.wood@talktalk.net","subject":"[PATCH v2 3/3] add -p: optimize line selection for short hunks","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-06T10:17:50Z","receivedAt":"2018-03-06T10:18:14Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf there are fewer than ten changes in a hunk then make spaces\noptional when selecting individual lines. This means that for short\nhunks one can just type -357 to stage lines 1, 2, 3, 5 & 7.\n\nSigned-off-by: Phillip Wood \n---\n Documentation/git-add.txt | 3 ++-\n git-add--interactive.perl | 30 ++++++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 2 +-\n 3 files changed, 33 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 0e2c11e97b..d52acfc722 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -340,7 +340,8 @@ patch::\n If you press \"l\" then the hunk will be reprinted with each insertion\n or deletion labelled with a number and you will be prompted to enter\n which lines you wish to select. Individual line numbers should be\n-separated by a space or comma, to specify a range of lines use a dash\n+separated by a space or comma (these can be omitted if there are fewer\n+than ten labelled lines), to specify a range of lines use a dash\n between them. To invert the selection prefix it with \"\\^\" so \"^3-5,8\"\n will select everything except lines 3, 4, 5 and 8.\n +\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 6fa3d0a87c..9a6bcd5085 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1082,6 +1082,33 @@ sub check_hunk_label {\n \treturn 1;\n }\n \n+sub split_hunk_selection {\n+\tlocal $_;\n+\tmy @fields = @_;\n+\tmy @ret;\n+\tfor (@fields) {\n+\t\tif (/^(-[0-9])(.*)/) {\n+\t\t\tpush @ret, $1;\n+\t\t\t$_ = $2;\n+\t\t}\n+\t\twhile ($_ ne '') {\n+\t\t\tif (/^[0-9]-$/) {\n+\t\t\t\tpush @ret, $_;\n+\t\t\t\tlast;\n+\t\t\t} elsif (/^([0-9](?:-[0-9])?)(.*)/) {\n+\t\t\t\tpush @ret, $1;\n+\t\t\t\t$_ = $2;\n+\t\t\t} else {\n+\t\t\t\terror_msg sprintf\n+\t\t\t\t __(\"invalid hunk line '%s'\\n\"),\n+\t\t\t\t substr($_, 0, 1);\n+\t\t\t\treturn ();\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn @ret;\n+}\n+\n sub parse_hunk_selection {\n \tlocal $_;\n \tmy ($hunk, $line) = @_;\n@@ -1100,6 +1127,9 @@ sub parse_hunk_selection {\n \t\t\t}\n \t\t}\n \t}\n+\tif ($max_label < 10) {\n+\t\t@fields = split_hunk_selection(@fields) or return undef;\n+\t}\n \tfor (@fields) {\n \t\tif (/^([0-9]*)-([0-9]*)$/) {\n \t\t\tif ($1 eq '' and $2 eq '') {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 89c0e73f2b..d3bce154da 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -410,7 +410,7 @@ test_expect_success 'setup expected diff' '\n '\n \n test_expect_success 'can reset individual lines of patch' '\n-\tprintf \"%s\\n\" l \"^1 3\" |\n+\tprintf \"%s\\n\" l ^13 |\n \tEDITOR=: git reset -p 2>error &&\n \ttest_must_be_empty error &&\n \tgit diff --cached HEAD >actual &&\n-- \n2.16.2\n\n"},{"id":"341148","messageId":"xmqq371d3rjz.fsf@gitster-ct.c.googlers.com","threadId":"47873","inReplyTo":"20180306101750.18794-3-phillip.wood@talktalk.net","subject":"Re: [PATCH v2 2/3] add -p: allow line selection to be inverted","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-06T19:57:52Z","receivedAt":"2018-03-06T19:58:00Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> If the list of lines to be selected begins with '^' select all the\n> lines except the ones listed.\n\nThere is \"# Input that begins with '-'; unchoose\" in list_and_choose\nhelper. Does it make things inconsistent to use '^' for negation\nlike this patch does with it?\n"},{"id":"341149","messageId":"64005dad-ec23-892c-4270-b01cae74f663@gmail.com","threadId":"47873","inReplyTo":"20180306101750.18794-1-phillip.wood@talktalk.net","subject":"Re: [PATCH v2 0/3] add -p: select individual hunk lines","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2018-03-06T20:19:15Z","receivedAt":"2018-03-06T20:19:32Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"Hi Phillip,\n\nOn 06/03/2018 11:17, Phillip Wood wrote:\n> \n> From: Phillip Wood \n> \n> I've added some documentation to git-add.txt for the new selection\n> mode and cleaned up some style issues, otherwise these are unchanged\n> since v1. These patches build on top of the recount fixes in [1]. The\n> commit message for the first patch describes the motivation:\n> \n> \"When I end up editing hunks it is almost always because I want to\n> stage a subset of the lines in the hunk. Doing this by editing the\n> hunk is inconvenient and error prone (especially so if the patch is\n> going to be reversed before being applied). Instead offer an option\n> for add -p to stage individual lines. When the user presses 'l' the\n> hunk is redrawn with labels by the insertions and deletions and they\n> are prompted to enter a list of the lines they wish to stage. Ranges\n> of lines may be specified using 'a-b' where either 'a' or 'b' may be\n> omitted to mean all lines from 'a' to the end of the hunk or all lines\n> from 1 upto and including 'b'.\"\n> \n> [1] https://public-inbox.org/git/xmqqbmg29x1n.fsf@gitster-ct.c.googlers.com/T/#m01d0f1af90f32b698e583b56f8e53b986bcec7c6\n\nNice, thank you :)\n\nA small nitpick - I see you use phrasing like \"select lines\", where \nthe other commands usually talk about \"staging\", instead, so \"stage \nlines\" might be more aligned with the existing text.\n\nI`ll quickly go through the patches regarding this (not being of much \nhelp for the code itself at the moment, sorry!).\n\nRegards, Buga\n"},{"id":"341151","messageId":"a6f70938-0834-8109-6b94-d93639bb6d0c@gmail.com","threadId":"47873","inReplyTo":"20180306101750.18794-2-phillip.wood@talktalk.net","subject":"Re: [PATCH v2 1/3] add -p: select individual hunk lines","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2018-03-06T20:29:36Z","receivedAt":"2018-03-06T20:29:53Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"On 06/03/2018 11:17, Phillip Wood wrote:\n> From: Phillip Wood \n> \n> When I end up editing hunks it is almost always because I want to\n> stage a subset of the lines in the hunk. Doing this by editing the\n> hunk is inconvenient and error prone (especially so if the patch is\n> going to be reversed before being applied). Instead offer an option\n> for add -p to stage individual lines. When the user presses 'l' the\n> hunk is redrawn with labels by the insertions and deletions and they\n> are prompted to enter a list of the lines they wish to stage. Ranges\n> of lines may be specified using 'a-b' where either 'a' or 'b' may be\n> omitted to mean all lines from 'a' to the end of the hunk or all lines\n> from 1 upto and including 'b'.\n> \n> Signed-off-by: Phillip Wood \n> ---\n> Documentation/git-add.txt | 7 +++\n> git-add--interactive.perl | 143 +++++++++++++++++++++++++++++++++++++++++++++\n> t/t3701-add-interactive.sh | 65 +++++++++++++++++++++\n> 3 files changed, 215 insertions(+)\n> \n> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\n> index d50fa339dc..ad33fda9a2 100644\n> --- a/Documentation/git-add.txt\n> +++ b/Documentation/git-add.txt\n> @@ -332,10 +332,17 @@ patch::\n> J - leave this hunk undecided, see next hunk\n> k - leave this hunk undecided, see previous undecided hunk\n> K - leave this hunk undecided, see previous hunk\n> + l - select hunk lines to use\n\nMight be more surrounding context aligned to say \"stage hunk lines\" \nhere (phrase \"stage\", instead of \"select to use\").\n\n> s - split the current hunk into smaller hunks\n> e - manually edit the current hunk\n> ? - print help\n> +\n> +If you press \"l\" then the hunk will be reprinted with each insertion\n> +or deletion labelled with a number and you will be prompted to enter\n> +which lines you wish to select. Individual line numbers should be\n\nLikewise, s/you wish to select/you wish to stage/.\n\n> +separated by a space or comma, to specify a range of lines use a dash\n> +between them.\n> ++\n> After deciding the fate for all hunks, if there is any hunk\n> that was chosen, the index is updated with the selected hunks.\n> +\n> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> index f83e7450ad..a273b41e95 100755\n> --- a/git-add--interactive.perl\n> +++ b/git-add--interactive.perl\n> @@ -1013,6 +1013,133 @@ sub color_diff {\n> \t} @_;\n> }\n> \n> +sub label_hunk_lines {\n> +\tlocal $_;\n> +\tmy $hunk = shift;\n> +\tmy $i = 0;\n> +\tmy $labels = [ map { /^[-+]/ ? ++$i : 0 } @{$hunk->{TEXT}} ];\n> +\tif ($i > 1) {\n> +\t\t@{$hunk}{qw(LABELS MAX_LABEL)} = ($labels, $i);\n> +\t\treturn 1;\n> +\t}\n> +\treturn 0;\n> +}\n> +\n> +sub select_hunk_lines {\n\nThis is just something I`ve spotted, but I have no actual idea if \nrenaming this to \"stage_hunk_lines\" might be better, too, or not \n(depending on the surrounding code context), so please take this with \na big grain of salt.\n\n> +\tmy ($hunk, $selected) = @_;\n> +\tmy ($text, $labels) = @{$hunk}{qw(TEXT LABELS)};\n> +\tmy ($i, $o_cnt, $n_cnt) = (0, 0, 0);\n> +\tmy ($push_eol, @newtext);\n> +\t# Lines with this mode will become context lines if they are\n> +\t# not selected\n> +\tmy $context_mode = $patch_mode_flavour{IS_REVERSE} ? '+' : '-';\n> +\tfor $i (1..$#{$text}) {\n> +\t\tmy $mode = substr($text->[$i], 0, 1);\n> +\t\tif ($mode eq '\\\\') {\n> +\t\t\tpush @newtext, $text->[$i] if ($push_eol);\n> +\t\t\tundef $push_eol;\n> +\t\t} elsif ($labels->[$i] and $selected->[$labels->[$i]]) {\n> +\t\t\tpush @newtext, $text->[$i];\n> +\t\t\tif ($mode eq '+') {\n> +\t\t\t\t$n_cnt++;\n> +\t\t\t} else {\n> +\t\t\t\t$o_cnt++;\n> +\t\t\t}\n> +\t\t\t$push_eol = 1;\n> +\t\t} elsif ($mode eq ' ' or $mode eq $context_mode) {\n> +\t\t\tpush @newtext, ' ' . substr($text->[$i], 1);\n> +\t\t\t$o_cnt++; $n_cnt++;\n> +\t\t\t$push_eol = 1;\n> +\t\t} else {\n> +\t\t\tundef $push_eol;\n> +\t\t}\n> +\t}\n> +\tmy ($o_ofs, $orig_o_cnt, $n_ofs, $orig_n_cnt) =\n> +\t\t\t\t\tparse_hunk_header($text->[0]);\n> +\tunshift @newtext, format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n> +\tmy $newhunk = {\n> +\t\tTEXT => \\@newtext,\n> +\t\tDISPLAY => [ color_diff(@newtext) ],\n> +\t\tOFS_DELTA => $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt,\n> +\t\tTYPE => $hunk->{TYPE},\n> +\t\tUSE => 1,\n> +\t};\n> +\t# If this hunk has previously been edited add the offset delta\n> +\t# of the old hunk to get the real delta from the original\n> +\t# hunk.\n> +\tif ($hunk->{OFS_DELTA}) {\n> +\t\t$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n> +\t}\n> +\treturn $newhunk;\n> +}\n> +\n> +sub check_hunk_label {\n> +\tmy ($max_label, $label) = ($_[0]->{MAX_LABEL}, $_[1]);\n> +\tif ($label < 1 or $label > $max_label) {\n> +\t\terror_msg sprintf(__(\"invalid hunk line '%d'\\n\"), $label);\n> +\t\treturn 0;\n> +\t}\n> +\treturn 1;\n> +}\n> +\n> +sub parse_hunk_selection {\n> +\tlocal $_;\n> +\tmy ($hunk, $line) = @_;\n> +\tmy $max_label = $hunk->{MAX_LABEL};\n> +\tmy @selected = (0) x ($max_label + 1);\n> +\tmy @fields = split(/[,\\s]+/, $line);\n> +\tfor (@fields) {\n> +\t\tif (/^([0-9]*)-([0-9]*)$/) {\n> +\t\t\tif ($1 eq '' and $2 eq '') {\n> +\t\t\t\terror_msg __(\"range '-' missing upper or lower bound\\n\");\n> +\t\t\t\treturn undef;\n> +\t\t\t}\n> +\t\t\tmy $lo = $1 eq '' ? 1 : $1;\n> +\t\t\tmy $hi = $2 eq '' ? $max_label : $2;\n> +\t\t\tcheck_hunk_label($hunk, $lo) or return undef;\n> +\t\t\tcheck_hunk_label($hunk, $hi) or return undef;\n> +\t\t\tif ($hi < $lo) {\n> +\t\t\t\t($lo, $hi) = ($hi, $lo);\n> +\t\t\t}\n> +\t\t\t@selected[$lo..$hi] = (1) x (1 + $hi - $lo);\n> +\t\t} elsif (/^([0-9]+)$/) {\n> +\t\t\tcheck_hunk_label($hunk, $1) or return undef;\n> +\t\t\t$selected[$1] = 1;\n> +\t\t} else {\n> +\t\t\terror_msg sprintf(__(\"invalid hunk line '%s'\\n\"), $_);\n> +\t\t\treturn undef;\n> +\t\t}\n> +\t}\n> +\treturn \\@selected;\n> +}\n> +\n> +sub display_hunk_lines {\n> +\tmy ($display, $labels, $max_label) =\n> +\t\t\t\t@{$_[0]}{qw(DISPLAY LABELS MAX_LABEL)};\n> +\tmy $width = int(log($max_label) / log(10)) + 1;\n> +\tmy $padding = ' ' x ($width + 1);\n> +\tfor my $i (0..$#{$display}) {\n> +\t\tif ($labels->[$i]) {\n> +\t\t\tprintf '%*d %s', $width, $labels->[$i], $display->[$i];\n> +\t\t} else {\n> +\t\t\tprint $padding . $display->[$i];\n> +\t\t}\n> +\t}\n> +}\n> +\n> +sub select_lines_loop {\n> +\tmy $hunk = shift;\n> +\tdisplay_hunk_lines($hunk);\n> +\tmy $selection = undef;\n> +\tuntil (defined $selection) {\n> +\t\tprint colored $prompt_color, __(\"select lines? \");\n> +\t\tmy $text = ;\n> +\t\tdefined $text and $text =~ /\\S/ or return undef;\n> +\t\t$selection = parse_hunk_selection($hunk, $text);\n> +\t}\n> +\treturn select_hunk_lines($hunk, $selection);\n> +}\n> +\n> my %edit_hunk_manually_modes = (\n> \tstage => N__(\n> \"If the patch applies cleanly, the edited hunk will immediately be\n> @@ -1255,6 +1382,7 @@ j - leave this hunk undecided, see next undecided hunk\n> J - leave this hunk undecided, see next hunk\n> k - leave this hunk undecided, see previous undecided hunk\n> K - leave this hunk undecided, see previous hunk\n> +l - select hunk lines to use\n\ns/select hunk lines to use/stage hunk lines/\n\n> s - split the current hunk into smaller hunks\n> e - manually edit the current hunk\n> ? - print help\n> @@ -1471,6 +1599,9 @@ sub patch_update_file {\n> \t\tif ($hunk[$ix]{TYPE} eq 'hunk') {\n> \t\t\t$other .= ',e';\n> \t\t}\n> +\t\tif (label_hunk_lines($hunk[$ix])) {\n> +\t\t\t$other .= ',l';\n> +\t\t}\n> \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n> \t\t\tprint;\n> \t\t}\n> @@ -1610,6 +1741,18 @@ sub patch_update_file {\n> \t\t\t\t\tnext;\n> \t\t\t\t}\n> \t\t\t}\n> +\t\t\telsif ($line =~ /^l/) {\n> +\t\t\t\tunless ($other =~ /l/) {\n> +\t\t\t\t\terror_msg __(\"Cannot select line by line\\n\");\n> +\t\t\t\t\tnext;\n> +\t\t\t\t}\n> +\t\t\t\tmy $newhunk = select_lines_loop($hunk[$ix]);\n> +\t\t\t\tif ($newhunk) {\n> +\t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n> +\t\t\t\t} else {\n> +\t\t\t\t\tnext;\n> +\t\t\t\t}\n> +\t\t\t}\n> \t\t\telsif ($other =~ /s/ && $line =~ /^s/) {\n> \t\t\t\tmy @split = split_hunk($hunk[$ix]{TEXT}, $hunk[$ix]{DISPLAY});\n> \t\t\t\tif (1 < @split) {\n> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n> index a9a9478a29..65c8c3354b 100755\n> --- a/t/t3701-add-interactive.sh\n> +++ b/t/t3701-add-interactive.sh\n> @@ -360,6 +360,63 @@ test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n> \t! grep \"^+31\" actual\n> '\n> \n> +test_expect_success 'setup expected diff' '\n> +\tcat >expected <<-\\EOF\n> +\tdiff --git a/test b/test\n> +\tindex 0889435..341cc6b 100644\n> +\t--- a/test\n> +\t+++ b/test\n> +\t@@ -1,6 +1,9 @@\n> +\t+5\n> +\t 10\n> +\t 20\n> +\t+21\n> +\t 30\n> +\t 40\n> +\t 50\n> +\t 60\n> +\t+61\n> +\t\\ No newline at end of file\n> +\tEOF\n> +'\n> +\n> +test_expect_success 'can stage individual lines of patch' '\n\nHere, you`re actually using \"stage lines\" yourself, good ;)\n\n> +\tgit reset &&\n> +\tprintf 61 >>test &&\n> +\tprintf \"%s\\n\" l \"-2 4\" |\n> +\tEDITOR=: git add -p 2>error &&\n> +\ttest_must_be_empty error &&\n> +\tgit diff --cached HEAD >actual &&\n> +\tdiff_cmp expected actual\n> +'\n> +\n> +test_expect_success 'setup expected diff' '\n> +\tcat >expected <<-\\EOF\n> +\tdiff --git a/test b/test\n> +\tindex 0889435..cc6163b 100644\n> +\t--- a/test\n> +\t+++ b/test\n> +\t@@ -1,6 +1,8 @@\n> +\t+5\n> +\t 10\n> +\t 20\n> +\t 30\n> +\t 40\n> +\t 50\n> +\t 60\n> +\t+61\n> +\t\\ No newline at end of file\n> +\tEOF\n> +'\n> +\n> +test_expect_success 'can reset individual lines of patch' '\n> +\tprintf \"%s\\n\" l 2 |\n> +\tEDITOR=: git reset -p 2>error &&\n> +\ttest_must_be_empty error &&\n> +\tgit diff --cached HEAD >actual &&\n> +\tdiff_cmp expected actual\n> +'\n> +\n> test_expect_success 'patch mode ignores unmerged entries' '\n> \tgit reset --hard &&\n> \ttest_commit conflict &&\n> @@ -576,4 +633,12 @@ test_expect_success 'add -p patch editing works with pathological context lines'\n> \ttest_cmp expected-2 actual\n> '\n> \n> +test_expect_success 'add -p selecting lines works with pathological context lines' '\n\nMaybe s/selecting lines/staging lines/ ?\n\n> +\tgit reset &&\n> +\tprintf \"%s\\n\" l 2 y |\n> +\tGIT_EDITOR=./editor git add -p &&\n> +\tgit cat-file blob :a >actual &&\n> +\ttest_cmp expected-2 actual\n> +'\n> +\n> test_done\n> \n"},{"id":"341152","messageId":"caf6cd4a-d45f-1a8d-428e-abf9c623a1ef@gmail.com","threadId":"47873","inReplyTo":"20180306101750.18794-4-phillip.wood@talktalk.net","subject":"Re: [PATCH v2 3/3] add -p: optimize line selection for short hunks","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2018-03-06T20:33:48Z","receivedAt":"2018-03-06T20:34:04Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"On 06/03/2018 11:17, Phillip Wood wrote:\n> From: Phillip Wood \n> \n> If there are fewer than ten changes in a hunk then make spaces\n> optional when selecting individual lines. This means that for short\n\nNot sure if using s/selecting individual lines/staging individual lines/ \nwould make sense here, too, but not that important (as you later do \nsay \"to stage lines\").\n\n> hunks one can just type -357 to stage lines 1, 2, 3, 5 & 7.\n> \n> Signed-off-by: Phillip Wood \n> ---\n> Documentation/git-add.txt | 3 ++-\n> git-add--interactive.perl | 30 ++++++++++++++++++++++++++++++\n> t/t3701-add-interactive.sh | 2 +-\n> 3 files changed, 33 insertions(+), 2 deletions(-)\n> \n> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\n> index 0e2c11e97b..d52acfc722 100644\n> --- a/Documentation/git-add.txt\n> +++ b/Documentation/git-add.txt\n> @@ -340,7 +340,8 @@ patch::\n> If you press \"l\" then the hunk will be reprinted with each insertion\n> or deletion labelled with a number and you will be prompted to enter\n> which lines you wish to select. Individual line numbers should be\n> -separated by a space or comma, to specify a range of lines use a dash\n> +separated by a space or comma (these can be omitted if there are fewer\n> +than ten labelled lines), to specify a range of lines use a dash\n> between them. To invert the selection prefix it with \"\\^\" so \"^3-5,8\"\n> will select everything except lines 3, 4, 5 and 8.\n> +\n> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> index 6fa3d0a87c..9a6bcd5085 100755\n> --- a/git-add--interactive.perl\n> +++ b/git-add--interactive.perl\n> @@ -1082,6 +1082,33 @@ sub check_hunk_label {\n> \treturn 1;\n> }\n> \n> +sub split_hunk_selection {\n> +\tlocal $_;\n> +\tmy @fields = @_;\n> +\tmy @ret;\n> +\tfor (@fields) {\n> +\t\tif (/^(-[0-9])(.*)/) {\n> +\t\t\tpush @ret, $1;\n> +\t\t\t$_ = $2;\n> +\t\t}\n> +\t\twhile ($_ ne '') {\n> +\t\t\tif (/^[0-9]-$/) {\n> +\t\t\t\tpush @ret, $_;\n> +\t\t\t\tlast;\n> +\t\t\t} elsif (/^([0-9](?:-[0-9])?)(.*)/) {\n> +\t\t\t\tpush @ret, $1;\n> +\t\t\t\t$_ = $2;\n> +\t\t\t} else {\n> +\t\t\t\terror_msg sprintf\n> +\t\t\t\t __(\"invalid hunk line '%s'\\n\"),\n> +\t\t\t\t substr($_, 0, 1);\n> +\t\t\t\treturn ();\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> +\treturn @ret;\n> +}\n> +\n> sub parse_hunk_selection {\n> \tlocal $_;\n> \tmy ($hunk, $line) = @_;\n> @@ -1100,6 +1127,9 @@ sub parse_hunk_selection {\n> \t\t\t}\n> \t\t}\n> \t}\n> +\tif ($max_label < 10) {\n> +\t\t@fields = split_hunk_selection(@fields) or return undef;\n> +\t}\n> \tfor (@fields) {\n> \t\tif (/^([0-9]*)-([0-9]*)$/) {\n> \t\t\tif ($1 eq '' and $2 eq '') {\n> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n> index 89c0e73f2b..d3bce154da 100755\n> --- a/t/t3701-add-interactive.sh\n> +++ b/t/t3701-add-interactive.sh\n> @@ -410,7 +410,7 @@ test_expect_success 'setup expected diff' '\n> '\n> \n> test_expect_success 'can reset individual lines of patch' '\n> -\tprintf \"%s\\n\" l \"^1 3\" |\n> +\tprintf \"%s\\n\" l ^13 |\n> \tEDITOR=: git reset -p 2>error &&\n> \ttest_must_be_empty error &&\n> \tgit diff --cached HEAD >actual &&\n> \n"},{"id":"341153","messageId":"4ce95d01-5cda-44f6-0504-f06ffe217032@gmail.com","threadId":"47873","inReplyTo":"20180306101750.18794-3-phillip.wood@talktalk.net","subject":"Re: [PATCH v2 2/3] add -p: allow line selection to be inverted","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2018-03-06T20:41:00Z","receivedAt":"2018-03-06T20:41:17Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"On 06/03/2018 11:17, Phillip Wood wrote:\n> From: Phillip Wood \n> \n> If the list of lines to be selected begins with '^' select all the\n> lines except the ones listed.\n\ns/to be selected begins with '^' select all/to be staged begins with '^' stage all/\n\n> \n> Signed-off-by: Phillip Wood \n> ---\n> Documentation/git-add.txt | 3 ++-\n> git-add--interactive.perl | 17 ++++++++++++++++-\n> t/t3701-add-interactive.sh | 2 +-\n> 3 files changed, 19 insertions(+), 3 deletions(-)\n> \n> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\n> index ad33fda9a2..0e2c11e97b 100644\n> --- a/Documentation/git-add.txt\n> +++ b/Documentation/git-add.txt\n> @@ -341,7 +341,8 @@ If you press \"l\" then the hunk will be reprinted with each insertion\n> or deletion labelled with a number and you will be prompted to enter\n> which lines you wish to select. Individual line numbers should be\n> separated by a space or comma, to specify a range of lines use a dash\n> -between them.\n> +between them. To invert the selection prefix it with \"\\^\" so \"^3-5,8\"\n> +will select everything except lines 3, 4, 5 and 8.\n\nHmm, here, first \"selection\" seems to make sense as it is (I guess),\nbut might still be better to later say \ns/will select everything/will stage everything/ ...?\n\nThat said, might be \"to invert the selection\" could rather be \"to unstage,\" \ninstead? Not sure, though.\n\n> +\n> After deciding the fate for all hunks, if there is any hunk\n> that was chosen, the index is updated with the selected hunks.\n> diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> index a273b41e95..6fa3d0a87c 100755\n> --- a/git-add--interactive.perl\n> +++ b/git-add--interactive.perl\n> @@ -1085,9 +1085,21 @@ sub check_hunk_label {\n> sub parse_hunk_selection {\n> \tlocal $_;\n> \tmy ($hunk, $line) = @_;\n> -\tmy $max_label = $hunk->{MAX_LABEL};\n> +\tmy ($max_label, $invert) = ($hunk->{MAX_LABEL}, undef);\n> \tmy @selected = (0) x ($max_label + 1);\n> \tmy @fields = split(/[,\\s]+/, $line);\n> +\tif ($fields[0] =~ /^\\^(.*)/) {\n> +\t\t$invert = 1;\n> +\t\tif ($1 ne '') {\n> +\t\t\t$fields[0] = $1;\n> +\t\t} else {\n> +\t\t\tshift @fields;\n> +\t\t\tunless (@fields) {\n> +\t\t\t\terror_msg __(\"no lines to invert\\n\");\n> +\t\t\t\treturn undef;\n> +\t\t\t}\n> +\t\t}\n> +\t}\n> \tfor (@fields) {\n> \t\tif (/^([0-9]*)-([0-9]*)$/) {\n> \t\t\tif ($1 eq '' and $2 eq '') {\n> @@ -1110,6 +1122,9 @@ sub parse_hunk_selection {\n> \t\t\treturn undef;\n> \t\t}\n> \t}\n> +\tif ($invert) {\n> +\t\t@selected = map { !$_ } @selected;\n> +\t}\n> \treturn \\@selected;\n> }\n> \n> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\n> index 65c8c3354b..89c0e73f2b 100755\n> --- a/t/t3701-add-interactive.sh\n> +++ b/t/t3701-add-interactive.sh\n> @@ -410,7 +410,7 @@ test_expect_success 'setup expected diff' '\n> '\n> \n> test_expect_success 'can reset individual lines of patch' '\n> -\tprintf \"%s\\n\" l 2 |\n> +\tprintf \"%s\\n\" l \"^1 3\" |\n> \tEDITOR=: git reset -p 2>error &&\n> \ttest_must_be_empty error &&\n> \tgit diff --cached HEAD >actual &&\n> \n"},{"id":"341157","messageId":"xmqqr2ow3ojb.fsf@gitster-ct.c.googlers.com","threadId":"47873","inReplyTo":"64005dad-ec23-892c-4270-b01cae74f663@gmail.com","subject":"Re: [PATCH v2 0/3] add -p: select individual hunk lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-06T21:03:04Z","receivedAt":"2018-03-06T21:03:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Igor Djordjevic writes:\n\n> A small nitpick - I see you use phrasing like \"select lines\", where \n> the other commands usually talk about \"staging\", instead, so \"stage \n> lines\" might be more aligned with the existing text.\n\nIsn't this machinery shared across \"add -p\" and \"reset -p\"? What is\ndone to the selected lines when you are using this UI while running\n\"reset -p\"? I hope it is not \"staging\". If the interface only\n\"selects lines\" and what is done to the selected lines depends on\nwhat operation is using this backend, then the current phrasing is\nperfectly fine and saying \"staging\" makes it actively worse.\n\n"},{"id":"341160","messageId":"9e46f6c7-952f-c4d3-6a12-40c59baaa702@gmail.com","threadId":"47873","inReplyTo":"xmqqr2ow3ojb.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 0/3] add -p: select individual hunk lines","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2018-03-06T21:20:16Z","receivedAt":"2018-03-06T21:20:32Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"Hi Junio,\n\nOn 06/03/2018 22:03, Junio C Hamano wrote:\n> \n> > A small nitpick - I see you use phrasing like \"select lines\", where \n> > the other commands usually talk about \"staging\", instead, so \"stage \n> > lines\" might be more aligned with the existing text.\n> \n> Isn't this machinery shared across \"add -p\" and \"reset -p\"? What is\n> done to the selected lines when you are using this UI while running\n> \"reset -p\"? I hope it is not \"staging\". If the interface only\n> \"selects lines\" and what is done to the selected lines depends on\n> what operation is using this backend, then the current phrasing is\n> perfectly fine and saying \"staging\" makes it actively worse.\n\nHmm, if that is the case, I agree, but I was merely trying to review \nthe files being changed - for example, inside \"Documentation/git-add.txt\":\n\n y - stage this hunk\n n - do not stage this hunk\n q - quit; do not stage this hunk or any of the remaining ones\n a - stage this hunk and all later hunks in the file\n d - do not stage this hunk or any of the later hunks in the file\n g - select a hunk to go to\n / - search for a hunk matching the given regex\n j - leave this hunk undecided, see next undecided hunk\n J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n s - split the current hunk into smaller hunks\n e - manually edit the current hunk\n ? - print help\n\n\nIn there, adding \"l\" should follow \"stage\" phrasing, I would think.\n\nBut you are right for \"git-add--interactive.perl\", for example - in \nthere, I didn`t notice the line (seems to be?) added inside the shared \n\"help_patch_cmd\".\n\nBut if so, I guess it should then be moved to more context-related \n\"help_patch_modes\", being phrased accordingly in there.\n\nThanks for pointing this out, let me recheck my comments.\n\nRegards, Buga\n"},{"id":"341162","messageId":"4328de01-f3e7-7cc4-5ed2-8b90da2fa94e@gmail.com","threadId":"47873","inReplyTo":"a6f70938-0834-8109-6b94-d93639bb6d0c@gmail.com","subject":"Re: [PATCH v2 1/3] add -p: select individual hunk lines","fromName":"Igor Djordjevic","fromEmail":"igor.d.djordjevic@gmail.com","sentAt":"2018-03-06T21:33:00Z","receivedAt":"2018-03-06T21:33:15Z","isPatch":true,"sender":{"key":"igor.d.djordjevic@gmail.com","avatar":null},"body":"On 06/03/2018 21:29, Igor Djordjevic wrote:\n> \n> > diff --git a/git-add--interactive.perl b/git-add--interactive.perl\n> > index f83e7450ad..a273b41e95 100755\n> > --- a/git-add--interactive.perl\n> > +++ b/git-add--interactive.perl\n> > \n> > [...]\n> > \n> > @@ -1255,6 +1382,7 @@ j - leave this hunk undecided, see next undecided hunk\n> > J - leave this hunk undecided, see next hunk\n> > k - leave this hunk undecided, see previous undecided hunk\n> > K - leave this hunk undecided, see previous hunk\n> > +l - select hunk lines to use\n> \n> s/select hunk lines to use/stage hunk lines/\n\nI was wrong here - in the context of Junio`s remark, I now think this \nmight even belong to context-aware \"help_patch_modes\" instead, \nphrased accordingly in there (stage/stash/unstage... etc.).\n"},{"id":"341266","messageId":"6476d776-dbf7-09cf-1c65-e413798b9987@talktalk.net","threadId":"47873","inReplyTo":"xmqq371d3rjz.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 2/3] add -p: allow line selection to be inverted","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-08T11:05:52Z","receivedAt":"2018-03-08T11:06:01Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 06/03/18 19:57, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> From: Phillip Wood \n>>\n>> If the list of lines to be selected begins with '^' select all the\n>> lines except the ones listed.\n> \n> There is \"# Input that begins with '-'; unchoose\" in list_and_choose\n> helper. Does it make things inconsistent to use '^' for negation\n> like this patch does with it?\n> \nHmm yes, I think it probably does (I've just checked and git clean also\nuses '-' for de-selection). I think I'll remove support for open-ended\nranges on the left side (it's not so hard to type '1-n' instead of '-n')\nand use a leading '-' for inversion. I'm tempted to keep supporting 'n-'\nto mean everything from 'n' to the last line though.\n\nBest Wishes\n\nPhillip\n"},{"id":"341304","messageId":"xmqq371axxm5.fsf@gitster-ct.c.googlers.com","threadId":"47873","inReplyTo":"6476d776-dbf7-09cf-1c65-e413798b9987@talktalk.net","subject":"Re: [PATCH v2 2/3] add -p: allow line selection to be inverted","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-08T17:53:22Z","receivedAt":"2018-03-08T17:53:30Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> and use a leading '-' for inversion. I'm tempted to keep supporting 'n-'\n> to mean everything from 'n' to the last line though.\n\nThanks for double checking. It would be a better endgame to follow\nup with an update to existing \"range selection\" code to also support\n\"n-\", if you go that route.\n\n"},{"id":"341537","messageId":"ea4f645f-5cf2-ef84-24cf-fb585f96039d@talktalk.net","threadId":"47873","inReplyTo":"xmqq371axxm5.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 2/3] add -p: allow line selection to be inverted","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-13T12:06:11Z","receivedAt":"2018-03-13T12:12:03Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 08/03/18 17:53, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> and use a leading '-' for inversion. I'm tempted to keep supporting 'n-'\n>> to mean everything from 'n' to the last line though.\n> \n> Thanks for double checking. It would be a better endgame to follow\n> up with an update to existing \"range selection\" code to also support\n> \"n-\", if you go that route.\n> \nI'm afraid I'm not sure exactly what you're suggesting. At the moment\nthe range selection code is in the first patch and supports incomplete\nranges. Are you suggesting that support for incomplete ranges should be\nin a separate patch or have I misunderstood?\n\nThanks\n\nPhillip\n"},{"id":"341547","messageId":"xmqqsh942ayf.fsf@gitster-ct.c.googlers.com","threadId":"47873","inReplyTo":"ea4f645f-5cf2-ef84-24cf-fb585f96039d@talktalk.net","subject":"Re: [PATCH v2 2/3] add -p: allow line selection to be inverted","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-13T16:32:08Z","receivedAt":"2018-03-13T16:32:18Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> On 08/03/18 17:53, Junio C Hamano wrote:\n>> Phillip Wood writes:\n>> \n>>> and use a leading '-' for inversion. I'm tempted to keep supporting 'n-'\n>>> to mean everything from 'n' to the last line though.\n>> \n>> Thanks for double checking. It would be a better endgame to follow\n>> up with an update to existing \"range selection\" code to also support\n>> \"n-\", if you go that route.\n>> \n> I'm afraid I'm not sure exactly what you're suggesting. At the moment\n> the range selection code is in the first patch and supports incomplete\n> ranges. Are you suggesting that support for incomplete ranges should be\n> in a separate patch or have I misunderstood?\n\nMy observation of the situation behind my reasoning is:\n\n - There is an existing UI that uses \"-X\" to mean \"exclude what\n matches X\" and that was the reason why you decided to follow suit\n instead of using \"^X\" for inversion of X.\n\n - Such an existing UI would not have used \"-X\" to mean \"the first\n possible choice thru X\". You will lose that from your new thing\n and you accepted that.\n\n - It is likely (I did not check, though) that the existing UI would\n not have used \"Y-\" to mean \"starting from Y all the possible\n choices thru to the end\", but that is merely for symmetry with\n the lack (inability to use) of \"-X\". There is no fundamental\n reason why \"Y-\" cannot mean that, and you are tempted to allow do\n so in your new thing for the same reason.\n\nSo if we are going to have \"N-\" to mean \"everything from N to the\nlast line\", then the same \"Starting at N to the end of the all the\npossible choices\" should be allowed in the existing UI (i.e. the one\nthat forced you to give up \"^X\" for the sake of consistency) for the\nsame consistency reasons, no?\n\nFor that, if you want to keep the \"n-\" you did in your first patch,\nthe most logical thing is to have a preparatory enhancement to teach\n\"N-\" to list_and_choose(), and then build your series on top. Or\nyou can do without such a change to list_and_choose() in your series,\nin which case, you drop \"n-\" support and then at the very end after\nthe series settles, add \"n-\" support to the new code in this series\nand to list_and_choose() at the same time in a follow-up patch.\n\n\n"},{"id":"341627","messageId":"59492efe-f455-1214-86e4-7a48bbe8f556@talktalk.net","threadId":"47873","inReplyTo":"xmqqsh942ayf.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v2 2/3] add -p: allow line selection to be inverted","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-14T11:02:58Z","receivedAt":"2018-03-14T11:03:07Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 13/03/18 16:32, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> On 08/03/18 17:53, Junio C Hamano wrote:\n>>> Phillip Wood writes:\n>>>\n>>>> and use a leading '-' for inversion. I'm tempted to keep supporting 'n-'\n>>>> to mean everything from 'n' to the last line though.\n>>>\n>>> Thanks for double checking. It would be a better endgame to follow\n>>> up with an update to existing \"range selection\" code to also support\n>>> \"n-\", if you go that route.\n>>>\n>> I'm afraid I'm not sure exactly what you're suggesting. At the moment\n>> the range selection code is in the first patch and supports incomplete\n>> ranges. Are you suggesting that support for incomplete ranges should be\n>> in a separate patch or have I misunderstood?\n> \n> My observation of the situation behind my reasoning is:\n> \n> - There is an existing UI that uses \"-X\" to mean \"exclude what\n> matches X\" and that was the reason why you decided to follow suit\n> instead of using \"^X\" for inversion of X.\n> \n> - Such an existing UI would not have used \"-X\" to mean \"the first\n> possible choice thru X\". You will lose that from your new thing\n> and you accepted that.\n> \n> - It is likely (I did not check, though) that the existing UI would\n> not have used \"Y-\" to mean \"starting from Y all the possible\n> choices thru to the end\", but that is merely for symmetry with\n> the lack (inability to use) of \"-X\". There is no fundamental\n> reason why \"Y-\" cannot mean that, and you are tempted to allow do\n> so in your new thing for the same reason.\n> \n> So if we are going to have \"N-\" to mean \"everything from N to the\n> last line\", then the same \"Starting at N to the end of the all the\n> possible choices\" should be allowed in the existing UI (i.e. the one\n> that forced you to give up \"^X\" for the sake of consistency) for the\n> same consistency reasons, no?\n> \n> For that, if you want to keep the \"n-\" you did in your first patch,\n> the most logical thing is to have a preparatory enhancement to teach\n> \"N-\" to list_and_choose(), and then build your series on top. Or\n> you can do without such a change to list_and_choose() in your series,\n> in which case, you drop \"n-\" support and then at the very end after\n> the series settles, add \"n-\" support to the new code in this series\n> and to list_and_choose() at the same time in a follow-up patch.\n> \n> \nThanks for taking the time to clarify that, I hadn't twigged you were\ntalking about changing list_and_choose() before. I think it would make\nsense for that and 'git clean' to match the line selection with \"n-\". I\njust opened it up my editor to do that and was pleasantly surprised to\ndiscover that \"n-\" is already implemented for both list_and_choose() and\n'git clean' so there's nothing that needs doing on that front. I'll\nreroll this series with the other changes and send it later this week or\nnext week\n\nBest Wishes\n\nPhillip\n"},{"id":"341864","messageId":"20180316101346.7137-2-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180316101346.7137-1-phillip.wood@talktalk.net","subject":"[PATCH v3 1/3] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-16T10:13:44Z","receivedAt":"2018-03-16T10:14:05Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen I end up editing hunks it is almost always because I want to\nstage a subset of the lines in the hunk. Doing this by editing the\nhunk is inconvenient and error prone (especially so if the patch is\ngoing to be reversed before being applied). Instead offer an option\nfor add -p to stage individual lines. When the user presses 'l' the\nhunk is redrawn with labels by the insertions and deletions and they\nare prompted to enter a list of the lines they wish to stage. Ranges\nof lines may be specified using 'a-b' where 'b' may be omitted to mean\nall lines from 'a' to the end of the hunk.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v2:\n - remove support for omitting the lower bound of a range of lines as\n this clashes with using '-' for inverting the selection.\n - document that the upper bound can be omitted.\n\n Documentation/git-add.txt | 8 +++\n git-add--interactive.perl | 140 +++++++++++++++++++++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 65 +++++++++++++++++++++\n 3 files changed, 213 insertions(+)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex d50fa339dc..965e192a09 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -332,10 +332,18 @@ patch::\n J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n+ l - select hunk lines to use\n s - split the current hunk into smaller hunks\n e - manually edit the current hunk\n ? - print help\n +\n+If you press \"l\" then the hunk will be reprinted with each insertion or\n+deletion labelled with a number and you will be prompted to enter which\n+lines you wish to select. Individual line numbers should be separated by\n+a space or comma, to specify a range of lines use a dash between\n+them. If the upper bound of a range of lines is omitted it defaults to\n+the last line.\n++\n After deciding the fate for all hunks, if there is any hunk\n that was chosen, the index is updated with the selected hunks.\n +\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex f83e7450ad..712226b34c 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1013,6 +1013,130 @@ sub color_diff {\n \t} @_;\n }\n \n+sub label_hunk_lines {\n+\tlocal $_;\n+\tmy $hunk = shift;\n+\tmy $i = 0;\n+\tmy $labels = [ map { /^[-+]/ ? ++$i : 0 } @{$hunk->{TEXT}} ];\n+\tif ($i > 1) {\n+\t\t@{$hunk}{qw(LABELS MAX_LABEL)} = ($labels, $i);\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+sub select_hunk_lines {\n+\tmy ($hunk, $selected) = @_;\n+\tmy ($text, $labels) = @{$hunk}{qw(TEXT LABELS)};\n+\tmy ($i, $o_cnt, $n_cnt) = (0, 0, 0);\n+\tmy ($push_eol, @newtext);\n+\t# Lines with this mode will become context lines if they are\n+\t# not selected\n+\tmy $context_mode = $patch_mode_flavour{IS_REVERSE} ? '+' : '-';\n+\tfor $i (1..$#{$text}) {\n+\t\tmy $mode = substr($text->[$i], 0, 1);\n+\t\tif ($mode eq '\\\\') {\n+\t\t\tpush @newtext, $text->[$i] if ($push_eol);\n+\t\t\tundef $push_eol;\n+\t\t} elsif ($labels->[$i] and $selected->[$labels->[$i]]) {\n+\t\t\tpush @newtext, $text->[$i];\n+\t\t\tif ($mode eq '+') {\n+\t\t\t\t$n_cnt++;\n+\t\t\t} else {\n+\t\t\t\t$o_cnt++;\n+\t\t\t}\n+\t\t\t$push_eol = 1;\n+\t\t} elsif ($mode eq ' ' or $mode eq $context_mode) {\n+\t\t\tpush @newtext, ' ' . substr($text->[$i], 1);\n+\t\t\t$o_cnt++; $n_cnt++;\n+\t\t\t$push_eol = 1;\n+\t\t} else {\n+\t\t\tundef $push_eol;\n+\t\t}\n+\t}\n+\tmy ($o_ofs, $orig_o_cnt, $n_ofs, $orig_n_cnt) =\n+\t\t\t\t\tparse_hunk_header($text->[0]);\n+\tunshift @newtext, format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n+\tmy $newhunk = {\n+\t\tTEXT => \\@newtext,\n+\t\tDISPLAY => [ color_diff(@newtext) ],\n+\t\tOFS_DELTA => $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt,\n+\t\tTYPE => $hunk->{TYPE},\n+\t\tUSE => 1,\n+\t};\n+\t# If this hunk has previously been edited add the offset delta\n+\t# of the old hunk to get the real delta from the original\n+\t# hunk.\n+\tif ($hunk->{OFS_DELTA}) {\n+\t\t$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n+\t}\n+\treturn $newhunk;\n+}\n+\n+sub check_hunk_label {\n+\tmy ($max_label, $label) = ($_[0]->{MAX_LABEL}, $_[1]);\n+\tif ($label < 1 or $label > $max_label) {\n+\t\terror_msg sprintf(__(\"invalid hunk line '%d'\\n\"), $label);\n+\t\treturn 0;\n+\t}\n+\treturn 1;\n+}\n+\n+sub parse_hunk_selection {\n+\tlocal $_;\n+\tmy ($hunk, $line) = @_;\n+\tmy $max_label = $hunk->{MAX_LABEL};\n+\tmy @selected = (0) x ($max_label + 1);\n+\tmy @fields = split(/[,\\s]+/, $line);\n+\tfor (@fields) {\n+\t\tif (my ($lo, $hi) = /^([0-9]+)-([0-9]*)$/) {\n+\t\t\tif ($hi eq '') {\n+\t\t\t\t$hi = $max_label;\n+\t\t\t}\n+\t\t\tcheck_hunk_label($hunk, $lo) or return undef;\n+\t\t\tcheck_hunk_label($hunk, $hi) or return undef;\n+\t\t\tif ($hi < $lo) {\n+\t\t\t\t($lo, $hi) = ($hi, $lo);\n+\t\t\t}\n+\t\t\t@selected[$lo..$hi] = (1) x (1 + $hi - $lo);\n+\t\t} elsif (/^([0-9]+)$/) {\n+\t\t\tcheck_hunk_label($hunk, $1) or return undef;\n+\t\t\t$selected[$1] = 1;\n+\t\t} else {\n+\t\t\terror_msg sprintf(__(\"invalid hunk line '%s'\\n\"), $_);\n+\t\t\treturn undef;\n+\t\t}\n+\t}\n+\treturn \\@selected;\n+}\n+\n+sub display_hunk_lines {\n+\tmy ($display, $labels, $max_label) =\n+\t\t\t\t@{$_[0]}{qw(DISPLAY LABELS MAX_LABEL)};\n+\tmy $width = int(log($max_label) / log(10)) + 1;\n+\tmy $padding = ' ' x ($width + 1);\n+\tfor my $i (0..$#{$display}) {\n+\t\tif ($labels->[$i]) {\n+\t\t\tprintf '%*d %s', $width, $labels->[$i], $display->[$i];\n+\t\t} else {\n+\t\t\tprint $padding . $display->[$i];\n+\t\t}\n+\t}\n+}\n+\n+sub select_lines_loop {\n+\tmy $hunk = shift;\n+\tdisplay_hunk_lines($hunk);\n+\tmy $selection = undef;\n+\tuntil (defined $selection) {\n+\t\tprint colored $prompt_color, __(\"select lines? \");\n+\t\tmy $text = ;\n+\t\tdefined $text and $text =~ /\\S/ or return undef;\n+\t\t$selection = parse_hunk_selection($hunk, $text);\n+\t}\n+\treturn select_hunk_lines($hunk, $selection);\n+}\n+\n my %edit_hunk_manually_modes = (\n \tstage => N__(\n \"If the patch applies cleanly, the edited hunk will immediately be\n@@ -1255,6 +1379,7 @@ j - leave this hunk undecided, see next undecided hunk\n J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n+l - select hunk lines to use\n s - split the current hunk into smaller hunks\n e - manually edit the current hunk\n ? - print help\n@@ -1471,6 +1596,9 @@ sub patch_update_file {\n \t\tif ($hunk[$ix]{TYPE} eq 'hunk') {\n \t\t\t$other .= ',e';\n \t\t}\n+\t\tif (label_hunk_lines($hunk[$ix])) {\n+\t\t\t$other .= ',l';\n+\t\t}\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -1610,6 +1738,18 @@ sub patch_update_file {\n \t\t\t\t\tnext;\n \t\t\t\t}\n \t\t\t}\n+\t\t\telsif ($line =~ /^l/) {\n+\t\t\t\tunless ($other =~ /l/) {\n+\t\t\t\t\terror_msg __(\"Cannot select line by line\\n\");\n+\t\t\t\t\tnext;\n+\t\t\t\t}\n+\t\t\t\tmy $newhunk = select_lines_loop($hunk[$ix]);\n+\t\t\t\tif ($newhunk) {\n+\t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n+\t\t\t\t} else {\n+\t\t\t\t\tnext;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\telsif ($other =~ /s/ && $line =~ /^s/) {\n \t\t\t\tmy @split = split_hunk($hunk[$ix]{TEXT}, $hunk[$ix]{DISPLAY});\n \t\t\t\tif (1 < @split) {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex a9a9478a29..5f3fcb1758 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -360,6 +360,63 @@ test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n \t! grep \"^+31\" actual\n '\n \n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..341cc6b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,9 @@\n+\t+5\n+\t 10\n+\t 20\n+\t+21\n+\t 30\n+\t 40\n+\t 50\n+\t 60\n+\t+61\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can stage individual lines of patch' '\n+\tgit reset &&\n+\tprintf 61 >>test &&\n+\tprintf \"%s\\n\" l \"1,2 4-\" |\n+\tEDITOR=: git add -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..cc6163b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,8 @@\n+\t+5\n+\t 10\n+\t 20\n+\t 30\n+\t 40\n+\t 50\n+\t 60\n+\t+61\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can reset individual lines of patch' '\n+\tprintf \"%s\\n\" l 2 |\n+\tEDITOR=: git reset -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n test_expect_success 'patch mode ignores unmerged entries' '\n \tgit reset --hard &&\n \ttest_commit conflict &&\n@@ -576,4 +633,12 @@ test_expect_success 'add -p patch editing works with pathological context lines'\n \ttest_cmp expected-2 actual\n '\n \n+test_expect_success 'add -p selecting lines works with pathological context lines' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l 2 y |\n+\tGIT_EDITOR=./editor git add -p &&\n+\tgit cat-file blob :a >actual &&\n+\ttest_cmp expected-2 actual\n+'\n+\n test_done\n-- \n2.16.2\n\n"},{"id":"341865","messageId":"20180316101346.7137-1-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180219113619.26566-1-phillip.wood@talktalk.net","subject":"[PATCH v3 0/3] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-16T10:13:43Z","receivedAt":"2018-03-16T10:14:07Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nSince v2 I've updated the patches to use '-' instead of '^' to invert\nthe selection to match the rest of add -i and clean -i.\n\nThese patches build on top of the recount fixes in [1]. The commit\nmessage for the first patch describes the motivation:\n\n\"When I end up editing hunks it is almost always because I want to\nstage a subset of the lines in the hunk. Doing this by editing the\nhunk is inconvenient and error prone (especially so if the patch is\ngoing to be reversed before being applied). Instead offer an option\nfor add -p to stage individual lines. When the user presses 'l' the\nhunk is redrawn with labels by the insertions and deletions and they\nare prompted to enter a list of the lines they wish to stage. Ranges\nof lines may be specified using 'a-b' where either 'a' or 'b' may be\nomitted to mean all lines from 'a' to the end of the hunk or all lines\nfrom 1 upto and including 'b'.\"\n\n[1] https://public-inbox.org/git/xmqqbmg29x1n.fsf@gitster-ct.c.googlers.com/T/#m01d0f1af90f32b698e583b56f8e53b986bcec7c6\n\nInterdiff to v2:\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex d52acfc722..f3c81dfb11 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -337,13 +337,14 @@ patch::\n e - manually edit the current hunk\n ? - print help\n +\n-If you press \"l\" then the hunk will be reprinted with each insertion\n-or deletion labelled with a number and you will be prompted to enter\n-which lines you wish to select. Individual line numbers should be\n-separated by a space or comma (these can be omitted if there are fewer\n-than ten labelled lines), to specify a range of lines use a dash\n-between them. To invert the selection prefix it with \"\\^\" so \"^3-5,8\"\n-will select everything except lines 3, 4, 5 and 8.\n+If you press \"l\" then the hunk will be reprinted with each insertion or\n+deletion labelled with a number and you will be prompted to enter which\n+lines you wish to select. Individual line numbers should be separated by\n+a space or comma (these can be omitted if there are fewer than ten\n+labelled lines), to specify a range of lines use a dash between them. If\n+the upper bound of a range of lines is omitted it defaults to the last\n+line. To invert the selection prefix it with \"-\" so \"-3-5,8\" will select\n+everything except lines 3, 4, 5 and 8.\n +\n After deciding the fate for all hunks, if there is any hunk\n that was chosen, the index is updated with the selected hunks.\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 9a6bcd5085..d65ad7c26d 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1087,10 +1087,6 @@ sub split_hunk_selection {\n \tmy @fields = @_;\n \tmy @ret;\n \tfor (@fields) {\n-\t\tif (/^(-[0-9])(.*)/) {\n-\t\t\tpush @ret, $1;\n-\t\t\t$_ = $2;\n-\t\t}\n \t\twhile ($_ ne '') {\n \t\t\tif (/^[0-9]-$/) {\n \t\t\t\tpush @ret, $_;\n@@ -1115,7 +1111,7 @@ sub parse_hunk_selection {\n \tmy ($max_label, $invert) = ($hunk->{MAX_LABEL}, undef);\n \tmy @selected = (0) x ($max_label + 1);\n \tmy @fields = split(/[,\\s]+/, $line);\n-\tif ($fields[0] =~ /^\\^(.*)/) {\n+\tif ($fields[0] =~ /^-(.*)/) {\n \t\t$invert = 1;\n \t\tif ($1 ne '') {\n \t\t\t$fields[0] = $1;\n@@ -1131,13 +1127,10 @@ sub parse_hunk_selection {\n \t\t@fields = split_hunk_selection(@fields) or return undef;\n \t}\n \tfor (@fields) {\n-\t\tif (/^([0-9]*)-([0-9]*)$/) {\n-\t\t\tif ($1 eq '' and $2 eq '') {\n-\t\t\t\terror_msg __(\"range '-' missing upper or lower bound\\n\");\n-\t\t\t\treturn undef;\n+\t\tif (my ($lo, $hi) = /^([0-9]+)-([0-9]*)$/) {\n+\t\t\tif ($hi eq '') {\n+\t\t\t\t$hi = $max_label;\n \t\t\t}\n-\t\t\tmy $lo = $1 eq '' ? 1 : $1;\n-\t\t\tmy $hi = $2 eq '' ? $max_label : $2;\n \t\t\tcheck_hunk_label($hunk, $lo) or return undef;\n \t\t\tcheck_hunk_label($hunk, $hi) or return undef;\n \t\t\tif ($hi < $lo) {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex d3bce154da..467c6eff0e 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -383,7 +383,7 @@ test_expect_success 'setup expected diff' '\n test_expect_success 'can stage individual lines of patch' '\n \tgit reset &&\n \tprintf 61 >>test &&\n-\tprintf \"%s\\n\" l \"-2 4\" |\n+\tprintf \"%s\\n\" l \"1,2 4-\" |\n \tEDITOR=: git add -p 2>error &&\n \ttest_must_be_empty error &&\n \tgit diff --cached HEAD >actual &&\n@@ -410,7 +410,7 @@ test_expect_success 'setup expected diff' '\n '\n \n test_expect_success 'can reset individual lines of patch' '\n-\tprintf \"%s\\n\" l ^13 |\n+\tprintf \"%s\\n\" l -13 |\n \tEDITOR=: git reset -p 2>error &&\n \ttest_must_be_empty error &&\n \tgit diff --cached HEAD >actual &&\n\n\nPhillip Wood (3):\n add -p: select individual hunk lines\n add -p: allow line selection to be inverted\n add -p: optimize line selection for short hunks\n\n Documentation/git-add.txt | 10 +++\n git-add--interactive.perl | 181 +++++++++++++++++++++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 65 ++++++++++++++++\n 3 files changed, 256 insertions(+)\n\n-- \n2.16.2\n\n"},{"id":"341866","messageId":"20180316101346.7137-3-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180316101346.7137-1-phillip.wood@talktalk.net","subject":"[PATCH v3 2/3] add -p: allow line selection to be inverted","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-16T10:13:45Z","receivedAt":"2018-03-16T10:14:10Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf the list of lines to be selected begins with '-' select all the\nlines except the ones listed.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v2:\n - use '-' to invert the selection instead of '^' to be consistent\n with the rest of add -i and clean -i.\n\n Documentation/git-add.txt | 3 ++-\n git-add--interactive.perl | 17 ++++++++++++++++-\n t/t3701-add-interactive.sh | 2 +-\n 3 files changed, 19 insertions(+), 3 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 965e192a09..01ff4d7d24 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -342,7 +342,8 @@ deletion labelled with a number and you will be prompted to enter which\n lines you wish to select. Individual line numbers should be separated by\n a space or comma, to specify a range of lines use a dash between\n them. If the upper bound of a range of lines is omitted it defaults to\n-the last line.\n+the last line. To invert the selection prefix it with \"-\" so \"-3-5,8\"\n+will select everything except lines 3, 4, 5 and 8.\n +\n After deciding the fate for all hunks, if there is any hunk\n that was chosen, the index is updated with the selected hunks.\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 712226b34c..54fbe114f3 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1085,9 +1085,21 @@ sub check_hunk_label {\n sub parse_hunk_selection {\n \tlocal $_;\n \tmy ($hunk, $line) = @_;\n-\tmy $max_label = $hunk->{MAX_LABEL};\n+\tmy ($max_label, $invert) = ($hunk->{MAX_LABEL}, undef);\n \tmy @selected = (0) x ($max_label + 1);\n \tmy @fields = split(/[,\\s]+/, $line);\n+\tif ($fields[0] =~ /^-(.*)/) {\n+\t\t$invert = 1;\n+\t\tif ($1 ne '') {\n+\t\t\t$fields[0] = $1;\n+\t\t} else {\n+\t\t\tshift @fields;\n+\t\t\tunless (@fields) {\n+\t\t\t\terror_msg __(\"no lines to invert\\n\");\n+\t\t\t\treturn undef;\n+\t\t\t}\n+\t\t}\n+\t}\n \tfor (@fields) {\n \t\tif (my ($lo, $hi) = /^([0-9]+)-([0-9]*)$/) {\n \t\t\tif ($hi eq '') {\n@@ -1107,6 +1119,9 @@ sub parse_hunk_selection {\n \t\t\treturn undef;\n \t\t}\n \t}\n+\tif ($invert) {\n+\t\t@selected = map { !$_ } @selected;\n+\t}\n \treturn \\@selected;\n }\n \ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 5f3fcb1758..46814babf3 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -410,7 +410,7 @@ test_expect_success 'setup expected diff' '\n '\n \n test_expect_success 'can reset individual lines of patch' '\n-\tprintf \"%s\\n\" l 2 |\n+\tprintf \"%s\\n\" l \"-1 3\" |\n \tEDITOR=: git reset -p 2>error &&\n \ttest_must_be_empty error &&\n \tgit diff --cached HEAD >actual &&\n-- \n2.16.2\n\n"},{"id":"341867","messageId":"20180316101346.7137-4-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180316101346.7137-1-phillip.wood@talktalk.net","subject":"[PATCH v3 3/3] add -p: optimize line selection for short hunks","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-16T10:13:46Z","receivedAt":"2018-03-16T10:14:13Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf there are fewer than ten changes in a hunk then make spaces\noptional when selecting individual lines. This means that for short\nhunks one can just type 1-357 to stage lines 1, 2, 3, 5 & 7.\n\nSigned-off-by: Phillip Wood \n---\n\nNotes:\n changes since v2:\n - removed code that handled the default lower bound now that '-...'\n means invert the selection.\n\n Documentation/git-add.txt | 9 +++++----\n git-add--interactive.perl | 26 ++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 2 +-\n 3 files changed, 32 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 01ff4d7d24..f3c81dfb11 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -340,10 +340,11 @@ patch::\n If you press \"l\" then the hunk will be reprinted with each insertion or\n deletion labelled with a number and you will be prompted to enter which\n lines you wish to select. Individual line numbers should be separated by\n-a space or comma, to specify a range of lines use a dash between\n-them. If the upper bound of a range of lines is omitted it defaults to\n-the last line. To invert the selection prefix it with \"-\" so \"-3-5,8\"\n-will select everything except lines 3, 4, 5 and 8.\n+a space or comma (these can be omitted if there are fewer than ten\n+labelled lines), to specify a range of lines use a dash between them. If\n+the upper bound of a range of lines is omitted it defaults to the last\n+line. To invert the selection prefix it with \"-\" so \"-3-5,8\" will select\n+everything except lines 3, 4, 5 and 8.\n +\n After deciding the fate for all hunks, if there is any hunk\n that was chosen, the index is updated with the selected hunks.\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 54fbe114f3..d65ad7c26d 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1082,6 +1082,29 @@ sub check_hunk_label {\n \treturn 1;\n }\n \n+sub split_hunk_selection {\n+\tlocal $_;\n+\tmy @fields = @_;\n+\tmy @ret;\n+\tfor (@fields) {\n+\t\twhile ($_ ne '') {\n+\t\t\tif (/^[0-9]-$/) {\n+\t\t\t\tpush @ret, $_;\n+\t\t\t\tlast;\n+\t\t\t} elsif (/^([0-9](?:-[0-9])?)(.*)/) {\n+\t\t\t\tpush @ret, $1;\n+\t\t\t\t$_ = $2;\n+\t\t\t} else {\n+\t\t\t\terror_msg sprintf\n+\t\t\t\t __(\"invalid hunk line '%s'\\n\"),\n+\t\t\t\t substr($_, 0, 1);\n+\t\t\t\treturn ();\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn @ret;\n+}\n+\n sub parse_hunk_selection {\n \tlocal $_;\n \tmy ($hunk, $line) = @_;\n@@ -1100,6 +1123,9 @@ sub parse_hunk_selection {\n \t\t\t}\n \t\t}\n \t}\n+\tif ($max_label < 10) {\n+\t\t@fields = split_hunk_selection(@fields) or return undef;\n+\t}\n \tfor (@fields) {\n \t\tif (my ($lo, $hi) = /^([0-9]+)-([0-9]*)$/) {\n \t\t\tif ($hi eq '') {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 46814babf3..467c6eff0e 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -410,7 +410,7 @@ test_expect_success 'setup expected diff' '\n '\n \n test_expect_success 'can reset individual lines of patch' '\n-\tprintf \"%s\\n\" l \"-1 3\" |\n+\tprintf \"%s\\n\" l -13 |\n \tEDITOR=: git reset -p 2>error &&\n \ttest_must_be_empty error &&\n \tgit diff --cached HEAD >actual &&\n-- \n2.16.2\n\n"},{"id":"343357","messageId":"xmqq1sg27mwf.fsf@gitster-ct.c.googlers.com","threadId":"47873","inReplyTo":"20180316101346.7137-1-phillip.wood@talktalk.net","subject":"Re: [PATCH v3 0/3] add -p: select individual hunk lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-03-29T18:32:48Z","receivedAt":"2018-03-29T18:33:02Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> From: Phillip Wood \n>\n> Since v2 I've updated the patches to use '-' instead of '^' to invert\n> the selection to match the rest of add -i and clean -i.\n>\n> These patches build on top of the recount fixes in [1]. The commit\n> message for the first patch describes the motivation:\n>\n> \"When I end up editing hunks it is almost always because I want to\n> stage a subset of the lines in the hunk. Doing this by editing the\n> hunk is inconvenient and error prone (especially so if the patch is\n> going to be reversed before being applied). Instead offer an option\n> for add -p to stage individual lines. When the user presses 'l' the\n> hunk is redrawn with labels by the insertions and deletions and they\n> are prompted to enter a list of the lines they wish to stage. Ranges\n> of lines may be specified using 'a-b' where either 'a' or 'b' may be\n> omitted to mean all lines from 'a' to the end of the hunk or all lines\n> from 1 upto and including 'b'.\"\n\nI haven't seen any review comments on this round, and as I am not a\nheavy user of \"add -i\" interface (even though I admit that I\noriginally wrote it), I haven't had a chance to exercise the code\nmyself in the two weeks since the patches have been queued in my\ntree.\n\nI am inclihned to merge them to 'next' soonish, but please stop me\nif anybody (including the original author) has further comments.\n\nThanks.\n"},{"id":"343403","messageId":"a8dd262b-8b0c-8632-bf28-e03f9405317f@talktalk.net","threadId":"47873","inReplyTo":"xmqq1sg27mwf.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v3 0/3] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-03-30T11:09:24Z","receivedAt":"2018-03-30T11:09:34Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 29/03/18 19:32, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> From: Phillip Wood \n>>\n>> Since v2 I've updated the patches to use '-' instead of '^' to invert\n>> the selection to match the rest of add -i and clean -i.\n>>\n>> These patches build on top of the recount fixes in [1]. The commit\n>> message for the first patch describes the motivation:\n>>\n>> \"When I end up editing hunks it is almost always because I want to\n>> stage a subset of the lines in the hunk. Doing this by editing the\n>> hunk is inconvenient and error prone (especially so if the patch is\n>> going to be reversed before being applied). Instead offer an option\n>> for add -p to stage individual lines. When the user presses 'l' the\n>> hunk is redrawn with labels by the insertions and deletions and they\n>> are prompted to enter a list of the lines they wish to stage. Ranges\n>> of lines may be specified using 'a-b' where either 'a' or 'b' may be\n>> omitted to mean all lines from 'a' to the end of the hunk or all lines\n>> from 1 upto and including 'b'.\"\n> \n> I haven't seen any review comments on this round, and as I am not a\n> heavy user of \"add -i\" interface (even though I admit that I\n> originally wrote it), I haven't had a chance to exercise the code\n> myself in the two weeks since the patches have been queued in my\n> tree.\n> \n> I am inclihned to merge them to 'next' soonish, but please stop me\n> if anybody (including the original author) has further comments.\n> \n> Thanks.\n> \nHi Junio, if no one else has any comments, then I think it's ready for\nnext. I've not used this latest incarnation much but I've used the\nprevious versions quite a bit.\n\nBest Wishes\n\nPhillip\n"},{"id":"343543","messageId":"878ta8vyqe.fsf@evledraar.gmail.com","threadId":"47873","inReplyTo":"a8dd262b-8b0c-8632-bf28-e03f9405317f@talktalk.net","subject":"Re: [PATCH v3 0/3] add -p: select individual hunk lines","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-03-31T19:20:09Z","receivedAt":"2018-03-31T19:20:17Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Mar 30 2018, Phillip Wood wrote:\n\n> On 29/03/18 19:32, Junio C Hamano wrote:\n>> Phillip Wood writes:\n>>\n>>> From: Phillip Wood \n>>>\n>>> Since v2 I've updated the patches to use '-' instead of '^' to invert\n>>> the selection to match the rest of add -i and clean -i.\n>>>\n>>> These patches build on top of the recount fixes in [1]. The commit\n>>> message for the first patch describes the motivation:\n>>>\n>>> \"When I end up editing hunks it is almost always because I want to\n>>> stage a subset of the lines in the hunk. Doing this by editing the\n>>> hunk is inconvenient and error prone (especially so if the patch is\n>>> going to be reversed before being applied). Instead offer an option\n>>> for add -p to stage individual lines. When the user presses 'l' the\n>>> hunk is redrawn with labels by the insertions and deletions and they\n>>> are prompted to enter a list of the lines they wish to stage. Ranges\n>>> of lines may be specified using 'a-b' where either 'a' or 'b' may be\n>>> omitted to mean all lines from 'a' to the end of the hunk or all lines\n>>> from 1 upto and including 'b'.\"\n>>\n>> I haven't seen any review comments on this round, and as I am not a\n>> heavy user of \"add -i\" interface (even though I admit that I\n>> originally wrote it), I haven't had a chance to exercise the code\n>> myself in the two weeks since the patches have been queued in my\n>> tree.\n>>\n>> I am inclihned to merge them to 'next' soonish, but please stop me\n>> if anybody (including the original author) has further comments.\n>>\n>> Thanks.\n>>\n> Hi Junio, if no one else has any comments, then I think it's ready for\n> next. I've not used this latest incarnation much but I've used the\n> previous versions quite a bit.\n\nFirst of all thinks for working on this. Something like this is a\nfeature I've long wanted to have and have just been manually using edit.\n\nAs for the code, one comment: For reasons of avoiding something like the\n2.17.0-rc* bug I just sent a patch for, I think you should change your\nuse of the implicit $_ to something where you explicitly create lexical\nvariables instead.\n\nIt's bad style in Perl to use $_ for anything except a one-liner, and\nsimilar to the $1 bug with your other patch, you'll get buggy code\n(regardless of your use of local $_) if one of the functions you're\ncalling in these >10 line for-loops starts doing something to set $_\nitself, as demonstrated by:\n\n $ perl -wE 'sub foo { local $_; for (1..3) { bar(); say } } sub bar { $_ = $_ ** 2; } foo()'\n 1\n 4\n 9\n\nLet's just name these variables, even if it wasn't for that caveat it\nwould still be a good idea, since for any non-trivial use of $_ you've\ngot to mentally keep track of what set $_ where, so it's hard to read.\n\nAs for the implementation, I *want* to love this, but it seems the way\nit works is just fatally flawed, consider. *The* use-case I've had for\nsomething like this (maybe yours differs?) is something where I do e.g.:\n\n $ perl -pi -e 's/git/Git/g' README.md\n\nWhich gives me (among other things):\n\n -See [Documentation/gittutorial.txt][] to get started, then see\n -[Documentation/giteveryday.txt][] for a useful minimum set of commands, and\n -Documentation/git-.txt for documentation of each command.\n -If git has been correctly installed, then the tutorial can also be\n -read with `man gittutorial` or `git help tutorial`, and the\n -documentation of each command with `man git-` or `git help\n +See [Documentation/Gittutorial.txt][] to get started, then see\n +[Documentation/Giteveryday.txt][] for a useful minimum set of commands, and\n +Documentation/Git-.txt for documentation of each command.\n +If Git has been correctly installed, then the tutorial can also be\n +read with `man Gittutorial` or `Git help tutorial`, and the\n +documentation of each command with `man Git-` or `Git help\n\nWhich to me, is a perfect use-case for this feature. Here I\nhypothetically want to change \"git\" to \"Git\" in prose, so I only want to\nchange that \"If git has been\" line, the rest are all references to\nfilenames or command names.\n\nSo I would manually edit the hunk via \"e\" to:\n\n See [Documentation/gittutorial.txt][] to get started, then see\n [Documentation/giteveryday.txt][] for a useful minimum set of commands, and\n Documentation/git-.txt for documentation of each command.\n -If git has been correctly installed, then the tutorial can also be\n +If Git has been correctly installed, then the tutorial can also be\n read with `man gittutorial` or `git help tutorial`, and the\n documentation of each command with `man git-` or `git help\n `.\n\nYay, but very tedious. Now let's use your feature to do this:\n\n 1 -See [Documentation/gittutorial.txt][] to get started, then see\n 2 -[Documentation/giteveryday.txt][] for a useful minimum set of commands, and\n 3 -Documentation/git-.txt for documentation of each command.\n 4 -If git has been correctly installed, then the tutorial can also be\n 5 -read with `man gittutorial` or `git help tutorial`, and the\n 6 -documentation of each command with `man git-` or `git help\n 7 +See [Documentation/Gittutorial.txt][] to get started, then see\n 8 +[Documentation/Giteveryday.txt][] for a useful minimum set of commands, and\n 9 +Documentation/Git-.txt for documentation of each command.\n 10 +If Git has been correctly installed, then the tutorial can also be\n 11 +read with `man Gittutorial` or `Git help tutorial`, and the\n 12 +documentation of each command with `man Git-` or `Git help\n `.\n\n select lines? 4,10\n\nSo what I was expecting this to do was some automagic where it would\npair up the 4 line, and based on the removed/added count figure out\nwhich line I'm also adding corresponds to that. I.e. both selected lines\nare the 4th line removed/added, so it should transpose the 10th to the\n4th, but instead I get a patch that looks like this:\n\n diff --git a/README.md b/README.md\n index f17af66a97..7234756e64 100644\n --- a/README.md\n +++ b/README.md\n @@ -18,9 +18,9 @@ including full documentation and Git related tools.\n See [Documentation/gittutorial.txt][] to get started, then see\n [Documentation/giteveryday.txt][] for a useful minimum set of commands, and\n Documentation/git-.txt for documentation of each command.\n -If git has been correctly installed, then the tutorial can also be\n read with `man gittutorial` or `git help tutorial`, and the\n documentation of each command with `man git-` or `git help\n +If Git has been correctly installed, then the tutorial can also be\n `.\n\nI.e. it just grepped out the removed line from the removed chunk, and\nthe same for the added bit, which of course means that now the added\nline doesn't get injected into the correct place, but added to the end.\n\nI can see *why* that happens, but I can't imagine a case where this\nbehavior isn't useless.\n\nWhat this seems useful for now is for chunks that only consist of lines\nthat are added or removed, maybe there's similar edge cases with those,\nbut I can't think of any, there I think we should do the obvious and\nintuitive thing.\n\nBut I think that as this stands we really should at least disable this\nwhere we present the user with a hunk that consists of both removed &\nadded lines, since I think the desired behavior I've described above\nshould be the default, and once we pick one we're going to have to\nsupport it forever, so it's important to get it right to begin with.\n"},{"id":"343573","messageId":"1e8daa35-3749-fec7-ae88-541169dd0b2a@talktalk.net","threadId":"47873","inReplyTo":"878ta8vyqe.fsf@evledraar.gmail.com","subject":"Re: [PATCH v3 0/3] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-04-02T10:55:23Z","receivedAt":"2018-04-02T10:55:35Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 31/03/18 20:20, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Fri, Mar 30 2018, Phillip Wood wrote:\n> \n>> On 29/03/18 19:32, Junio C Hamano wrote:\n>>> Phillip Wood writes:\n>>>\n>>>> From: Phillip Wood \n>>>>\n>>>> Since v2 I've updated the patches to use '-' instead of '^' to invert\n>>>> the selection to match the rest of add -i and clean -i.\n>>>>\n>>>> These patches build on top of the recount fixes in [1]. The commit\n>>>> message for the first patch describes the motivation:\n>>>>\n>>>> \"When I end up editing hunks it is almost always because I want to\n>>>> stage a subset of the lines in the hunk. Doing this by editing the\n>>>> hunk is inconvenient and error prone (especially so if the patch is\n>>>> going to be reversed before being applied). Instead offer an option\n>>>> for add -p to stage individual lines. When the user presses 'l' the\n>>>> hunk is redrawn with labels by the insertions and deletions and they\n>>>> are prompted to enter a list of the lines they wish to stage. Ranges\n>>>> of lines may be specified using 'a-b' where either 'a' or 'b' may be\n>>>> omitted to mean all lines from 'a' to the end of the hunk or all lines\n>>>> from 1 upto and including 'b'.\"\n>>>\n>>> I haven't seen any review comments on this round, and as I am not a\n>>> heavy user of \"add -i\" interface (even though I admit that I\n>>> originally wrote it), I haven't had a chance to exercise the code\n>>> myself in the two weeks since the patches have been queued in my\n>>> tree.\n>>>\n>>> I am inclihned to merge them to 'next' soonish, but please stop me\n>>> if anybody (including the original author) has further comments.\n>>>\n>>> Thanks.\n>>>\n>> Hi Junio, if no one else has any comments, then I think it's ready for\n>> next. I've not used this latest incarnation much but I've used the\n>> previous versions quite a bit.\n\nAh it seems I spoke too soon.\n\nThanks for taking a look at this Ævar\n\n> First of all thinks for working on this. Something like this is a\n> feature I've long wanted to have and have just been manually using edit.\n> \n> As for the code, one comment: For reasons of avoiding something like the\n> 2.17.0-rc* bug I just sent a patch for, I think you should change your\n> use of the implicit $_ to something where you explicitly create lexical\n> variables instead.\n> \n> It's bad style in Perl to use $_ for anything except a one-liner, and\n> similar to the $1 bug with your other patch, you'll get buggy code\n> (regardless of your use of local $_) if one of the functions you're\n> calling in these >10 line for-loops starts doing something to set $_\n> itself, as demonstrated by:\n> \n> $ perl -wE 'sub foo { local $_; for (1..3) { bar(); say } } sub bar { $_ = $_ ** 2; } foo()'\n> 1\n> 4\n> 9\n> \n> Let's just name these variables, even if it wasn't for that caveat it\n> would still be a good idea, since for any non-trivial use of $_ you've\n> got to mentally keep track of what set $_ where, so it's hard to read.\n\nRight, I'll use lexical variables.\n\n> \n> As for the implementation, I *want* to love this, but it seems the way\n> it works is just fatally flawed, consider. *The* use-case I've had for\n> something like this (maybe yours differs?) is something where I do e.g.:\n\nI've used it for selecting a subset of additions or deletions when my\nwork has run ahead of a logical commit boundary. I've also used it in\ncases such as\n\n\t-original\n\t+modified\n\t+new stuff\n\nTo separate the modification from the addition of new stuff, but I've\nnot used it on a list of modifications as in your example.\n\n> $ perl -pi -e 's/git/Git/g' README.md\n> \n> Which gives me (among other things):\n> \n> -See [Documentation/gittutorial.txt][] to get started, then see\n> -[Documentation/giteveryday.txt][] for a useful minimum set of commands, and\n> -Documentation/git-.txt for documentation of each command.\n> -If git has been correctly installed, then the tutorial can also be\n> -read with `man gittutorial` or `git help tutorial`, and the\n> -documentation of each command with `man git-` or `git help\n> +See [Documentation/Gittutorial.txt][] to get started, then see\n> +[Documentation/Giteveryday.txt][] for a useful minimum set of commands, and\n> +Documentation/Git-.txt for documentation of each command.\n> +If Git has been correctly installed, then the tutorial can also be\n> +read with `man Gittutorial` or `Git help tutorial`, and the\n> +documentation of each command with `man Git-` or `Git help\n> \n> Which to me, is a perfect use-case for this feature. Here I\n> hypothetically want to change \"git\" to \"Git\" in prose, so I only want to\n> change that \"If git has been\" line, the rest are all references to\n> filenames or command names.\n> \n> So I would manually edit the hunk via \"e\" to:\n> \n> See [Documentation/gittutorial.txt][] to get started, then see\n> [Documentation/giteveryday.txt][] for a useful minimum set of commands, and\n> Documentation/git-.txt for documentation of each command.\n> -If git has been correctly installed, then the tutorial can also be\n> +If Git has been correctly installed, then the tutorial can also be\n> read with `man gittutorial` or `git help tutorial`, and the\n> documentation of each command with `man git-` or `git help\n> `.\n> \n> Yay, but very tedious. Now let's use your feature to do this:\n> \n> 1 -See [Documentation/gittutorial.txt][] to get started, then see\n> 2 -[Documentation/giteveryday.txt][] for a useful minimum set of commands, and\n> 3 -Documentation/git-.txt for documentation of each command.\n> 4 -If git has been correctly installed, then the tutorial can also be\n> 5 -read with `man gittutorial` or `git help tutorial`, and the\n> 6 -documentation of each command with `man git-` or `git help\n> 7 +See [Documentation/Gittutorial.txt][] to get started, then see\n> 8 +[Documentation/Giteveryday.txt][] for a useful minimum set of commands, and\n> 9 +Documentation/Git-.txt for documentation of each command.\n> 10 +If Git has been correctly installed, then the tutorial can also be\n> 11 +read with `man Gittutorial` or `Git help tutorial`, and the\n> 12 +documentation of each command with `man Git-` or `Git help\n> `.\n> \n> select lines? 4,10\n> \n> So what I was expecting this to do was some automagic where it would\n> pair up the 4 line, and based on the removed/added count figure out\n> which line I'm also adding corresponds to that. I.e. both selected lines\n> are the 4th line removed/added, so it should transpose the 10th to the\n> 4th, but instead I get a patch that looks like this:\n> \n> diff --git a/README.md b/README.md\n> index f17af66a97..7234756e64 100644\n> --- a/README.md\n> +++ b/README.md\n> @@ -18,9 +18,9 @@ including full documentation and Git related tools.\n> See [Documentation/gittutorial.txt][] to get started, then see\n> [Documentation/giteveryday.txt][] for a useful minimum set of commands, and\n> Documentation/git-.txt for documentation of each command.\n> -If git has been correctly installed, then the tutorial can also be\n> read with `man gittutorial` or `git help tutorial`, and the\n> documentation of each command with `man git-` or `git help\n> +If Git has been correctly installed, then the tutorial can also be\n> `.\n> \n> I.e. it just grepped out the removed line from the removed chunk, and\n> the same for the added bit, which of course means that now the added\n> line doesn't get injected into the correct place, but added to the end.\n> \n> I can see *why* that happens, but I can't imagine a case where this\n> behavior isn't useless.\n\nI agree it's useless here I've got some suggestions on how to fix it\nthough I've not spent much time thinking them through - any comments\nwould be most welcome.\n\nI think we want something that pairs up groups of selected deletions and\ninsertions rather than lines, so that it can handle cases where the\nnumber of inserted lines differs from the number of deletions but there\nare the same number of groups. For example given\n\n\t1 -* first\n\t2 -* second\n\t3 -* third\n\t4 +* a longer description of the\n\t5 + first item\n\t6 +* the second item\n\t7 +* the third item\n\n1,3,4-5,7 would give\n\n\t-* first\n\t+* a longer description of the\n\t+ first item\n\t * second\n\t-* third\n\t+* the third item\n\nIf the number of deletion groups differs from the number of insertion\ngroups then there is no way to automagically pair them up so we'd need\nsome syntax to do that. We could make the order of the numbers given by\nthe user significant so that 1,6,7,2,4,9 would pair line 1 with 6 & 7\nand line 4 with 9 and delete line 2. It would then be an error if the an\nearlier deletion was pair with a later insertion, so in the example\nabove 1,6,7,4,9,2 or 2,4,9,1,6,7 would be valid but 1,9,2,4,8 wouldn't).\nWe could also use brackets to group things e.g. [1,6,7],2,[4,9] which\nmakes the groups clearer visually.\n\nThinking further there is a problem with\n\n\t1 -original\n\t2 +a new paragraph before\n\t3 +original\n\t4 +\n\t5 +modified original\n\nif the user wants to commit the new paragraph before the modified\noriginal then they need a way to specify that the insertion should come\nbefore the deletion. At the moment giving 2-4 will put the new paragraph\nafter the unmodified original. To solve that there needs to be a way to\nindicate that a group of insertions needs to come before the context\nlines created by not staging the deletions - perhaps ^[2-4] or [^2-4]?\n(we could distinguish between commas and spaces so that ^2,3,5 is\ndifferent to ^2 3,5 but that is then different to how lists work in the\nrest of git)\n\n\n> What this seems useful for now is for chunks that only consist of lines\n> that are added or removed, maybe there's similar edge cases with those,\n> but I can't think of any, there I think we should do the obvious and\n> intuitive thing.\n> \n> But I think that as this stands we really should at least disable this\n> where we present the user with a hunk that consists of both removed &\n> added lines, since I think the desired behavior I've described above\n> should be the default, and once we pick one we're going to have to\n> support it forever, so it's important to get it right to begin with.\n> \n\nThanks for your feedback it's been really useful, let me know what you\nthink about grouping things.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"343574","messageId":"87370dx2fq.fsf@evledraar.gmail.com","threadId":"47873","inReplyTo":"1e8daa35-3749-fec7-ae88-541169dd0b2a@talktalk.net","subject":"Re: [PATCH v3 0/3] add -p: select individual hunk lines","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-04-02T11:39:21Z","receivedAt":"2018-04-02T11:42:40Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Apr 02 2018, Phillip Wood wrote:\n\n> On 31/03/18 20:20, Ævar Arnfjörð Bjarmason wrote:\n>>\n>> On Fri, Mar 30 2018, Phillip Wood wrote:\n>>\n>>> On 29/03/18 19:32, Junio C Hamano wrote:\n>>>> Phillip Wood writes:\n>>>>\n>>>>> From: Phillip Wood \n>>>>>\n>>>>> Since v2 I've updated the patches to use '-' instead of '^' to invert\n>>>>> the selection to match the rest of add -i and clean -i.\n>>>>>\n>>>>> These patches build on top of the recount fixes in [1]. The commit\n>>>>> message for the first patch describes the motivation:\n>>>>>\n>>>>> \"When I end up editing hunks it is almost always because I want to\n>>>>> stage a subset of the lines in the hunk. Doing this by editing the\n>>>>> hunk is inconvenient and error prone (especially so if the patch is\n>>>>> going to be reversed before being applied). Instead offer an option\n>>>>> for add -p to stage individual lines. When the user presses 'l' the\n>>>>> hunk is redrawn with labels by the insertions and deletions and they\n>>>>> are prompted to enter a list of the lines they wish to stage. Ranges\n>>>>> of lines may be specified using 'a-b' where either 'a' or 'b' may be\n>>>>> omitted to mean all lines from 'a' to the end of the hunk or all lines\n>>>>> from 1 upto and including 'b'.\"\n>>>>\n>>>> I haven't seen any review comments on this round, and as I am not a\n>>>> heavy user of \"add -i\" interface (even though I admit that I\n>>>> originally wrote it), I haven't had a chance to exercise the code\n>>>> myself in the two weeks since the patches have been queued in my\n>>>> tree.\n>>>>\n>>>> I am inclihned to merge them to 'next' soonish, but please stop me\n>>>> if anybody (including the original author) has further comments.\n>>>>\n>>>> Thanks.\n>>>>\n>>> Hi Junio, if no one else has any comments, then I think it's ready for\n>>> next. I've not used this latest incarnation much but I've used the\n>>> previous versions quite a bit.\n>\n> Ah it seems I spoke too soon.\n>\n> Thanks for taking a look at this Ævar\n>\n>> First of all thinks for working on this. Something like this is a\n>> feature I've long wanted to have and have just been manually using edit.\n>>\n>> As for the code, one comment: For reasons of avoiding something like the\n>> 2.17.0-rc* bug I just sent a patch for, I think you should change your\n>> use of the implicit $_ to something where you explicitly create lexical\n>> variables instead.\n>>\n>> It's bad style in Perl to use $_ for anything except a one-liner, and\n>> similar to the $1 bug with your other patch, you'll get buggy code\n>> (regardless of your use of local $_) if one of the functions you're\n>> calling in these >10 line for-loops starts doing something to set $_\n>> itself, as demonstrated by:\n>>\n>> $ perl -wE 'sub foo { local $_; for (1..3) { bar(); say } } sub bar { $_ = $_ ** 2; } foo()'\n>> 1\n>> 4\n>> 9\n>>\n>> Let's just name these variables, even if it wasn't for that caveat it\n>> would still be a good idea, since for any non-trivial use of $_ you've\n>> got to mentally keep track of what set $_ where, so it's hard to read.\n>\n> Right, I'll use lexical variables.\n>\n>>\n>> As for the implementation, I *want* to love this, but it seems the way\n>> it works is just fatally flawed, consider. *The* use-case I've had for\n>> something like this (maybe yours differs?) is something where I do e.g.:\n>\n> I've used it for selecting a subset of additions or deletions when my\n> work has run ahead of a logical commit boundary. I've also used it in\n> cases such as\n>\n> \t-original\n> \t+modified\n> \t+new stuff\n>\n> To separate the modification from the addition of new stuff, but I've\n> not used it on a list of modifications as in your example.\n\nRight. I was wrong in saying that it wouldn't work as expected for hunks\nwith removed/added lines, but only for a subset of those cases.\n\n>> $ perl -pi -e 's/git/Git/g' README.md\n>>\n>> Which gives me (among other things):\n>>\n>> -See [Documentation/gittutorial.txt][] to get started, then see\n>> -[Documentation/giteveryday.txt][] for a useful minimum set of commands, and\n>> -Documentation/git-.txt for documentation of each command.\n>> -If git has been correctly installed, then the tutorial can also be\n>> -read with `man gittutorial` or `git help tutorial`, and the\n>> -documentation of each command with `man git-` or `git help\n>> +See [Documentation/Gittutorial.txt][] to get started, then see\n>> +[Documentation/Giteveryday.txt][] for a useful minimum set of commands, and\n>> +Documentation/Git-.txt for documentation of each command.\n>> +If Git has been correctly installed, then the tutorial can also be\n>> +read with `man Gittutorial` or `Git help tutorial`, and the\n>> +documentation of each command with `man Git-` or `Git help\n>>\n>> Which to me, is a perfect use-case for this feature. Here I\n>> hypothetically want to change \"git\" to \"Git\" in prose, so I only want to\n>> change that \"If git has been\" line, the rest are all references to\n>> filenames or command names.\n>>\n>> So I would manually edit the hunk via \"e\" to:\n>>\n>> See [Documentation/gittutorial.txt][] to get started, then see\n>> [Documentation/giteveryday.txt][] for a useful minimum set of commands, and\n>> Documentation/git-.txt for documentation of each command.\n>> -If git has been correctly installed, then the tutorial can also be\n>> +If Git has been correctly installed, then the tutorial can also be\n>> read with `man gittutorial` or `git help tutorial`, and the\n>> documentation of each command with `man git-` or `git help\n>> `.\n>>\n>> Yay, but very tedious. Now let's use your feature to do this:\n>>\n>> 1 -See [Documentation/gittutorial.txt][] to get started, then see\n>> 2 -[Documentation/giteveryday.txt][] for a useful minimum set of commands, and\n>> 3 -Documentation/git-.txt for documentation of each command.\n>> 4 -If git has been correctly installed, then the tutorial can also be\n>> 5 -read with `man gittutorial` or `git help tutorial`, and the\n>> 6 -documentation of each command with `man git-` or `git help\n>> 7 +See [Documentation/Gittutorial.txt][] to get started, then see\n>> 8 +[Documentation/Giteveryday.txt][] for a useful minimum set of commands, and\n>> 9 +Documentation/Git-.txt for documentation of each command.\n>> 10 +If Git has been correctly installed, then the tutorial can also be\n>> 11 +read with `man Gittutorial` or `Git help tutorial`, and the\n>> 12 +documentation of each command with `man Git-` or `Git help\n>> `.\n>>\n>> select lines? 4,10\n>>\n>> So what I was expecting this to do was some automagic where it would\n>> pair up the 4 line, and based on the removed/added count figure out\n>> which line I'm also adding corresponds to that. I.e. both selected lines\n>> are the 4th line removed/added, so it should transpose the 10th to the\n>> 4th, but instead I get a patch that looks like this:\n>>\n>> diff --git a/README.md b/README.md\n>> index f17af66a97..7234756e64 100644\n>> --- a/README.md\n>> +++ b/README.md\n>> @@ -18,9 +18,9 @@ including full documentation and Git related tools.\n>> See [Documentation/gittutorial.txt][] to get started, then see\n>> [Documentation/giteveryday.txt][] for a useful minimum set of commands, and\n>> Documentation/git-.txt for documentation of each command.\n>> -If git has been correctly installed, then the tutorial can also be\n>> read with `man gittutorial` or `git help tutorial`, and the\n>> documentation of each command with `man git-` or `git help\n>> +If Git has been correctly installed, then the tutorial can also be\n>> `.\n>>\n>> I.e. it just grepped out the removed line from the removed chunk, and\n>> the same for the added bit, which of course means that now the added\n>> line doesn't get injected into the correct place, but added to the end.\n>>\n>> I can see *why* that happens, but I can't imagine a case where this\n>> behavior isn't useless.\n>\n> I agree it's useless here I've got some suggestions on how to fix it\n> though I've not spent much time thinking them through - any comments\n> would be most welcome.\n>\n> I think we want something that pairs up groups of selected deletions and\n> insertions rather than lines, so that it can handle cases where the\n> number of inserted lines differs from the number of deletions but there\n> are the same number of groups. For example given\n>\n> \t1 -* first\n> \t2 -* second\n> \t3 -* third\n> \t4 +* a longer description of the\n> \t5 + first item\n> \t6 +* the second item\n> \t7 +* the third item\n>\n> 1,3,4-5,7 would give\n>\n> \t-* first\n> \t+* a longer description of the\n> \t+ first item\n> \t * second\n> \t-* third\n> \t+* the third item\n>\n> If the number of deletion groups differs from the number of insertion\n> groups then there is no way to automagically pair them up so we'd need\n> some syntax to do that. We could make the order of the numbers given by\n> the user significant so that 1,6,7,2,4,9 would pair line 1 with 6 & 7\n> and line 4 with 9 and delete line 2. It would then be an error if the an\n> earlier deletion was pair with a later insertion, so in the example\n> above 1,6,7,4,9,2 or 2,4,9,1,6,7 would be valid but 1,9,2,4,8 wouldn't).\n> We could also use brackets to group things e.g. [1,6,7],2,[4,9] which\n> makes the groups clearer visually.\n>\n> Thinking further there is a problem with\n>\n> \t1 -original\n> \t2 +a new paragraph before\n> \t3 +original\n> \t4 +\n> \t5 +modified original\n>\n> if the user wants to commit the new paragraph before the modified\n> original then they need a way to specify that the insertion should come\n> before the deletion. At the moment giving 2-4 will put the new paragraph\n> after the unmodified original. To solve that there needs to be a way to\n> indicate that a group of insertions needs to come before the context\n> lines created by not staging the deletions - perhaps ^[2-4] or [^2-4]?\n> (we could distinguish between commas and spaces so that ^2,3,5 is\n> different to ^2 3,5 but that is then different to how lists work in the\n> rest of git)\n>\n>\n>> What this seems useful for now is for chunks that only consist of lines\n>> that are added or removed, maybe there's similar edge cases with those,\n>> but I can't think of any, there I think we should do the obvious and\n>> intuitive thing.\n>>\n>> But I think that as this stands we really should at least disable this\n>> where we present the user with a hunk that consists of both removed &\n>> added lines, since I think the desired behavior I've described above\n>> should be the default, and once we pick one we're going to have to\n>> support it forever, so it's important to get it right to begin with.\n>>\n>\n> Thanks for your feedback it's been really useful, let me know what you\n> think about grouping things.\n\nAll of that makes sense to me. Yes as you noted earlier there's only so\nfar we can get in this case by automatically trying to pair up\nremoved/added lines, and actually your example wasn't even very\npathological, consider:\n\n 1 -* a longer description of the\n 2 - first item\n 3 -* second\n 4 -* third\n 5 +* first\n 6 +* a longer description of the\n 7 + second item\n 8 +* the third item\n\nThere the naïve heuristic I initially noted for selecting 4,8 would\nwork, but not we couldn't in the general case rely on the removed/added\nlines being equal in number as some fallback for the simpler transpose\nbehavior.\n\nIt sounds like you're interested in hacking more on this, great. I'll be\nhappy to review & add tests once you have something.\n\nI'll just say that I wonder if we should simply leave some of these more\ncomplex cases on the table, and only handle some subsets of simple cases\nin liue of coming up with a syntax that handles everything. there's\nalways \"edit\" as the fallback for the complex cases.\n"},{"id":"353626","messageId":"20180726102227.997-1-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180219113619.26566-1-phillip.wood@talktalk.net","subject":"[RFC PATCH v4 0/4] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-07-26T10:22:23Z","receivedAt":"2018-07-26T10:22:39Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nI've updated this series based on Ævar's feedback on v3 (to paraphrase\nstop using '$_' so much and fix staging modified lines.). The first\npatch is functionally equivalent to the previous version but with a\nreworked implementation. Patch 2 is new, it implements correctly\nstaging modified lines with a couple of limitations - see the commit\nmessage for more details, I'm keen to get some feedback on it. Patches\n3 and 4 are essentially rebased and tweaked versions of patches 2 and\n3 from the previous version.\n\nThis series is based on pw/add-p-recount (f4d35a6b49 \"add -p: fix\ncounting empty context lines in edited patches\")\n\nThe motivation for this series is summed up in the first commit\nmessage:\n\n\"When I end up editing hunks it is almost always because I want to\nstage a subset of the lines in the hunk. Doing this by editing the\nhunk is inconvenient and error prone (especially so if the patch is\ngoing to be reversed before being applied). Instead offer an option\nfor add -p to stage individual lines. When the user presses 'l' the\nhunk is redrawn with labels by the insertions and deletions and they\nare prompted to enter a list of the lines they wish to stage. Ranges\nof lines may be specified using 'a-b' where either 'a' or 'b' may be\nomitted to mean all lines from 'a' to the end of the hunk or all lines\nfrom 1 upto and including 'b'.\"\n\nPhillip Wood (4):\n add -p: select individual hunk lines\n add -p: select modified lines correctly\n add -p: allow line selection to be inverted\n add -p: optimize line selection for short hunks\n\n Documentation/git-add.txt | 10 ++\n git-add--interactive.perl | 350 +++++++++++++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 209 ++++++++++++++++++++++\n 3 files changed, 569 insertions(+)\n\n-- \n2.18.0\n\n"},{"id":"353627","messageId":"20180726102227.997-2-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180726102227.997-1-phillip.wood@talktalk.net","subject":"[PATCH v4 1/4] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-07-26T10:22:24Z","receivedAt":"2018-07-26T10:22:40Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen I end up editing hunks it is almost always because I want to\nstage a subset of the lines in the hunk. Doing this by editing the\nhunk is inconvenient and error prone (especially so if the patch is\ngoing to be reversed before being applied). Instead offer an option\nfor add -p to stage individual lines. When the user presses 'l' the\nhunk is redrawn with labels by the insertions and deletions and they\nare prompted to enter a list of the lines they wish to stage. Ranges\nof lines may be specified using 'a-b' where 'b' may be omitted to mean\nall lines from 'a' to the end of the hunk. Modified lines are not\nhandled correctly, that will be fixed in the next commit.\n\nSigned-off-by: Phillip Wood \n---\n Documentation/git-add.txt | 8 ++\n git-add--interactive.perl | 181 +++++++++++++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 103 +++++++++++++++++++++\n 3 files changed, 292 insertions(+)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex d50fa339dc..965e192a09 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -332,10 +332,18 @@ patch::\n J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n+ l - select hunk lines to use\n s - split the current hunk into smaller hunks\n e - manually edit the current hunk\n ? - print help\n +\n+If you press \"l\" then the hunk will be reprinted with each insertion or\n+deletion labelled with a number and you will be prompted to enter which\n+lines you wish to select. Individual line numbers should be separated by\n+a space or comma, to specify a range of lines use a dash between\n+them. If the upper bound of a range of lines is omitted it defaults to\n+the last line.\n++\n After deciding the fate for all hunks, if there is any hunk\n that was chosen, the index is updated with the selected hunks.\n +\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 8361ef45e7..cbc9e5698a 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1013,6 +1013,171 @@ sub color_diff {\n \t} @_;\n }\n \n+use constant {\n+\tNO_NEWLINE => 1,\n+};\n+\n+sub label_hunk_lines {\n+\tmy $hunk = shift;\n+\tmy $text = $hunk->{TEXT};\n+\tmy (@line_flags, @lines);\n+\tmy ($block, $label, $last_mode) = (0, 0, '');\n+\tfor my $line (1..$#{$text}) {\n+\t\t$line_flags[$line] = 0;\n+\t\tmy $mode = substr($text->[$line], 0, 1);\n+\t\tif ($mode eq '\\\\') {\n+\t\t\t$line_flags[$line - 1] |= NO_NEWLINE;\n+\t\t}\n+\t\tif ($mode eq '-' or $mode eq '+') {\n+\t\t\t$lines[++$label] = $line;\n+\t\t}\n+\t}\n+\tif ($label > 1) {\n+\t\t$hunk->{LABELS} = {\n+\t\t\tLINES => \\@lines,\n+\t\t};\n+\t\t$hunk->{LINE_FLAGS} = \\@line_flags;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+sub select_hunk_lines {\n+\tmy ($hunk, $selected) = @_;\n+\tmy ($line_flags, $text) = @{$hunk}{qw(LINE_FLAGS TEXT)};\n+\tmy ($i, $o_cnt, $n_cnt) = (0, 0, 0);\n+\tmy @newtext;\n+\n+\tmy $select_lines = sub {\n+\t\tfor my $i (@_) {\n+\t\t\tmy $line = $text->[$i];\n+\t\t\tmy $mode = substr($line, 0, 1);\n+\t\t\tpush @newtext, $line;\n+\t\t\tif ($mode eq '+') {\n+\t\t\t\t$n_cnt++;\n+\t\t\t} elsif ($mode eq '-') {\n+\t\t\t\t$o_cnt++;\n+\t\t\t}\n+\t\t}\n+\t};\n+\n+\tmy ($lo, $hi) = splice(@$selected, 0, 2);\n+\t# Lines with this mode will become context lines if they are\n+\t# not selected\n+\tmy $context_mode = $patch_mode_flavour{IS_REVERSE} ? '+' : '-';\n+\tfor $i (1..$#{$text}) {\n+\t\tif ($lo <= $i and $i <= $hi) {\n+\t\t\t$select_lines->($i);\n+\t\t} else {\n+\t\t\tmy $line = $text->[$i];\n+\t\t\tmy $mode = substr($line, 0, 1);\n+\t\t\tif ($mode eq ' ' or $mode eq $context_mode) {\n+\t\t\t\tpush @newtext, ' ' . substr($line, 1);\n+\t\t\t\t$o_cnt++; $n_cnt++;\n+\t\t\t\tif ($line_flags->[$i] & NO_NEWLINE) {\n+\t\t\t\t\tpush @newtext, $text->[$i + 1];\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t\tif ($i == $hi) {\n+\t\t\tif (@$selected) {\n+\t\t\t\t($lo, $hi) = splice(@$selected, 0, 2);\n+\t\t\t}\n+\t\t}\n+\t}\n+\tmy ($o_ofs, $orig_o_cnt, $n_ofs, $orig_n_cnt) =\n+\t\t\t\t\tparse_hunk_header($text->[0]);\n+\tunshift @newtext, format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n+\tmy $newhunk = {\n+\t\tTEXT => \\@newtext,\n+\t\tDISPLAY => [ color_diff(@newtext) ],\n+\t\tOFS_DELTA => $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt,\n+\t\tTYPE => $hunk->{TYPE},\n+\t\tUSE => 1,\n+\t};\n+\t# If this hunk has previously been edited add the offset delta\n+\t# of the old hunk to get the real delta from the original\n+\t# hunk.\n+\tif ($hunk->{OFS_DELTA}) {\n+\t\t$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n+\t}\n+\treturn $newhunk;\n+}\n+\n+sub check_hunk_label {\n+\tmy ($max_label, $label) = @_;\n+\tif ($label < 1 or $label > $max_label) {\n+\t\terror_msg sprintf(__(\"invalid hunk line '%d'\\n\"), $label);\n+\t\treturn 0;\n+\t}\n+\treturn 1;\n+}\n+\n+sub parse_hunk_selection {\n+\tmy ($hunk, $line) = @_;\n+\tmy $lines = $hunk->{LABELS}->{LINES};\n+\tmy $max_label = $#{$lines};\n+\tmy %selected;\n+\tmy @fields = split(/[,\\s]+/, $line);\n+\tfor my $f (@fields) {\n+\t\tif (my ($lo, $hi) = ($f =~ /^([0-9]+)-([0-9]*)$/)) {\n+\t\t\tif ($hi eq '') {\n+\t\t\t\t$hi = $max_label;\n+\t\t\t}\n+\t\t\tcheck_hunk_label($max_label, $lo) or return undef;\n+\t\t\tcheck_hunk_label($max_label, $hi) or return undef;\n+\t\t\tif ($hi < $lo) {\n+\t\t\t\t($lo, $hi) = ($hi, $lo);\n+\t\t\t}\n+\t\t\tundef @selected{$lo..$hi};\n+\t\t} elsif (my ($label) = ($f =~ /^([0-9]+)$/)) {\n+\t\t\tcheck_hunk_label($max_label, $label) or return undef;\n+\t\t\tundef $selected{$label};\n+\t\t} else {\n+\t\t\terror_msg sprintf(__(\"invalid hunk line '%s'\\n\"), $f);\n+\t\t\treturn undef;\n+\t\t}\n+\t}\n+\t[ map {\n+\t\tmy $line = $lines->[$_];\n+\t\tif ($hunk->{LINE_FLAGS}->[$line] & NO_NEWLINE) {\n+\t\t\t($line, $line + 1);\n+\t\t} else {\n+\t\t\t($line, $line);\n+\t\t}\n+\t} sort { $a <=> $b } keys(%selected) ];\n+}\n+\n+sub display_hunk_lines {\n+\tmy $hunk = shift;\n+\tmy ($display, $lines) = ($hunk->{DISPLAY}, $hunk->{LABELS}->{LINES});\n+\tmy $max_label = $#{$lines};\n+\tmy $width = int(log($max_label) / log(10)) + 1;\n+\tmy $padding = ' ' x ($width + 1);\n+\tmy $label = 1;\n+\tfor my $line (0..$#{$display}) {\n+\t\tif ($lines->[$label] == $line) {\n+\t\t\tprintf '%*d %s', $width, $label, $display->[$line];\n+\t\t\t$label++ if ($label < $max_label);\n+\t\t} else {\n+\t\t\tprint $padding . $display->[$line];\n+\t\t}\n+\t}\n+}\n+\n+sub select_lines_loop {\n+\tmy $hunk = shift;\n+\tdisplay_hunk_lines($hunk);\n+\tmy $selection = undef;\n+\tuntil (defined $selection) {\n+\t\tprint colored $prompt_color, __(\"select lines? \");\n+\t\tmy $text = ;\n+\t\tdefined $text and $text =~ /\\S/ or return undef;\n+\t\t$selection = parse_hunk_selection($hunk, $text);\n+\t}\n+\treturn select_hunk_lines($hunk, $selection);\n+}\n+\n my %edit_hunk_manually_modes = (\n \tstage => N__(\n \"If the patch applies cleanly, the edited hunk will immediately be\n@@ -1255,6 +1420,7 @@ j - leave this hunk undecided, see next undecided hunk\n J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n+l - select hunk lines to use\n s - split the current hunk into smaller hunks\n e - manually edit the current hunk\n ? - print help\n@@ -1471,6 +1637,9 @@ sub patch_update_file {\n \t\tif ($hunk[$ix]{TYPE} eq 'hunk') {\n \t\t\t$other .= ',e';\n \t\t}\n+\t\tif (label_hunk_lines($hunk[$ix])) {\n+\t\t\t$other .= ',l';\n+\t\t}\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -1610,6 +1779,18 @@ sub patch_update_file {\n \t\t\t\t\tnext;\n \t\t\t\t}\n \t\t\t}\n+\t\t\telsif ($line =~ /^l/) {\n+\t\t\t\tunless ($other =~ /l/) {\n+\t\t\t\t\terror_msg __(\"Cannot select line by line\\n\");\n+\t\t\t\t\tnext;\n+\t\t\t\t}\n+\t\t\t\tmy $newhunk = select_lines_loop($hunk[$ix]);\n+\t\t\t\tif ($newhunk) {\n+\t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n+\t\t\t\t} else {\n+\t\t\t\t\tnext;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\telsif ($other =~ /s/ && $line =~ /^s/) {\n \t\t\t\tmy @split = split_hunk($hunk[$ix]{TEXT}, $hunk[$ix]{DISPLAY});\n \t\t\t\tif (1 < @split) {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex f1bb879ea4..5b535a22d5 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -403,6 +403,101 @@ test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n \t! grep \"^+31\" actual\n '\n \n+test_expect_success 'setup test file' '\n+\ttest_write_lines 10 31 32 33 60 >test &&\n+\tprintf 61 >>test\n+'\n+\n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..341cc6b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,6 @@\n+\t 10\n+\t 20\n+\t-30\n+\t-40\n+\t-50\n+\t+31\n+\t+33\n+\t 60\n+\t+61\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can stage individual lines of patch' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l \"5-2,7-\" |\n+\tEDITOR=: git add -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..cc6163b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,7 @@\n+\t 10\n+\t 20\n+\t 30\n+\t-40\n+\t 50\n+\t+33\n+\t 60\n+\t+61\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can reset individual lines of patch' '\n+\tprintf \"%s\\n\" l 4,1,3 |\n+\tEDITOR=: git reset -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n+test_expect_success 'setup file' '\n+\ttest_write_lines ten twenty thirty forty fifty >test &&\n+\tprintf sixty >>test\n+'\n+\n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..cc6163b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,8 @@\n+\t-10\n+\t+ten\n+\t+twenty\n+\t+thirty\n+\t 20\n+\t-30\n+\t+sixty\n+\t 40\n+\t 50\n+\t 60\n+\tEOF\n+'\n+\n+test_expect_failure 'can stage modified lines of patch (1)' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l \"1,3 7-9 12\" |\n+\tEDITOR=: git add -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n test_expect_success 'patch mode ignores unmerged entries' '\n \tgit reset --hard &&\n \ttest_commit conflict &&\n@@ -571,4 +666,12 @@ test_expect_success 'add -p patch editing works with pathological context lines'\n \ttest_cmp expected-2 actual\n '\n \n+test_expect_success 'add -p selecting lines works with pathological context lines' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l 2 y |\n+\tGIT_EDITOR=./editor git add -p &&\n+\tgit cat-file blob :a >actual &&\n+\ttest_cmp expected-2 actual\n+'\n+\n test_done\n-- \n2.18.0\n\n"},{"id":"353628","messageId":"20180726102227.997-5-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180726102227.997-1-phillip.wood@talktalk.net","subject":"[PATCH v4 4/4] add -p: optimize line selection for short hunks","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-07-26T10:22:27Z","receivedAt":"2018-07-26T10:22:40Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf there are fewer than ten changes in a hunk then make spaces\noptional when selecting individual lines. This means that for short\nhunks one can just type 1-357 to stage lines 1, 2, 3, 5 & 7.\n\nSigned-off-by: Phillip Wood \n---\n Documentation/git-add.txt | 9 +++++----\n git-add--interactive.perl | 26 ++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 2 +-\n 3 files changed, 32 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 01ff4d7d24..f3c81dfb11 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -340,10 +340,11 @@ patch::\n If you press \"l\" then the hunk will be reprinted with each insertion or\n deletion labelled with a number and you will be prompted to enter which\n lines you wish to select. Individual line numbers should be separated by\n-a space or comma, to specify a range of lines use a dash between\n-them. If the upper bound of a range of lines is omitted it defaults to\n-the last line. To invert the selection prefix it with \"-\" so \"-3-5,8\"\n-will select everything except lines 3, 4, 5 and 8.\n+a space or comma (these can be omitted if there are fewer than ten\n+labelled lines), to specify a range of lines use a dash between them. If\n+the upper bound of a range of lines is omitted it defaults to the last\n+line. To invert the selection prefix it with \"-\" so \"-3-5,8\" will select\n+everything except lines 3, 4, 5 and 8.\n +\n After deciding the fate for all hunks, if there is any hunk\n that was chosen, the index is updated with the selected hunks.\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 0b490cf1e9..56bc7ab496 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1244,6 +1244,29 @@ sub check_hunk_label {\n \treturn 1;\n }\n \n+sub split_hunk_selection {\n+\tmy @fields = @_;\n+\tmy @ret;\n+\tfor my $field (@fields) {\n+\t\twhile ($field ne '') {\n+\t\t\tif ($field =~ /^[0-9]-$/) {\n+\t\t\t\tpush @ret, $field;\n+\t\t\t\tlast;\n+\t\t\t} elsif (my ($sel, $rest) =\n+\t\t\t\t\t($field =~ /^([0-9](?:-[0-9])?)(.*)/)) {\n+\t\t\t\tpush @ret, $sel;\n+\t\t\t\t$field = $rest;\n+\t\t\t} else {\n+\t\t\t\terror_msg sprintf\n+\t\t\t\t __(\"invalid hunk line '%s'\\n\"),\n+\t\t\t\t substr($field, 0, 1);\n+\t\t\t\treturn ();\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn @ret;\n+}\n+\n sub parse_hunk_selection {\n \tmy ($hunk, $line) = @_;\n \tmy $lines = $hunk->{LABELS}->{LINES};\n@@ -1263,6 +1286,9 @@ sub parse_hunk_selection {\n \t\t\t}\n \t\t}\n \t}\n+\tif ($max_label < 10) {\n+\t\t@fields = split_hunk_selection(@fields) or return undef;\n+\t}\n \tfor my $f (@fields) {\n \t\tif (my ($lo, $hi) = ($f =~ /^([0-9]+)-([0-9]*)$/)) {\n \t\t\tif ($hi eq '') {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 2fd456017f..b2b808275c 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -457,7 +457,7 @@ test_expect_success 'setup expected diff' '\n '\n \n test_expect_success 'can reset individual lines of patch' '\n-\tprintf \"%s\\n\" l -6,2,5 |\n+\tprintf \"%s\\n\" l -625 |\n \tEDITOR=: git reset -p 2>error &&\n \ttest_must_be_empty error &&\n \tgit diff --cached HEAD >actual &&\n-- \n2.18.0\n\n"},{"id":"353629","messageId":"20180726102227.997-4-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180726102227.997-1-phillip.wood@talktalk.net","subject":"[PATCH v4 3/4] add -p: allow line selection to be inverted","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-07-26T10:22:26Z","receivedAt":"2018-07-26T10:22:41Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf the list of lines to be selected begins with '-' select all the\nlines except the ones listed.\n\nSigned-off-by: Phillip Wood \n---\n Documentation/git-add.txt | 3 ++-\n git-add--interactive.perl | 19 +++++++++++++++++++\n t/t3701-add-interactive.sh | 2 +-\n 3 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 965e192a09..01ff4d7d24 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -342,7 +342,8 @@ deletion labelled with a number and you will be prompted to enter which\n lines you wish to select. Individual line numbers should be separated by\n a space or comma, to specify a range of lines use a dash between\n them. If the upper bound of a range of lines is omitted it defaults to\n-the last line.\n+the last line. To invert the selection prefix it with \"-\" so \"-3-5,8\"\n+will select everything except lines 3, 4, 5 and 8.\n +\n After deciding the fate for all hunks, if there is any hunk\n that was chosen, the index is updated with the selected hunks.\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex b7e3da0210..0b490cf1e9 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1248,8 +1248,21 @@ sub parse_hunk_selection {\n \tmy ($hunk, $line) = @_;\n \tmy $lines = $hunk->{LABELS}->{LINES};\n \tmy $max_label = $#{$lines};\n+\tmy $invert = undef;\n \tmy %selected;\n \tmy @fields = split(/[,\\s]+/, $line);\n+\tif (my ($rest) = ($fields[0] =~ /^-(.*)/)) {\n+\t\t$invert = 1;\n+\t\tif ($rest ne '') {\n+\t\t\t$fields[0] = $rest;\n+\t\t} else {\n+\t\t\tshift @fields;\n+\t\t\tunless (@fields) {\n+\t\t\t\terror_msg __(\"no lines to invert\\n\");\n+\t\t\t\treturn undef;\n+\t\t\t}\n+\t\t}\n+\t}\n \tfor my $f (@fields) {\n \t\tif (my ($lo, $hi) = ($f =~ /^([0-9]+)-([0-9]*)$/)) {\n \t\t\tif ($hi eq '') {\n@@ -1269,6 +1282,12 @@ sub parse_hunk_selection {\n \t\t\treturn undef;\n \t\t}\n \t}\n+\tif ($invert) {\n+\t\tmy %inverted;\n+\t\tundef @inverted{1..$max_label};\n+\t\tdelete @inverted{keys(%selected)};\n+\t\t%selected = %inverted;\n+\t}\n \treturn process_hunk_selection($hunk, keys(%selected));\n }\n \ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 1d917ad018..2fd456017f 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -457,7 +457,7 @@ test_expect_success 'setup expected diff' '\n '\n \n test_expect_success 'can reset individual lines of patch' '\n-\tprintf \"%s\\n\" l 4,1,3 |\n+\tprintf \"%s\\n\" l -6,2,5 |\n \tEDITOR=: git reset -p 2>error &&\n \ttest_must_be_empty error &&\n \tgit diff --cached HEAD >actual &&\n-- \n2.18.0\n\n"},{"id":"353630","messageId":"20180726102227.997-3-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180726102227.997-1-phillip.wood@talktalk.net","subject":"[RFC PATCH v4 2/4] add -p: select modified lines correctly","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-07-26T10:22:25Z","receivedAt":"2018-07-26T10:22:43Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen a set of lines is modified the hunk contains deletions followed\nby insertions. To correctly stage a subset of the modified lines we\nneed to match up the selected deletions with the selected insertions\notherwise we end up with deletions and context lines followed by\ninsertions which is not what we want.\n\nFor example given the hunk\n 1 -* a longer description of the\n 2 - first item\n 3 -* second\n 4 -* third\n 5 +* first\n 6 + second item\n 7 +* the third item\n\nIf the user selects 1,2,4–5,7 then we should generate\n\t-* a longer description of the\n\t- first item\n\t+* first\n\t * second\n\t-* third\n\t+* the third item\n\nnot\n\t-* a longer description of the\n\t- first item\n\t * second\n\t-* third\n\t+* first\n\t+* the third item\n\nCurrently the code can only cope with selections that contain the same\nnumber of groups of insertions and deletions, though each group need\nnot contain the same number of insertions and deletions. If the user\nwants to stage an unpaired deletion or insertion in a hunk where they\nalso want to stage modified lines they have to do it with two\ninvocations of 'git add -p'.\n\nIt would be possible to add some syntax to allow lines to be excluded\nfrom groups to allow the user to stage such changes in a single\ngo. It may also be useful to allow users to explicitly group lines. If\nin the example above the second item is deleted we have\n\n 1 -* a longer description of the\n 2 - first item\n 3 -* second\n 4 -* third\n 5 +* first\n 6 +* the third item\n\nSelecting 1,2,4–6 will give an error. If lines could be grouped\nexplicitly then it would be possible to type something like\n1,2,4,[5],6 to indicate that there are two groups of insertions giving\n\n\t-* a longer description of the\n\t- first item\n\t+* first\n\t * second\n\t-* third\n\t+* the third item\n\nWe may want to be able to stage an insertion before an unselected\ndeletion to allow the user to stage a new paragraph before the\nunmodified original in\n\n 1 -original\n 2 +a new paragraph before\n 3 +original\n 4 +\n 5 +modified original\n\nby specifying something like ^2-4 to give\n\n\t+a new paragraph before\n\t+original\n\t+\n\t original\n\nI'm not sure how common these cases are in real life and how much\neffort it's worth putting into handling them at the moment when the\nuser can edit the hunk if need be. Perhaps it would be better to leave\nit for future extensions when it becomes clearer what would be most\nuseful.\n\nReported-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 148 ++++++++++++++++++++++++++++++++++---\n t/t3701-add-interactive.sh | 108 ++++++++++++++++++++++++++-\n 2 files changed, 243 insertions(+), 13 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex cbc9e5698a..b7e3da0210 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1015,26 +1015,47 @@ sub color_diff {\n \n use constant {\n \tNO_NEWLINE => 1,\n+\tLAST_ADD_DEL => 2,\n+\tFIRST_ADD => 4\n };\n \n sub label_hunk_lines {\n \tmy $hunk = shift;\n \tmy $text = $hunk->{TEXT};\n-\tmy (@line_flags, @lines);\n+\t# A block contains the insertions and deletions occurring context\n+\t# lines.\n+\tmy (@blocks, @line_flags, @lines, @modes);\n \tmy ($block, $label, $last_mode) = (0, 0, '');\n \tfor my $line (1..$#{$text}) {\n \t\t$line_flags[$line] = 0;\n \t\tmy $mode = substr($text->[$line], 0, 1);\n \t\tif ($mode eq '\\\\') {\n \t\t\t$line_flags[$line - 1] |= NO_NEWLINE;\n \t\t}\n+\t\tif ($mode ne '-' and $last_mode eq '-' or\n+\t\t $mode ne '+' and $last_mode eq '+') {\n+\t\t\t$line_flags[$line - 1] |= LAST_ADD_DEL;\n+\t\t}\n+\t\tif ($mode eq '+' and $last_mode ne '+') {\n+\t\t\t$line_flags[$line] |= FIRST_ADD;\n+\t\t}\n \t\tif ($mode eq '-' or $mode eq '+') {\n-\t\t\t$lines[++$label] = $line;\n+\t\t\t$blocks[++$label] = $block;\n+\t\t\t$lines[$label] = $line;\n+\t\t\t$modes[$label] = $mode;\n+\t\t} elsif ($mode eq ' ' and $last_mode ne ' ') {\n+\t\t\t$block++;\n \t\t}\n+\t\t$last_mode = $mode;\n+\t}\n+\tif ($last_mode eq '-' or $last_mode eq '+') {\n+\t\t$line_flags[-1] |= LAST_ADD_DEL;\n \t}\n \tif ($label > 1) {\n \t\t$hunk->{LABELS} = {\n+\t\t\tBLOCKS => \\@blocks,\n \t\t\tLINES => \\@lines,\n+\t\t\tMODES => \\@modes\n \t\t};\n \t\t$hunk->{LINE_FLAGS} = \\@line_flags;\n \t\treturn 1;\n@@ -1061,11 +1082,14 @@ sub select_hunk_lines {\n \t\t}\n \t};\n \n-\tmy ($lo, $hi) = splice(@$selected, 0, 2);\n+\tmy ($lo, $hi, $pre_insert, $post_insert) = splice(@$selected, 0, 4);\n \t# Lines with this mode will become context lines if they are\n \t# not selected\n \tmy $context_mode = $patch_mode_flavour{IS_REVERSE} ? '+' : '-';\n \tfor $i (1..$#{$text}) {\n+\t\tif ($i == $lo and defined($pre_insert)) {\n+\t\t\t$select_lines->(@$pre_insert);\n+\t\t}\n \t\tif ($lo <= $i and $i <= $hi) {\n \t\t\t$select_lines->($i);\n \t\t} else {\n@@ -1080,8 +1104,12 @@ sub select_hunk_lines {\n \t\t\t}\n \t\t}\n \t\tif ($i == $hi) {\n+\t\t\tif (defined($post_insert)) {\n+\t\t\t\t$select_lines->(@$post_insert);\n+\t\t\t}\n \t\t\tif (@$selected) {\n-\t\t\t\t($lo, $hi) = splice(@$selected, 0, 2);\n+\t\t\t\t($lo, $hi, $pre_insert, $post_insert) =\n+\t\t\t\t\t\tsplice(@$selected, 0, 4);\n \t\t\t}\n \t\t}\n \t}\n@@ -1104,6 +1132,109 @@ sub select_hunk_lines {\n \treturn $newhunk;\n }\n \n+sub pair_hunk_selection {\n+\tmy $line_flags = shift;\n+\tmy @ret;\n+\tmy $reverse = $patch_mode_flavour{IS_REVERSE};\n+\tfor my $sel (@_) {\n+\t\tmy $i = 0;\n+\t\tmy ($del, $add) = @$sel;\n+\t\tmy $ndel = @$del / 2;\n+\t\tmy $nadd = @$add / 2;\n+\t\t# If there the same number of insertion and deletion\n+\t\t# groups then pair up in insertions and deletions.\n+\t\tif ($nadd == $ndel) {\n+\t\t\twhile ($i < @{$del}) {\n+\t\t\t\tmy ($ra, $rb) = $reverse ? ($add, $del) :\n+\t\t\t\t\t\t\t ($del, $add);\n+\t\t\t\tmy ($starta, $enda) = @{$ra}[$i, $i + 1];\n+\t\t\t\tmy ($startb, $endb) = @{$rb}[$i, $i + 1];\n+\t\t\t\tif ($line_flags->[$enda] & LAST_ADD_DEL and\n+\t\t\t\t $line_flags->[$endb] & NO_NEWLINE) {\n+\t\t\t\t\t$endb++;\n+\t\t\t\t}\n+\t\t\t\tif ($line_flags->[$enda] & NO_NEWLINE) {\n+\t\t\t\t\t$enda++;\n+\t\t\t\t}\n+\t\t\t\tpush @ret, $reverse ?\n+\t\t\t\t\t($starta, $enda,\n+\t\t\t\t\t [ $startb..$endb ], undef) :\n+\t\t\t\t\t($starta, $enda,\n+\t\t\t\t\t undef, [ $startb..$endb ]);\n+\t\t\t\t$i += 2;\n+\t\t\t}\n+\t\t}\n+\t\t# If there are only deletions or only insertions or\n+\t\t# a) we're adding to the index and there is a single group\n+\t\t# of insertions starting at the first inserted line\n+\t\t# or\n+\t\t# b) we're resetting the index and there is a single group\n+\t\t# of deletions finishing with the last deleted line\n+\t\t# then it is okay just to have deletions followed by\n+\t\t# insertions.\n+\t\t} elsif (!$nadd or !$ndel or\n+\t\t\t (!$reverse and $ndel == 1 and\n+\t\t\t $line_flags->[$del->[1]] & LAST_ADD_DEL) or\n+\t\t\t ($reverse and $nadd == 1 and\n+\t\t\t $line_flags->[$add->[0]] & FIRST_ADD)) {\n+\t\t\twhile ($i < @$del) {\n+\t\t\t\tmy ($start, $end) = @{$del}[$i, $i + 1];\n+\t\t\t\tif ($line_flags->[$end] & NO_NEWLINE) {\n+\t\t\t\t\t$end++;\n+\t\t\t\t}\n+\t\t\t\tpush @ret, $start, $end, undef, undef;\n+\t\t\t\t$i += 2;\n+\t\t\t}\n+\t\t\t$i = 0;\n+\t\t\twhile ($i < @$add) {\n+\t\t\t\tmy ($start, $end) = @{$add}[$i, $i + 1];\n+\t\t\t\tif ($line_flags->[$end] & NO_NEWLINE) {\n+\t\t\t\t\t$end++;\n+\t\t\t\t}\n+\t\t\t\tpush @ret, $start, $end, undef, undef;\n+\t\t\t\t$i += 2;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tprintf STDERR __(\"unable to pair up insertions and deletions\\n\");\n+\t\t\treturn undef;\n+\t\t}\n+\t}\n+\treturn \\@ret;\n+}\n+\n+sub process_hunk_selection {\n+\tmy ($labels, $line_flags) = @{shift()}{qw(LABELS LINE_FLAGS)};\n+\tmy ($blocks, $lines, $modes) =\n+\t\t\t\t@{$labels}{qw(BLOCKS LINES MODES)};\n+\tmy @selection = sort { $a <=> $b } @_;\n+\tunless (@selection) {\n+\t\treturn [];\n+\t}\n+\tmy $last_label = shift @selection;\n+\tmy ($last_block, $last_line, $last_mode) =\n+\t ($blocks->[$last_label], $lines->[$last_label], $modes->[$last_label]);\n+\tmy ($del, $add) = ([], []);\n+\tpush @{$last_mode eq '-' ? $del : $add}, $last_line;\n+\tmy @sel;\n+\tfor my $label (@selection) {\n+\t\tmy ($block, $line, $mode) =\n+\t\t\t($blocks->[$label], $lines->[$label], $modes->[$label]);\n+\t\tif ($block != $last_block) {\n+\t\t\tpush @{$last_mode eq '-' ? $del : $add}, $last_line;\n+\t\t\tpush @sel, [ $del, $add ];\n+\t\t\t($del, $add) = ([], []);\n+\t\t\tpush @{$mode eq '-' ? $del : $add}, $line;\n+\t\t} elsif ($line != $last_line + 1 or $mode ne $last_mode) {\n+\t\t\tpush @{$last_mode eq '-' ? $del : $add}, $last_line;\n+\t\t\tpush @{$mode eq '-' ? $del : $add}, $line;\n+\t\t}\n+\t\t($last_block, $last_line, $last_mode) = ($block, $line, $mode);\n+\t}\n+\tpush @{$last_mode eq '-' ? $del : $add}, $last_line;\n+\tpush @sel, [ $del, $add ];\n+\tpair_hunk_selection($line_flags, @sel);\n+}\n+\n sub check_hunk_label {\n \tmy ($max_label, $label) = @_;\n \tif ($label < 1 or $label > $max_label) {\n@@ -1138,14 +1269,7 @@ sub parse_hunk_selection {\n \t\t\treturn undef;\n \t\t}\n \t}\n-\t[ map {\n-\t\tmy $line = $lines->[$_];\n-\t\tif ($hunk->{LINE_FLAGS}->[$line] & NO_NEWLINE) {\n-\t\t\t($line, $line + 1);\n-\t\t} else {\n-\t\t\t($line, $line);\n-\t\t}\n-\t} sort { $a <=> $b } keys(%selected) ];\n+\treturn process_hunk_selection($hunk, keys(%selected));\n }\n \n sub display_hunk_lines {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 5b535a22d5..1d917ad018 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -489,7 +489,7 @@ test_expect_success 'setup expected diff' '\n \tEOF\n '\n \n-test_expect_failure 'can stage modified lines of patch (1)' '\n+test_expect_success 'can stage modified lines of patch (1)' '\n \tgit reset &&\n \tprintf \"%s\\n\" l \"1,3 7-9 12\" |\n \tEDITOR=: git add -p 2>error &&\n@@ -498,6 +498,112 @@ test_expect_failure 'can stage modified lines of patch (1)' '\n \tdiff_cmp expected actual\n '\n \n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..cc6163b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,7 @@\n+\t 10\n+\t-20\n+\t+twenty\n+\t+thirty\n+\t 30\n+\t 40\n+\t 50\n+\t-60\n+\t+sixty\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can stage modified lines of patch (2)' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l \"2,6,8,9,12\" |\n+\tEDITOR=: git add -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n+test_expect_success 'setup HEAD' '\n+\tgit reset &&\n+\tgit apply --cached <<-\\EOF &&\n+\tdiff --git a/test b/test\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -3,4 +3,4 @@\n+\t 30\n+\t 40\n+\t 50\n+\t-60\n+\t+60\n+\t\\ No newline at end of file\n+\tEOF\n+\tgit commit\n+'\n+\n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..cc6163b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,8 @@\n+\t-10\n+\t+ten\n+\t+twenty\n+\t+thirty\n+\t 20\n+\t-30\n+\t+fifty\n+\t 40\n+\t 50\n+\t 60\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can stage modified lines of patch (3)' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l \"11,1,3 7-9\" |\n+\tEDITOR=: git add -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..cc6163b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,8 @@\n+\t-10\n+\t+ten\n+\t+twenty\n+\t+thirty\n+\t 20\n+\t 30\n+\t 40\n+\t 50\n+\t-60\n+\t\\ No newline at end of file\n+\t+fifty\n+\tEOF\n+'\n+\n+test_expect_success 'can stage modified lines of patch (4)' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l \"9-6 11,1\" |\n+\tEDITOR=: git add -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n test_expect_success 'patch mode ignores unmerged entries' '\n \tgit reset --hard &&\n \ttest_commit conflict &&\n-- \n2.18.0\n\n"},{"id":"353650","messageId":"20180726155854.20832-1-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180219113619.26566-1-phillip.wood@talktalk.net","subject":"[RFC PATCH v5 0/4] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-07-26T15:58:50Z","receivedAt":"2018-07-26T16:06:25Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nUnfortuantely v4 had test failures due to a suprious brace from a last\nminute edit to a comment that I forgot to test. This version fixes\nthat, my applogies for the patch churn.\n\nI've updated this series based on Ævar's feedback on v3 (to paraphrase\nstop using '$_' so much and fix staging modified lines.). The first\npatch is functionally equivalent to the previous version but with a\nreworked implementation. Patch 2 is new, it implements correctly\nstaging modified lines with a couple of limitations - see the commit\nmessage for more details, I'm keen to get some feedback on it. Patches\n3 and 4 are essentially rebased and tweaked versions of patches 2 and\n3 from the previous version.\n\nThis series is based on pw/add-p-recount (f4d35a6b49 \"add -p: fix\ncounting empty context lines in edited patches\")\n\nThe motivation for this series is summed up in the first commit\nmessage:\n\n\"When I end up editing hunks it is almost always because I want to\nstage a subset of the lines in the hunk. Doing this by editing the\nhunk is inconvenient and error prone (especially so if the patch is\ngoing to be reversed before being applied). Instead offer an option\nfor add -p to stage individual lines. When the user presses 'l' the\nhunk is redrawn with labels by the insertions and deletions and they\nare prompted to enter a list of the lines they wish to stage. Ranges\nof lines may be specified using 'a-b' where either 'a' or 'b' may be\nomitted to mean all lines from 'a' to the end of the hunk or all lines\nfrom 1 upto and including 'b'.\"\n\nPhillip Wood (4):\n add -p: select individual hunk lines\n add -p: select modified lines correctly\n add -p: allow line selection to be inverted\n add -p: optimize line selection for short hunks\n\n Documentation/git-add.txt | 10 ++\n git-add--interactive.perl | 349 +++++++++++++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 209 ++++++++++++++++++++++\n 3 files changed, 568 insertions(+)\n\n-- \n2.18.0\n\n"},{"id":"353651","messageId":"20180726155854.20832-2-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180726155854.20832-1-phillip.wood@talktalk.net","subject":"[PATCH v5 1/4] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-07-26T15:58:51Z","receivedAt":"2018-07-26T16:06:26Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen I end up editing hunks it is almost always because I want to\nstage a subset of the lines in the hunk. Doing this by editing the\nhunk is inconvenient and error prone (especially so if the patch is\ngoing to be reversed before being applied). Instead offer an option\nfor add -p to stage individual lines. When the user presses 'l' the\nhunk is redrawn with labels by the insertions and deletions and they\nare prompted to enter a list of the lines they wish to stage. Ranges\nof lines may be specified using 'a-b' where 'b' may be omitted to mean\nall lines from 'a' to the end of the hunk. Modified lines are not\nhandled correctly, that will be fixed in the next commit.\n\nSigned-off-by: Phillip Wood \n---\n Documentation/git-add.txt | 8 ++\n git-add--interactive.perl | 181 +++++++++++++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 103 +++++++++++++++++++++\n 3 files changed, 292 insertions(+)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex d50fa339dc..965e192a09 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -332,10 +332,18 @@ patch::\n J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n+ l - select hunk lines to use\n s - split the current hunk into smaller hunks\n e - manually edit the current hunk\n ? - print help\n +\n+If you press \"l\" then the hunk will be reprinted with each insertion or\n+deletion labelled with a number and you will be prompted to enter which\n+lines you wish to select. Individual line numbers should be separated by\n+a space or comma, to specify a range of lines use a dash between\n+them. If the upper bound of a range of lines is omitted it defaults to\n+the last line.\n++\n After deciding the fate for all hunks, if there is any hunk\n that was chosen, the index is updated with the selected hunks.\n +\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 8361ef45e7..cbc9e5698a 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1013,6 +1013,171 @@ sub color_diff {\n \t} @_;\n }\n \n+use constant {\n+\tNO_NEWLINE => 1,\n+};\n+\n+sub label_hunk_lines {\n+\tmy $hunk = shift;\n+\tmy $text = $hunk->{TEXT};\n+\tmy (@line_flags, @lines);\n+\tmy ($block, $label, $last_mode) = (0, 0, '');\n+\tfor my $line (1..$#{$text}) {\n+\t\t$line_flags[$line] = 0;\n+\t\tmy $mode = substr($text->[$line], 0, 1);\n+\t\tif ($mode eq '\\\\') {\n+\t\t\t$line_flags[$line - 1] |= NO_NEWLINE;\n+\t\t}\n+\t\tif ($mode eq '-' or $mode eq '+') {\n+\t\t\t$lines[++$label] = $line;\n+\t\t}\n+\t}\n+\tif ($label > 1) {\n+\t\t$hunk->{LABELS} = {\n+\t\t\tLINES => \\@lines,\n+\t\t};\n+\t\t$hunk->{LINE_FLAGS} = \\@line_flags;\n+\t\treturn 1;\n+\t}\n+\treturn 0;\n+}\n+\n+sub select_hunk_lines {\n+\tmy ($hunk, $selected) = @_;\n+\tmy ($line_flags, $text) = @{$hunk}{qw(LINE_FLAGS TEXT)};\n+\tmy ($i, $o_cnt, $n_cnt) = (0, 0, 0);\n+\tmy @newtext;\n+\n+\tmy $select_lines = sub {\n+\t\tfor my $i (@_) {\n+\t\t\tmy $line = $text->[$i];\n+\t\t\tmy $mode = substr($line, 0, 1);\n+\t\t\tpush @newtext, $line;\n+\t\t\tif ($mode eq '+') {\n+\t\t\t\t$n_cnt++;\n+\t\t\t} elsif ($mode eq '-') {\n+\t\t\t\t$o_cnt++;\n+\t\t\t}\n+\t\t}\n+\t};\n+\n+\tmy ($lo, $hi) = splice(@$selected, 0, 2);\n+\t# Lines with this mode will become context lines if they are\n+\t# not selected\n+\tmy $context_mode = $patch_mode_flavour{IS_REVERSE} ? '+' : '-';\n+\tfor $i (1..$#{$text}) {\n+\t\tif ($lo <= $i and $i <= $hi) {\n+\t\t\t$select_lines->($i);\n+\t\t} else {\n+\t\t\tmy $line = $text->[$i];\n+\t\t\tmy $mode = substr($line, 0, 1);\n+\t\t\tif ($mode eq ' ' or $mode eq $context_mode) {\n+\t\t\t\tpush @newtext, ' ' . substr($line, 1);\n+\t\t\t\t$o_cnt++; $n_cnt++;\n+\t\t\t\tif ($line_flags->[$i] & NO_NEWLINE) {\n+\t\t\t\t\tpush @newtext, $text->[$i + 1];\n+\t\t\t\t}\n+\t\t\t}\n+\t\t}\n+\t\tif ($i == $hi) {\n+\t\t\tif (@$selected) {\n+\t\t\t\t($lo, $hi) = splice(@$selected, 0, 2);\n+\t\t\t}\n+\t\t}\n+\t}\n+\tmy ($o_ofs, $orig_o_cnt, $n_ofs, $orig_n_cnt) =\n+\t\t\t\t\tparse_hunk_header($text->[0]);\n+\tunshift @newtext, format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);\n+\tmy $newhunk = {\n+\t\tTEXT => \\@newtext,\n+\t\tDISPLAY => [ color_diff(@newtext) ],\n+\t\tOFS_DELTA => $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt,\n+\t\tTYPE => $hunk->{TYPE},\n+\t\tUSE => 1,\n+\t};\n+\t# If this hunk has previously been edited add the offset delta\n+\t# of the old hunk to get the real delta from the original\n+\t# hunk.\n+\tif ($hunk->{OFS_DELTA}) {\n+\t\t$newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};\n+\t}\n+\treturn $newhunk;\n+}\n+\n+sub check_hunk_label {\n+\tmy ($max_label, $label) = @_;\n+\tif ($label < 1 or $label > $max_label) {\n+\t\terror_msg sprintf(__(\"invalid hunk line '%d'\\n\"), $label);\n+\t\treturn 0;\n+\t}\n+\treturn 1;\n+}\n+\n+sub parse_hunk_selection {\n+\tmy ($hunk, $line) = @_;\n+\tmy $lines = $hunk->{LABELS}->{LINES};\n+\tmy $max_label = $#{$lines};\n+\tmy %selected;\n+\tmy @fields = split(/[,\\s]+/, $line);\n+\tfor my $f (@fields) {\n+\t\tif (my ($lo, $hi) = ($f =~ /^([0-9]+)-([0-9]*)$/)) {\n+\t\t\tif ($hi eq '') {\n+\t\t\t\t$hi = $max_label;\n+\t\t\t}\n+\t\t\tcheck_hunk_label($max_label, $lo) or return undef;\n+\t\t\tcheck_hunk_label($max_label, $hi) or return undef;\n+\t\t\tif ($hi < $lo) {\n+\t\t\t\t($lo, $hi) = ($hi, $lo);\n+\t\t\t}\n+\t\t\tundef @selected{$lo..$hi};\n+\t\t} elsif (my ($label) = ($f =~ /^([0-9]+)$/)) {\n+\t\t\tcheck_hunk_label($max_label, $label) or return undef;\n+\t\t\tundef $selected{$label};\n+\t\t} else {\n+\t\t\terror_msg sprintf(__(\"invalid hunk line '%s'\\n\"), $f);\n+\t\t\treturn undef;\n+\t\t}\n+\t}\n+\t[ map {\n+\t\tmy $line = $lines->[$_];\n+\t\tif ($hunk->{LINE_FLAGS}->[$line] & NO_NEWLINE) {\n+\t\t\t($line, $line + 1);\n+\t\t} else {\n+\t\t\t($line, $line);\n+\t\t}\n+\t} sort { $a <=> $b } keys(%selected) ];\n+}\n+\n+sub display_hunk_lines {\n+\tmy $hunk = shift;\n+\tmy ($display, $lines) = ($hunk->{DISPLAY}, $hunk->{LABELS}->{LINES});\n+\tmy $max_label = $#{$lines};\n+\tmy $width = int(log($max_label) / log(10)) + 1;\n+\tmy $padding = ' ' x ($width + 1);\n+\tmy $label = 1;\n+\tfor my $line (0..$#{$display}) {\n+\t\tif ($lines->[$label] == $line) {\n+\t\t\tprintf '%*d %s', $width, $label, $display->[$line];\n+\t\t\t$label++ if ($label < $max_label);\n+\t\t} else {\n+\t\t\tprint $padding . $display->[$line];\n+\t\t}\n+\t}\n+}\n+\n+sub select_lines_loop {\n+\tmy $hunk = shift;\n+\tdisplay_hunk_lines($hunk);\n+\tmy $selection = undef;\n+\tuntil (defined $selection) {\n+\t\tprint colored $prompt_color, __(\"select lines? \");\n+\t\tmy $text = ;\n+\t\tdefined $text and $text =~ /\\S/ or return undef;\n+\t\t$selection = parse_hunk_selection($hunk, $text);\n+\t}\n+\treturn select_hunk_lines($hunk, $selection);\n+}\n+\n my %edit_hunk_manually_modes = (\n \tstage => N__(\n \"If the patch applies cleanly, the edited hunk will immediately be\n@@ -1255,6 +1420,7 @@ j - leave this hunk undecided, see next undecided hunk\n J - leave this hunk undecided, see next hunk\n k - leave this hunk undecided, see previous undecided hunk\n K - leave this hunk undecided, see previous hunk\n+l - select hunk lines to use\n s - split the current hunk into smaller hunks\n e - manually edit the current hunk\n ? - print help\n@@ -1471,6 +1637,9 @@ sub patch_update_file {\n \t\tif ($hunk[$ix]{TYPE} eq 'hunk') {\n \t\t\t$other .= ',e';\n \t\t}\n+\t\tif (label_hunk_lines($hunk[$ix])) {\n+\t\t\t$other .= ',l';\n+\t\t}\n \t\tfor (@{$hunk[$ix]{DISPLAY}}) {\n \t\t\tprint;\n \t\t}\n@@ -1610,6 +1779,18 @@ sub patch_update_file {\n \t\t\t\t\tnext;\n \t\t\t\t}\n \t\t\t}\n+\t\t\telsif ($line =~ /^l/) {\n+\t\t\t\tunless ($other =~ /l/) {\n+\t\t\t\t\terror_msg __(\"Cannot select line by line\\n\");\n+\t\t\t\t\tnext;\n+\t\t\t\t}\n+\t\t\t\tmy $newhunk = select_lines_loop($hunk[$ix]);\n+\t\t\t\tif ($newhunk) {\n+\t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n+\t\t\t\t} else {\n+\t\t\t\t\tnext;\n+\t\t\t\t}\n+\t\t\t}\n \t\t\telsif ($other =~ /s/ && $line =~ /^s/) {\n \t\t\t\tmy @split = split_hunk($hunk[$ix]{TEXT}, $hunk[$ix]{DISPLAY});\n \t\t\t\tif (1 < @split) {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex f1bb879ea4..5b535a22d5 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -403,6 +403,101 @@ test_expect_failure 'split hunk \"add -p (no, yes, edit)\"' '\n \t! grep \"^+31\" actual\n '\n \n+test_expect_success 'setup test file' '\n+\ttest_write_lines 10 31 32 33 60 >test &&\n+\tprintf 61 >>test\n+'\n+\n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..341cc6b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,6 @@\n+\t 10\n+\t 20\n+\t-30\n+\t-40\n+\t-50\n+\t+31\n+\t+33\n+\t 60\n+\t+61\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can stage individual lines of patch' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l \"5-2,7-\" |\n+\tEDITOR=: git add -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..cc6163b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,7 @@\n+\t 10\n+\t 20\n+\t 30\n+\t-40\n+\t 50\n+\t+33\n+\t 60\n+\t+61\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can reset individual lines of patch' '\n+\tprintf \"%s\\n\" l 4,1,3 |\n+\tEDITOR=: git reset -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n+test_expect_success 'setup file' '\n+\ttest_write_lines ten twenty thirty forty fifty >test &&\n+\tprintf sixty >>test\n+'\n+\n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..cc6163b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,8 @@\n+\t-10\n+\t+ten\n+\t+twenty\n+\t+thirty\n+\t 20\n+\t-30\n+\t+sixty\n+\t 40\n+\t 50\n+\t 60\n+\tEOF\n+'\n+\n+test_expect_failure 'can stage modified lines of patch (1)' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l \"1,3 7-9 12\" |\n+\tEDITOR=: git add -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n test_expect_success 'patch mode ignores unmerged entries' '\n \tgit reset --hard &&\n \ttest_commit conflict &&\n@@ -571,4 +666,12 @@ test_expect_success 'add -p patch editing works with pathological context lines'\n \ttest_cmp expected-2 actual\n '\n \n+test_expect_success 'add -p selecting lines works with pathological context lines' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l 2 y |\n+\tGIT_EDITOR=./editor git add -p &&\n+\tgit cat-file blob :a >actual &&\n+\ttest_cmp expected-2 actual\n+'\n+\n test_done\n-- \n2.18.0\n\n"},{"id":"353652","messageId":"20180726155854.20832-3-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180726155854.20832-1-phillip.wood@talktalk.net","subject":"[RFC PATCH v5 2/4] add -p: select modified lines correctly","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-07-26T15:58:52Z","receivedAt":"2018-07-26T16:06:27Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nWhen a set of lines is modified the hunk contains deletions followed\nby insertions. To correctly stage a subset of the modified lines we\nneed to match up the selected deletions with the selected insertions\notherwise we end up with deletions and context lines followed by\ninsertions which is not what we want.\n\nFor example given the hunk\n 1 -* a longer description of the\n 2 - first item\n 3 -* second\n 4 -* third\n 5 +* first\n 6 + second item\n 7 +* the third item\n\nIf the user selects 1,2,4–5,7 then we should generate\n\t-* a longer description of the\n\t- first item\n\t+* first\n\t * second\n\t-* third\n\t+* the third item\n\nnot\n\t-* a longer description of the\n\t- first item\n\t * second\n\t-* third\n\t+* first\n\t+* the third item\n\nCurrently the code can only cope with selections that contain the same\nnumber of groups of insertions and deletions, though each group need\nnot contain the same number of insertions and deletions. If the user\nwants to stage an unpaired deletion or insertion in a hunk where they\nalso want to stage modified lines they have to do it with two\ninvocations of 'git add -p'.\n\nIt would be possible to add some syntax to allow lines to be excluded\nfrom groups to allow the user to stage such changes in a single\ngo. It may also be useful to allow users to explicitly group lines. If\nin the example above the second item is deleted we have\n\n 1 -* a longer description of the\n 2 - first item\n 3 -* second\n 4 -* third\n 5 +* first\n 6 +* the third item\n\nSelecting 1,2,4–6 will give an error. If lines could be grouped\nexplicitly then it would be possible to type something like\n1,2,4,[5],6 to indicate that there are two groups of insertions giving\n\n\t-* a longer description of the\n\t- first item\n\t+* first\n\t * second\n\t-* third\n\t+* the third item\n\nWe may want to be able to stage an insertion before an unselected\ndeletion to allow the user to stage a new paragraph before the\nunmodified original in\n\n 1 -original\n 2 +a new paragraph before\n 3 +original\n 4 +\n 5 +modified original\n\nby specifying something like ^2-4 to give\n\n\t+a new paragraph before\n\t+original\n\t+\n\t original\n\nI'm not sure how common these cases are in real life and how much\neffort it's worth putting into handling them at the moment when the\nuser can edit the hunk if need be. Perhaps it would be better to leave\nit for future extensions when it becomes clearer what would be most\nuseful.\n\nReported-by: Ævar Arnfjörð Bjarmason \nSigned-off-by: Phillip Wood \n---\n git-add--interactive.perl | 147 ++++++++++++++++++++++++++++++++++---\n t/t3701-add-interactive.sh | 108 ++++++++++++++++++++++++++-\n 2 files changed, 242 insertions(+), 13 deletions(-)\n\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex cbc9e5698a..7e4daee2fc 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1015,26 +1015,47 @@ sub color_diff {\n \n use constant {\n \tNO_NEWLINE => 1,\n+\tLAST_ADD_DEL => 2,\n+\tFIRST_ADD => 4\n };\n \n sub label_hunk_lines {\n \tmy $hunk = shift;\n \tmy $text = $hunk->{TEXT};\n-\tmy (@line_flags, @lines);\n+\t# A block contains the insertions and deletions occurring context\n+\t# lines.\n+\tmy (@blocks, @line_flags, @lines, @modes);\n \tmy ($block, $label, $last_mode) = (0, 0, '');\n \tfor my $line (1..$#{$text}) {\n \t\t$line_flags[$line] = 0;\n \t\tmy $mode = substr($text->[$line], 0, 1);\n \t\tif ($mode eq '\\\\') {\n \t\t\t$line_flags[$line - 1] |= NO_NEWLINE;\n \t\t}\n+\t\tif ($mode ne '-' and $last_mode eq '-' or\n+\t\t $mode ne '+' and $last_mode eq '+') {\n+\t\t\t$line_flags[$line - 1] |= LAST_ADD_DEL;\n+\t\t}\n+\t\tif ($mode eq '+' and $last_mode ne '+') {\n+\t\t\t$line_flags[$line] |= FIRST_ADD;\n+\t\t}\n \t\tif ($mode eq '-' or $mode eq '+') {\n-\t\t\t$lines[++$label] = $line;\n+\t\t\t$blocks[++$label] = $block;\n+\t\t\t$lines[$label] = $line;\n+\t\t\t$modes[$label] = $mode;\n+\t\t} elsif ($mode eq ' ' and $last_mode ne ' ') {\n+\t\t\t$block++;\n \t\t}\n+\t\t$last_mode = $mode;\n+\t}\n+\tif ($last_mode eq '-' or $last_mode eq '+') {\n+\t\t$line_flags[-1] |= LAST_ADD_DEL;\n \t}\n \tif ($label > 1) {\n \t\t$hunk->{LABELS} = {\n+\t\t\tBLOCKS => \\@blocks,\n \t\t\tLINES => \\@lines,\n+\t\t\tMODES => \\@modes\n \t\t};\n \t\t$hunk->{LINE_FLAGS} = \\@line_flags;\n \t\treturn 1;\n@@ -1061,11 +1082,14 @@ sub select_hunk_lines {\n \t\t}\n \t};\n \n-\tmy ($lo, $hi) = splice(@$selected, 0, 2);\n+\tmy ($lo, $hi, $pre_insert, $post_insert) = splice(@$selected, 0, 4);\n \t# Lines with this mode will become context lines if they are\n \t# not selected\n \tmy $context_mode = $patch_mode_flavour{IS_REVERSE} ? '+' : '-';\n \tfor $i (1..$#{$text}) {\n+\t\tif ($i == $lo and defined($pre_insert)) {\n+\t\t\t$select_lines->(@$pre_insert);\n+\t\t}\n \t\tif ($lo <= $i and $i <= $hi) {\n \t\t\t$select_lines->($i);\n \t\t} else {\n@@ -1080,8 +1104,12 @@ sub select_hunk_lines {\n \t\t\t}\n \t\t}\n \t\tif ($i == $hi) {\n+\t\t\tif (defined($post_insert)) {\n+\t\t\t\t$select_lines->(@$post_insert);\n+\t\t\t}\n \t\t\tif (@$selected) {\n-\t\t\t\t($lo, $hi) = splice(@$selected, 0, 2);\n+\t\t\t\t($lo, $hi, $pre_insert, $post_insert) =\n+\t\t\t\t\t\tsplice(@$selected, 0, 4);\n \t\t\t}\n \t\t}\n \t}\n@@ -1104,6 +1132,108 @@ sub select_hunk_lines {\n \treturn $newhunk;\n }\n \n+sub pair_hunk_selection {\n+\tmy $line_flags = shift;\n+\tmy @ret;\n+\tmy $reverse = $patch_mode_flavour{IS_REVERSE};\n+\tfor my $sel (@_) {\n+\t\tmy $i = 0;\n+\t\tmy ($del, $add) = @$sel;\n+\t\tmy $ndel = @$del / 2;\n+\t\tmy $nadd = @$add / 2;\n+\t\t# If there the same number of insertion and deletion\n+\t\t# groups then pair up in insertions and deletions.\n+\t\tif ($nadd == $ndel) {\n+\t\t\twhile ($i < @{$del}) {\n+\t\t\t\tmy ($ra, $rb) = $reverse ? ($add, $del) :\n+\t\t\t\t\t\t\t ($del, $add);\n+\t\t\t\tmy ($starta, $enda) = @{$ra}[$i, $i + 1];\n+\t\t\t\tmy ($startb, $endb) = @{$rb}[$i, $i + 1];\n+\t\t\t\tif ($line_flags->[$enda] & LAST_ADD_DEL and\n+\t\t\t\t $line_flags->[$endb] & NO_NEWLINE) {\n+\t\t\t\t\t$endb++;\n+\t\t\t\t}\n+\t\t\t\tif ($line_flags->[$enda] & NO_NEWLINE) {\n+\t\t\t\t\t$enda++;\n+\t\t\t\t}\n+\t\t\t\tpush @ret, $reverse ?\n+\t\t\t\t\t($starta, $enda,\n+\t\t\t\t\t [ $startb..$endb ], undef) :\n+\t\t\t\t\t($starta, $enda,\n+\t\t\t\t\t undef, [ $startb..$endb ]);\n+\t\t\t\t$i += 2;\n+\t\t\t}\n+\t\t# If there are only deletions or only insertions or\n+\t\t# a) we're adding to the index and there is a single group\n+\t\t# of insertions starting at the first inserted line\n+\t\t# or\n+\t\t# b) we're resetting the index and there is a single group\n+\t\t# of deletions finishing with the last deleted line\n+\t\t# then it is okay just to have deletions followed by\n+\t\t# insertions.\n+\t\t} elsif (!$nadd or !$ndel or\n+\t\t\t (!$reverse and $ndel == 1 and\n+\t\t\t $line_flags->[$del->[1]] & LAST_ADD_DEL) or\n+\t\t\t ($reverse and $nadd == 1 and\n+\t\t\t $line_flags->[$add->[0]] & FIRST_ADD)) {\n+\t\t\twhile ($i < @$del) {\n+\t\t\t\tmy ($start, $end) = @{$del}[$i, $i + 1];\n+\t\t\t\tif ($line_flags->[$end] & NO_NEWLINE) {\n+\t\t\t\t\t$end++;\n+\t\t\t\t}\n+\t\t\t\tpush @ret, $start, $end, undef, undef;\n+\t\t\t\t$i += 2;\n+\t\t\t}\n+\t\t\t$i = 0;\n+\t\t\twhile ($i < @$add) {\n+\t\t\t\tmy ($start, $end) = @{$add}[$i, $i + 1];\n+\t\t\t\tif ($line_flags->[$end] & NO_NEWLINE) {\n+\t\t\t\t\t$end++;\n+\t\t\t\t}\n+\t\t\t\tpush @ret, $start, $end, undef, undef;\n+\t\t\t\t$i += 2;\n+\t\t\t}\n+\t\t} else {\n+\t\t\tprintf STDERR __(\"unable to pair up insertions and deletions\\n\");\n+\t\t\treturn undef;\n+\t\t}\n+\t}\n+\treturn \\@ret;\n+}\n+\n+sub process_hunk_selection {\n+\tmy ($labels, $line_flags) = @{shift()}{qw(LABELS LINE_FLAGS)};\n+\tmy ($blocks, $lines, $modes) =\n+\t\t\t\t@{$labels}{qw(BLOCKS LINES MODES)};\n+\tmy @selection = sort { $a <=> $b } @_;\n+\tunless (@selection) {\n+\t\treturn [];\n+\t}\n+\tmy $last_label = shift @selection;\n+\tmy ($last_block, $last_line, $last_mode) =\n+\t ($blocks->[$last_label], $lines->[$last_label], $modes->[$last_label]);\n+\tmy ($del, $add) = ([], []);\n+\tpush @{$last_mode eq '-' ? $del : $add}, $last_line;\n+\tmy @sel;\n+\tfor my $label (@selection) {\n+\t\tmy ($block, $line, $mode) =\n+\t\t\t($blocks->[$label], $lines->[$label], $modes->[$label]);\n+\t\tif ($block != $last_block) {\n+\t\t\tpush @{$last_mode eq '-' ? $del : $add}, $last_line;\n+\t\t\tpush @sel, [ $del, $add ];\n+\t\t\t($del, $add) = ([], []);\n+\t\t\tpush @{$mode eq '-' ? $del : $add}, $line;\n+\t\t} elsif ($line != $last_line + 1 or $mode ne $last_mode) {\n+\t\t\tpush @{$last_mode eq '-' ? $del : $add}, $last_line;\n+\t\t\tpush @{$mode eq '-' ? $del : $add}, $line;\n+\t\t}\n+\t\t($last_block, $last_line, $last_mode) = ($block, $line, $mode);\n+\t}\n+\tpush @{$last_mode eq '-' ? $del : $add}, $last_line;\n+\tpush @sel, [ $del, $add ];\n+\tpair_hunk_selection($line_flags, @sel);\n+}\n+\n sub check_hunk_label {\n \tmy ($max_label, $label) = @_;\n \tif ($label < 1 or $label > $max_label) {\n@@ -1138,14 +1268,7 @@ sub parse_hunk_selection {\n \t\t\treturn undef;\n \t\t}\n \t}\n-\t[ map {\n-\t\tmy $line = $lines->[$_];\n-\t\tif ($hunk->{LINE_FLAGS}->[$line] & NO_NEWLINE) {\n-\t\t\t($line, $line + 1);\n-\t\t} else {\n-\t\t\t($line, $line);\n-\t\t}\n-\t} sort { $a <=> $b } keys(%selected) ];\n+\treturn process_hunk_selection($hunk, keys(%selected));\n }\n \n sub display_hunk_lines {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 5b535a22d5..1d917ad018 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -489,7 +489,7 @@ test_expect_success 'setup expected diff' '\n \tEOF\n '\n \n-test_expect_failure 'can stage modified lines of patch (1)' '\n+test_expect_success 'can stage modified lines of patch (1)' '\n \tgit reset &&\n \tprintf \"%s\\n\" l \"1,3 7-9 12\" |\n \tEDITOR=: git add -p 2>error &&\n@@ -498,6 +498,112 @@ test_expect_failure 'can stage modified lines of patch (1)' '\n \tdiff_cmp expected actual\n '\n \n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..cc6163b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,7 @@\n+\t 10\n+\t-20\n+\t+twenty\n+\t+thirty\n+\t 30\n+\t 40\n+\t 50\n+\t-60\n+\t+sixty\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can stage modified lines of patch (2)' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l \"2,6,8,9,12\" |\n+\tEDITOR=: git add -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n+test_expect_success 'setup HEAD' '\n+\tgit reset &&\n+\tgit apply --cached <<-\\EOF &&\n+\tdiff --git a/test b/test\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -3,4 +3,4 @@\n+\t 30\n+\t 40\n+\t 50\n+\t-60\n+\t+60\n+\t\\ No newline at end of file\n+\tEOF\n+\tgit commit\n+'\n+\n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..cc6163b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,8 @@\n+\t-10\n+\t+ten\n+\t+twenty\n+\t+thirty\n+\t 20\n+\t-30\n+\t+fifty\n+\t 40\n+\t 50\n+\t 60\n+\t\\ No newline at end of file\n+\tEOF\n+'\n+\n+test_expect_success 'can stage modified lines of patch (3)' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l \"11,1,3 7-9\" |\n+\tEDITOR=: git add -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n+test_expect_success 'setup expected diff' '\n+\tcat >expected <<-\\EOF\n+\tdiff --git a/test b/test\n+\tindex 0889435..cc6163b 100644\n+\t--- a/test\n+\t+++ b/test\n+\t@@ -1,6 +1,8 @@\n+\t-10\n+\t+ten\n+\t+twenty\n+\t+thirty\n+\t 20\n+\t 30\n+\t 40\n+\t 50\n+\t-60\n+\t\\ No newline at end of file\n+\t+fifty\n+\tEOF\n+'\n+\n+test_expect_success 'can stage modified lines of patch (4)' '\n+\tgit reset &&\n+\tprintf \"%s\\n\" l \"9-6 11,1\" |\n+\tEDITOR=: git add -p 2>error &&\n+\ttest_must_be_empty error &&\n+\tgit diff --cached HEAD >actual &&\n+\tdiff_cmp expected actual\n+'\n+\n test_expect_success 'patch mode ignores unmerged entries' '\n \tgit reset --hard &&\n \ttest_commit conflict &&\n-- \n2.18.0\n\n"},{"id":"353653","messageId":"20180726155854.20832-4-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180726155854.20832-1-phillip.wood@talktalk.net","subject":"[PATCH v5 3/4] add -p: allow line selection to be inverted","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-07-26T15:58:53Z","receivedAt":"2018-07-26T16:06:28Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf the list of lines to be selected begins with '-' select all the\nlines except the ones listed.\n\nSigned-off-by: Phillip Wood \n---\n Documentation/git-add.txt | 3 ++-\n git-add--interactive.perl | 19 +++++++++++++++++++\n t/t3701-add-interactive.sh | 2 +-\n 3 files changed, 22 insertions(+), 2 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 965e192a09..01ff4d7d24 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -342,7 +342,8 @@ deletion labelled with a number and you will be prompted to enter which\n lines you wish to select. Individual line numbers should be separated by\n a space or comma, to specify a range of lines use a dash between\n them. If the upper bound of a range of lines is omitted it defaults to\n-the last line.\n+the last line. To invert the selection prefix it with \"-\" so \"-3-5,8\"\n+will select everything except lines 3, 4, 5 and 8.\n +\n After deciding the fate for all hunks, if there is any hunk\n that was chosen, the index is updated with the selected hunks.\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 7e4daee2fc..63541d0f90 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1247,8 +1247,21 @@ sub parse_hunk_selection {\n \tmy ($hunk, $line) = @_;\n \tmy $lines = $hunk->{LABELS}->{LINES};\n \tmy $max_label = $#{$lines};\n+\tmy $invert = undef;\n \tmy %selected;\n \tmy @fields = split(/[,\\s]+/, $line);\n+\tif (my ($rest) = ($fields[0] =~ /^-(.*)/)) {\n+\t\t$invert = 1;\n+\t\tif ($rest ne '') {\n+\t\t\t$fields[0] = $rest;\n+\t\t} else {\n+\t\t\tshift @fields;\n+\t\t\tunless (@fields) {\n+\t\t\t\terror_msg __(\"no lines to invert\\n\");\n+\t\t\t\treturn undef;\n+\t\t\t}\n+\t\t}\n+\t}\n \tfor my $f (@fields) {\n \t\tif (my ($lo, $hi) = ($f =~ /^([0-9]+)-([0-9]*)$/)) {\n \t\t\tif ($hi eq '') {\n@@ -1268,6 +1281,12 @@ sub parse_hunk_selection {\n \t\t\treturn undef;\n \t\t}\n \t}\n+\tif ($invert) {\n+\t\tmy %inverted;\n+\t\tundef @inverted{1..$max_label};\n+\t\tdelete @inverted{keys(%selected)};\n+\t\t%selected = %inverted;\n+\t}\n \treturn process_hunk_selection($hunk, keys(%selected));\n }\n \ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 1d917ad018..2fd456017f 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -457,7 +457,7 @@ test_expect_success 'setup expected diff' '\n '\n \n test_expect_success 'can reset individual lines of patch' '\n-\tprintf \"%s\\n\" l 4,1,3 |\n+\tprintf \"%s\\n\" l -6,2,5 |\n \tEDITOR=: git reset -p 2>error &&\n \ttest_must_be_empty error &&\n \tgit diff --cached HEAD >actual &&\n-- \n2.18.0\n\n"},{"id":"353654","messageId":"20180726155854.20832-5-phillip.wood@talktalk.net","threadId":"47873","inReplyTo":"20180726155854.20832-1-phillip.wood@talktalk.net","subject":"[PATCH v5 4/4] add -p: optimize line selection for short hunks","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-07-26T15:58:54Z","receivedAt":"2018-07-26T16:06:29Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"From: Phillip Wood \n\nIf there are fewer than ten changes in a hunk then make spaces\noptional when selecting individual lines. This means that for short\nhunks one can just type 1-357 to stage lines 1, 2, 3, 5 & 7.\n\nSigned-off-by: Phillip Wood \n---\n Documentation/git-add.txt | 9 +++++----\n git-add--interactive.perl | 26 ++++++++++++++++++++++++++\n t/t3701-add-interactive.sh | 2 +-\n 3 files changed, 32 insertions(+), 5 deletions(-)\n\ndiff --git a/Documentation/git-add.txt b/Documentation/git-add.txt\nindex 01ff4d7d24..f3c81dfb11 100644\n--- a/Documentation/git-add.txt\n+++ b/Documentation/git-add.txt\n@@ -340,10 +340,11 @@ patch::\n If you press \"l\" then the hunk will be reprinted with each insertion or\n deletion labelled with a number and you will be prompted to enter which\n lines you wish to select. Individual line numbers should be separated by\n-a space or comma, to specify a range of lines use a dash between\n-them. If the upper bound of a range of lines is omitted it defaults to\n-the last line. To invert the selection prefix it with \"-\" so \"-3-5,8\"\n-will select everything except lines 3, 4, 5 and 8.\n+a space or comma (these can be omitted if there are fewer than ten\n+labelled lines), to specify a range of lines use a dash between them. If\n+the upper bound of a range of lines is omitted it defaults to the last\n+line. To invert the selection prefix it with \"-\" so \"-3-5,8\" will select\n+everything except lines 3, 4, 5 and 8.\n +\n After deciding the fate for all hunks, if there is any hunk\n that was chosen, the index is updated with the selected hunks.\ndiff --git a/git-add--interactive.perl b/git-add--interactive.perl\nindex 63541d0f90..054c1168a7 100755\n--- a/git-add--interactive.perl\n+++ b/git-add--interactive.perl\n@@ -1243,6 +1243,29 @@ sub check_hunk_label {\n \treturn 1;\n }\n \n+sub split_hunk_selection {\n+\tmy @fields = @_;\n+\tmy @ret;\n+\tfor my $field (@fields) {\n+\t\twhile ($field ne '') {\n+\t\t\tif ($field =~ /^[0-9]-$/) {\n+\t\t\t\tpush @ret, $field;\n+\t\t\t\tlast;\n+\t\t\t} elsif (my ($sel, $rest) =\n+\t\t\t\t\t($field =~ /^([0-9](?:-[0-9])?)(.*)/)) {\n+\t\t\t\tpush @ret, $sel;\n+\t\t\t\t$field = $rest;\n+\t\t\t} else {\n+\t\t\t\terror_msg sprintf\n+\t\t\t\t __(\"invalid hunk line '%s'\\n\"),\n+\t\t\t\t substr($field, 0, 1);\n+\t\t\t\treturn ();\n+\t\t\t}\n+\t\t}\n+\t}\n+\treturn @ret;\n+}\n+\n sub parse_hunk_selection {\n \tmy ($hunk, $line) = @_;\n \tmy $lines = $hunk->{LABELS}->{LINES};\n@@ -1262,6 +1285,9 @@ sub parse_hunk_selection {\n \t\t\t}\n \t\t}\n \t}\n+\tif ($max_label < 10) {\n+\t\t@fields = split_hunk_selection(@fields) or return undef;\n+\t}\n \tfor my $f (@fields) {\n \t\tif (my ($lo, $hi) = ($f =~ /^([0-9]+)-([0-9]*)$/)) {\n \t\t\tif ($hi eq '') {\ndiff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh\nindex 2fd456017f..b2b808275c 100755\n--- a/t/t3701-add-interactive.sh\n+++ b/t/t3701-add-interactive.sh\n@@ -457,7 +457,7 @@ test_expect_success 'setup expected diff' '\n '\n \n test_expect_success 'can reset individual lines of patch' '\n-\tprintf \"%s\\n\" l -6,2,5 |\n+\tprintf \"%s\\n\" l -625 |\n \tEDITOR=: git reset -p 2>error &&\n \ttest_must_be_empty error &&\n \tgit diff --cached HEAD >actual &&\n-- \n2.18.0\n\n"},{"id":"353673","messageId":"xmqqmuudpzfl.fsf@gitster-ct.c.googlers.com","threadId":"47873","inReplyTo":"20180726155854.20832-3-phillip.wood@talktalk.net","subject":"Re: [RFC PATCH v5 2/4] add -p: select modified lines correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-26T19:30:38Z","receivedAt":"2018-07-26T19:30:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\nAn interesting problem you are solving ;-)\n\n> For example given the hunk\n> 1 -* a longer description of the\n> 2 - first item\n> 3 -* second\n> 4 -* third\n> 5 +* first\n> 6 + second item\n> 7 +* the third item\n>\n> If the user selects 1,2,4–5,7 then we should generate\n> \t-* a longer description of the\n> \t- first item\n> \t+* first\n> \t * second\n> \t-* third\n> \t+* the third item\n\nI understood this example as \"3 that is removal and 6 that is\naddition are excluded---we consider that these two lines (one in the\npre-image and the other in the post-image) are _matching\". As we\nare excluding a deletion, it becomes the common context line, and\nany removal or addition that appear before that must stay to happen\nbefore the common context line (i.e. removal of 1 and 2, and\naddition of 5, both precede common context line \"second\") and any\nremoval or addition that appear after that must stay after the\ncommon context (i.e. removal of \"third\" and addition of \"the third\nitem\" come after \"second\").\n\nBut then it is not clear to me what you mean by \"group\" below. What\ngroups does the above example have? Ones before the retained\n\"second\" (i.e. removal 1, 2, 4 and addition 5) form one group and\nones after it (i.e. removal 4 and addition 7) form another group?\n\n> Reported-by: Ævar Arnfjörð Bjarmason \n\nIs this fixing any bug? I usually see \"Reported-by\" only for a\nbugfix patch but this seems to be adding a new feature (and lack of\nfeature is usually not a bug).\n"},{"id":"353674","messageId":"xmqqin51pz5u.fsf@gitster-ct.c.googlers.com","threadId":"47873","inReplyTo":"20180726155854.20832-2-phillip.wood@talktalk.net","subject":"Re: [PATCH v5 1/4] add -p: select individual hunk lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-26T19:36:29Z","receivedAt":"2018-07-26T19:36:34Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> +sub label_hunk_lines {\n> +\tmy $hunk = shift;\n> +\tmy $text = $hunk->{TEXT};\n> +\tmy (@line_flags, @lines);\n> +\tmy ($block, $label, $last_mode) = (0, 0, '');\n> +\tfor my $line (1..$#{$text}) {\n\n$text is a ref to an array so @$text is the whole thing, $#{$text}\nis the index of the last item in that array, and $text->[0] is the\nfirst element of that array. This for loop runs with $line == 1\nthru $line == $#{$text}, so we are somehow excluding the very first\nelement?\n\n> +\t\t$line_flags[$line] = 0;\n> +\t\tmy $mode = substr($text->[$line], 0, 1);\n> +\t\tif ($mode eq '\\\\') {\n> +\t\t\t$line_flags[$line - 1] |= NO_NEWLINE;\n> +\t\t}\n> +\t\tif ($mode eq '-' or $mode eq '+') {\n> +\t\t\t$lines[++$label] = $line;\n> +\t\t}\n> +\t}\n> +\tif ($label > 1) {\n"},{"id":"353694","messageId":"b28e1f88-bc87-2f57-ba41-cc4a67c9a5b0@talktalk.net","threadId":"47873","inReplyTo":"xmqqin51pz5u.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH v5 1/4] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-07-27T10:05:04Z","receivedAt":"2018-07-27T10:05:38Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 26/07/18 20:36, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n>> +sub label_hunk_lines {\n>> +\tmy $hunk = shift;\n>> +\tmy $text = $hunk->{TEXT};\n>> +\tmy (@line_flags, @lines);\n>> +\tmy ($block, $label, $last_mode) = (0, 0, '');\n>> +\tfor my $line (1..$#{$text}) {\n> \n> $text is a ref to an array so @$text is the whole thing, $#{$text}\n> is the index of the last item in that array, and $text->[0] is the\n> first element of that array. This for loop runs with $line == 1\n> thru $line == $#{$text}, so we are somehow excluding the very first\n> element?\n\nYes that's right, $text->[0] contains the hunk header\n\n>> +\t\t$line_flags[$line] = 0;\n>> +\t\tmy $mode = substr($text->[$line], 0, 1);\n>> +\t\tif ($mode eq '\\\\') {\n>> +\t\t\t$line_flags[$line - 1] |= NO_NEWLINE;\n>> +\t\t}\n>> +\t\tif ($mode eq '-' or $mode eq '+') {\n>> +\t\t\t$lines[++$label] = $line;\n>> +\t\t}\n>> +\t}\n>> +\tif ($label > 1) {\n\n"},{"id":"353695","messageId":"2e78e077-1801-9c92-6b53-badf588fe262@talktalk.net","threadId":"47873","inReplyTo":"xmqqmuudpzfl.fsf@gitster-ct.c.googlers.com","subject":"Re: [RFC PATCH v5 2/4] add -p: select modified lines correctly","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-07-27T10:19:38Z","receivedAt":"2018-07-27T10:19:43Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Junio, thanks for the comments\n\nOn 26/07/18 20:30, Junio C Hamano wrote:\n> Phillip Wood writes:\n> \n> An interesting problem you are solving ;-)\n> \n>> For example given the hunk\n>> 1 -* a longer description of the\n>> 2 - first item\n>> 3 -* second\n>> 4 -* third\n>> 5 +* first\n>> 6 + second item\n>> 7 +* the third item\n>>\n>> If the user selects 1,2,4–5,7 then we should generate\n>> \t-* a longer description of the\n>> \t- first item\n>> \t+* first\n>> \t * second\n>> \t-* third\n>> \t+* the third item\n> \n> I understood this example as \"3 that is removal and 6 that is\n> addition are excluded---we consider that these two lines (one in the\n> pre-image and the other in the post-image) are _matching\" As we> are excluding a deletion, it becomes the common context line, and\n> any removal or addition that appear before that must stay to happen\n> before the common context line (i.e. removal of 1 and 2, and\n> addition of 5, both precede common context line \"second\") and any\n> removal or addition that appear after that must stay after the\n> common context (i.e. removal of \"third\" and addition of \"the third\n> item\" come after \"second\").\n> \n> But then it is not clear to me what you mean by \"group\" below. What\n> groups does the above example have? Ones before the retained\n> \"second\" (i.e. removal 1, 2, 4 and addition 5) form one group and\n> ones after it (i.e. removal 4 and addition 7) form another group?\n\nThe code actually looks at the lines that are selected rather than\nomitted. So in the example above it groups them as [1,2] (because they\nare contiguous), [4],[5] (these are split because one is an insertion\nand one a deletion) and [7]. It then sees that there are two groups of\ndeletions ([1,2],[4]) and two groups of insertions ([5],[7]) and so\npairs up the deletions in [12] with the insertion in [5] and likewise\nwith [4] and [7]. Lines 3 and 6 are never explicitly paired, although\nthey basically behave as if they were. One the insertions are all paired\nup it walks over the list and creates a new hunk where the paired\ninsertions come immediately after their corresponding deletions,\nunselected deletions are converted to context lines and unselected\nadditions are dropped.\n\n> \n>> Reported-by: Ævar Arnfjörð Bjarmason \n> \n> Is this fixing any bug? I usually see \"Reported-by\" only for a\n> bugfix patch but this seems to be adding a new feature (and lack of\n> feature is usually not a bug).\n\nI guess I meant that the previous series was effectively buggy as it\nwould give the wrong result for modified lines. I wanted to acknowledge\nthat Ævar spent some time testing it and pointed that out.\n\nBest Wishes\n\nPhillip\n\n"},{"id":"353726","messageId":"xmqq4lgkpsnx.fsf@gitster-ct.c.googlers.com","threadId":"47873","inReplyTo":"b28e1f88-bc87-2f57-ba41-cc4a67c9a5b0@talktalk.net","subject":"Re: [PATCH v5 1/4] add -p: select individual hunk lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-27T16:09:06Z","receivedAt":"2018-07-27T16:09:12Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> On 26/07/18 20:36, Junio C Hamano wrote:\n>> Phillip Wood writes:\n>> \n>>> +sub label_hunk_lines {\n>>> +\tmy $hunk = shift;\n>>> +\tmy $text = $hunk->{TEXT};\n>>> +\tmy (@line_flags, @lines);\n>>> +\tmy ($block, $label, $last_mode) = (0, 0, '');\n>>> +\tfor my $line (1..$#{$text}) {\n>> \n>> $text is a ref to an array so @$text is the whole thing, $#{$text}\n>> is the index of the last item in that array, and $text->[0] is the\n>> first element of that array. This for loop runs with $line == 1\n>> thru $line == $#{$text}, so we are somehow excluding the very first\n>> element?\n>\n> Yes that's right, $text->[0] contains the hunk header\n\nOK, thanks.\n\n>\n>>> +\t\t$line_flags[$line] = 0;\n>>> +\t\tmy $mode = substr($text->[$line], 0, 1);\n>>> +\t\tif ($mode eq '\\\\') {\n>>> +\t\t\t$line_flags[$line - 1] |= NO_NEWLINE;\n>>> +\t\t}\n>>> +\t\tif ($mode eq '-' or $mode eq '+') {\n>>> +\t\t\t$lines[++$label] = $line;\n>>> +\t\t}\n>>> +\t}\n>>> +\tif ($label > 1) {\n"},{"id":"353727","messageId":"xmqqzhycodva.fsf@gitster-ct.c.googlers.com","threadId":"47873","inReplyTo":"2e78e077-1801-9c92-6b53-badf588fe262@talktalk.net","subject":"Re: [RFC PATCH v5 2/4] add -p: select modified lines correctly","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-07-27T16:14:01Z","receivedAt":"2018-07-27T16:14:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> The code actually looks at the lines that are selected rather than\n> omitted. So in the example above it groups them as [1,2] (because they\n> are contiguous), [4],[5] (these are split because one is an insertion\n> and one a deletion) and [7]. It then sees that there are two groups of\n> deletions ([1,2],[4]) and two groups of insertions ([5],[7]) and so\n> pairs up the deletions in [12] with the insertion in [5] and likewise\n> with [4] and [7]. Lines 3 and 6 are never explicitly paired, although\n> they basically behave as if they were. One the insertions are all paired\n> up it walks over the list and creates a new hunk where the paired\n> insertions come immediately after their corresponding deletions,\n> unselected deletions are converted to context lines and unselected\n> additions are dropped.\n\nNow, without that much explanation in help text, can an average end\nuser use the feature, specifically, understand the reason why the\ntool says it cannot handle a particular set of selected lines, and\nfollow the workaround suggested by the tool to do it in two (or\nmore) batches? That was the real question I was getting at. I\nhaven't played with the feature long enough to answer that question.\n\n>>> Reported-by: Ævar Arnfjörð Bjarmason \n>> \n>> Is this fixing any bug? I usually see \"Reported-by\" only for a\n>> bugfix patch but this seems to be adding a new feature (and lack of\n>> feature is usually not a bug).\n>\n> I guess I meant that the previous series was effectively buggy as it\n> would give the wrong result for modified lines. I wanted to acknowledge\n> that Ævar spent some time testing it and pointed that out.\n\nAh, I see. We generally throw these into \"Helped-by\", I'd think.\n\nThanks.\n\n"},{"id":"353742","messageId":"87effo5ybv.fsf@evledraar.gmail.com","threadId":"47873","inReplyTo":"20180726155854.20832-1-phillip.wood@talktalk.net","subject":"Re: [RFC PATCH v5 0/4] add -p: select individual hunk lines","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-07-27T18:27:00Z","receivedAt":"2018-07-27T18:27:06Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jul 26 2018, Phillip Wood wrote:\n\n> From: Phillip Wood \n>\n> Unfortuantely v4 had test failures due to a suprious brace from a last\n> minute edit to a comment that I forgot to test. This version fixes\n> that, my applogies for the patch churn.\n>\n> I've updated this series based on Ævar's feedback on v3 (to paraphrase\n> stop using '$_' so much and fix staging modified lines.). The first\n> patch is functionally equivalent to the previous version but with a\n> reworked implementation. Patch 2 is new, it implements correctly\n> staging modified lines with a couple of limitations - see the commit\n> message for more details, I'm keen to get some feedback on it. Patches\n> 3 and 4 are essentially rebased and tweaked versions of patches 2 and\n> 3 from the previous version.\n\nI was going to review this, but can't find what it's based on, I can't\napply 1/4 to master, next or pu. It seems to be based on some older\nversion of master, e.g. 1/4 has this hunk:\n\n +\t\t\telsif ($line =~ /^l/) {\n +\t\t\t\tunless ($other =~ /l/) {\n +\t\t\t\t\terror_msg __(\"Cannot select line by line\\n\");\n +\t\t\t\t\tnext;\n +\t\t\t\t}\n +\t\t\t\tmy $newhunk = select_lines_loop($hunk[$ix]);\n +\t\t\t\tif ($newhunk) {\n +\t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n +\t\t\t\t} else {\n +\t\t\t\t\tnext;\n +\t\t\t\t}\n +\t\t\t}\n \t\t\telsif ($other =~ /s/ && $line =~ /^s/) {\n\nWhich seems to conflict with your 4bdd6e7ce3 (\"add -p: improve error\nmessages\", 2018-02-13). I could have tried to manually apply this, but\nfigured I'd bounce this back to you...\n\nHaving just skimmed through the patches themselves I agree with this\napproach of handling the simple case (as discussed before) and leaving\nthe rest for some future change, but let's see about the details once I\nhave this running.\n"},{"id":"353795","messageId":"e3d5f5a7-b173-5fa3-d007-f07013d554c8@talktalk.net","threadId":"47873","inReplyTo":"87effo5ybv.fsf@evledraar.gmail.com","subject":"Re: [RFC PATCH v5 0/4] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-07-28T10:08:14Z","receivedAt":"2018-07-28T10:08:19Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 27/07/18 19:27, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Thu, Jul 26 2018, Phillip Wood wrote:\n> \n>> From: Phillip Wood \n>>\n>> Unfortuantely v4 had test failures due to a suprious brace from a last\n>> minute edit to a comment that I forgot to test. This version fixes\n>> that, my applogies for the patch churn.\n>>\n>> I've updated this series based on Ævar's feedback on v3 (to paraphrase\n>> stop using '$_' so much and fix staging modified lines.). The first\n>> patch is functionally equivalent to the previous version but with a\n>> reworked implementation. Patch 2 is new, it implements correctly\n>> staging modified lines with a couple of limitations - see the commit\n>> message for more details, I'm keen to get some feedback on it. Patches\n>> 3 and 4 are essentially rebased and tweaked versions of patches 2 and\n>> 3 from the previous version.\n> \n> I was going to review this, but can't find what it's based on, I can't\n> apply 1/4 to master, next or pu. It seems to be based on some older\n> version of master, e.g. 1/4 has this hunk:\n> \n> +\t\t\telsif ($line =~ /^l/) {\n> +\t\t\t\tunless ($other =~ /l/) {\n> +\t\t\t\t\terror_msg __(\"Cannot select line by line\\n\");\n> +\t\t\t\t\tnext;\n> +\t\t\t\t}\n> +\t\t\t\tmy $newhunk = select_lines_loop($hunk[$ix]);\n> +\t\t\t\tif ($newhunk) {\n> +\t\t\t\t\tsplice @hunk, $ix, 1, $newhunk;\n> +\t\t\t\t} else {\n> +\t\t\t\t\tnext;\n> +\t\t\t\t}\n> +\t\t\t}\n> \t\t\telsif ($other =~ /s/ && $line =~ /^s/) {\n> \n> Which seems to conflict with your 4bdd6e7ce3 (\"add -p: improve error\n> messages\", 2018-02-13). I could have tried to manually apply this, but\n> figured I'd bounce this back to you...\n\nYes, I wasn't sure whether to rebase or not and in the end I didn't. The\nline below where you cut the cover letter message says it is based on\nf4d35a6b49 \"add -p: fix counting empty context lines in edited patches\".\nSo you could just apply it there and test it. You can fetch it with\n\ngit fetch https://github.com/phillipwood/git add-i-select-lines-v5\n\n> Having just skimmed through the patches themselves I agree with this\n> approach of handling the simple case (as discussed before) and leaving\n> the rest for some future change, but let's see about the details once I\n> have this running.\n\nThanks\n\nPhillip\n\n\n"},{"id":"353798","messageId":"874lgjv8h0.fsf@evledraar.gmail.com","threadId":"47873","inReplyTo":"20180726155854.20832-1-phillip.wood@talktalk.net","subject":"Re: [RFC PATCH v5 0/4] add -p: select individual hunk lines","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-07-28T12:40:59Z","receivedAt":"2018-07-28T12:41:58Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Thu, Jul 26 2018, Phillip Wood wrote:\n\n> Unfortuantely v4 had test failures due to a suprious brace from a last\n> minute edit to a comment that I forgot to test. This version fixes\n> that, my applogies for the patch churn.\n>\n> I've updated this series based on Ævar's feedback on v3 (to paraphrase\n> stop using '$_' so much and fix staging modified lines.). The first\n> patch is functionally equivalent to the previous version but with a\n> reworked implementation. Patch 2 is new, it implements correctly\n> staging modified lines with a couple of limitations - see the commit\n> message for more details, I'm keen to get some feedback on it. Patches\n> 3 and 4 are essentially rebased and tweaked versions of patches 2 and\n> 3 from the previous version.\n>\n> This series is based on pw/add-p-recount (f4d35a6b49 \"add -p: fix\n> counting empty context lines in edited patches\")\n>\n> The motivation for this series is summed up in the first commit\n> message:\n>\n> \"When I end up editing hunks it is almost always because I want to\n> stage a subset of the lines in the hunk. Doing this by editing the\n> hunk is inconvenient and error prone (especially so if the patch is\n> going to be reversed before being applied). Instead offer an option\n> for add -p to stage individual lines. When the user presses 'l' the\n> hunk is redrawn with labels by the insertions and deletions and they\n> are prompted to enter a list of the lines they wish to stage. Ranges\n> of lines may be specified using 'a-b' where either 'a' or 'b' may be\n> omitted to mean all lines from 'a' to the end of the hunk or all lines\n> from 1 upto and including 'b'.\"\n\nI tested this with an eye towards what I pointed out in\nhttps://public-inbox.org/git/878ta8vyqe.fsf@evledraar.gmail.com/\n\nUsing the same workflow (search for \"So what I was expecting\" in that\nE-Mail) this now does the right thing in that example:\n\n select lines? 4,10\n [...]\n $ git diff --staged -U1\n diff --git a/README.md b/README.md\n index ff990622a3..6d16f7e52b 100644\n --- a/README.md\n +++ b/README.md\n @@ -20,3 +20,3 @@ See [Documentation/gittutorial.txt][] to get started, then see\n Documentation/git-.txt for documentation of each command.\n -If git has been correctly installed, then the tutorial can also be\n +If Git has been correctly installed, then the tutorial can also be\n read with `man gittutorial` or `git help tutorial`, and the\n u git ((49703a4754...) $) $\n\nSome other comments on this:\n\n1) It needs to be more obvious how to exit this sub-mode, i.e. consider\nthis confused user:\n\n Stage this hunk [y,n,q,a,d,/,j,J,g,s,e,l,?]? l\n select lines? ?\n invalid hunk line '?'\n select lines? q\n invalid hunk line 'q'\n select lines? exit\n invalid hunk line 'exit'\n select lines? quit\n invalid hunk line 'quit'\n select lines? :wq\n invalid hunk line ':wq'\n select lines? help\n invalid hunk line 'help'\n\nJust doing Ctrl+D or RET exits it. Instead \"?\" should print some help\nrelated to this sub-mode showing what the syntax is, and how to exit the\nsub-mode. I think it would make sense for \"q\" to by synonymous with\n\"RET\", i.e. you'd need \"qq\" to fully exit, but I don't know...\n\n2) I think it's confusing UI that selecting some of the lines won't\nre-present the hunk to you again in line mode, but I see this is\nconsistent with how e.g. \"e\" works, it won't re-present the hunk to you\nif there's still something to do, you need to exit and run \"git add -p\"\nagain.\n\nI think it makes sense to change that and you'd either \"e\" or \"l\" and\nthen \"n\" to proceed, or continue, but that's per-se unrelated to this\nfeature. Just something I ran into...\n\n3) I don't see any way around this, but we need to carefully explain\nthat selecting a list of things in one session is *not* the same thing\nas selecting them incrementally in multiple sessions. I.e. consider this\ndiff:\n\n @@ -1,3 +1,3 @@\n -a\n -b\n -c\n +1\n +2\n +3\n\nIf I select 1,4 I get, as expected:\n\n @@ -1,3 +1,3 @@\n -a\n +1\n b\n c\n\nAnd then in the next session:\n\n @@ -1,3 +1,3 @@\n 1\n 1 -b\n 2 -c\n 3 +2\n 4 +3\n select lines? 1,3\n\nYields, as expected:\n\n @@ -1,3 +1,3 @@\n -a\n -b\n +1\n +2\n c\n\nBut this is not the same as redoing the whole thing as:\n\n select lines? 1,4\n select lines? 1\n select lines? 3\n\nWhich instead yields:\n\n @@ -1,3 +1,3 @@\n -a\n -b\n +1\n c\n +3\n\nNow, rummaging through my wetware and that E-Mail from back in March I\ndon't see how it could work differently, and you *also* want to be able\nto select one line at a time like that.\n\nJust something that's not very intuative / hard to explain, and maybe\nthere should be a different syntax (e.g. 1:4) for this \"swap 1 for 4\"\noperation, as opposed to selecting lines 1 and 4 as they appear in the\ndiff.\n\n4) With that abc 123 diff noted above, why am I in two sessions allowed\nto do:\n\n @@ -1,3 +1,3 @@\n 1 -a\n 2 -b\n 3 -c\n 4 +1\n 5 +2\n 6 +3\n select lines? 1,4\n select lines? 1,4\n\nTo end up with this staged:\n\n @@ -1,3 +1,3 @@\n -a\n -b\n +1\n +3\n c\n\nBut not allowed to do the same thing in one operation via:\n\n @@ -1,3 +1,3 @@\n 1 -a\n 2 -b\n 3 -c\n 4 +1\n 5 +2\n 6 +3\n select lines? 1,4,2,6\n unable to pair up insertions and deletions\n\nBut I am allowed to do e.g.:\n\n select lines? 1,4,2,5\n\nTo end up with:\n\n @@ -1,3 +1,3 @@\n -a\n -b\n +1\n +2\n c\n\nI can do this in two steps.\n"},{"id":"354368","messageId":"d622a95b-7302-43d4-4ec9-b2cf3388c653@talktalk.net","threadId":"47873","inReplyTo":"874lgjv8h0.fsf@evledraar.gmail.com","subject":"Re: [RFC PATCH v5 0/4] add -p: select individual hunk lines","fromName":"Phillip Wood","fromEmail":"phillip.wood@talktalk.net","sentAt":"2018-08-03T10:01:38Z","receivedAt":"2018-08-03T10:01:42Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ævar\n\nThanks for looking at this.\n\nOn 28/07/18 13:40, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Thu, Jul 26 2018, Phillip Wood wrote:\n> \n>> Unfortuantely v4 had test failures due to a suprious brace from a last\n>> minute edit to a comment that I forgot to test. This version fixes\n>> that, my applogies for the patch churn.\n>>\n>> I've updated this series based on Ævar's feedback on v3 (to paraphrase\n>> stop using '$_' so much and fix staging modified lines.). The first\n>> patch is functionally equivalent to the previous version but with a\n>> reworked implementation. Patch 2 is new, it implements correctly\n>> staging modified lines with a couple of limitations - see the commit\n>> message for more details, I'm keen to get some feedback on it. Patches\n>> 3 and 4 are essentially rebased and tweaked versions of patches 2 and\n>> 3 from the previous version.\n>>\n>> This series is based on pw/add-p-recount (f4d35a6b49 \"add -p: fix\n>> counting empty context lines in edited patches\")\n>>\n>> The motivation for this series is summed up in the first commit\n>> message:\n>>\n>> \"When I end up editing hunks it is almost always because I want to\n>> stage a subset of the lines in the hunk. Doing this by editing the\n>> hunk is inconvenient and error prone (especially so if the patch is\n>> going to be reversed before being applied). Instead offer an option\n>> for add -p to stage individual lines. When the user presses 'l' the\n>> hunk is redrawn with labels by the insertions and deletions and they\n>> are prompted to enter a list of the lines they wish to stage. Ranges\n>> of lines may be specified using 'a-b' where either 'a' or 'b' may be\n>> omitted to mean all lines from 'a' to the end of the hunk or all lines\n>> from 1 upto and including 'b'.\"\n> \n> I tested this with an eye towards what I pointed out in\n> https://public-inbox.org/git/878ta8vyqe.fsf@evledraar.gmail.com/\n> \n> Using the same workflow (search for \"So what I was expecting\" in that\n> E-Mail) this now does the right thing in that example:\n> \n> select lines? 4,10\n> [...]\n> $ git diff --staged -U1\n> diff --git a/README.md b/README.md\n> index ff990622a3..6d16f7e52b 100644\n> --- a/README.md\n> +++ b/README.md\n> @@ -20,3 +20,3 @@ See [Documentation/gittutorial.txt][] to get started, then see\n> Documentation/git-.txt for documentation of each command.\n> -If git has been correctly installed, then the tutorial can also be\n> +If Git has been correctly installed, then the tutorial can also be\n> read with `man gittutorial` or `git help tutorial`, and the\n> u git ((49703a4754...) $) $\n> \n> Some other comments on this:\n> \n> 1) It needs to be more obvious how to exit this sub-mode, i.e. consider\n> this confused user:\n> \n> Stage this hunk [y,n,q,a,d,/,j,J,g,s,e,l,?]? l\n> select lines? ?\n> invalid hunk line '?'\n> select lines? q\n> invalid hunk line 'q'\n> select lines? exit\n> invalid hunk line 'exit'\n> select lines? quit\n> invalid hunk line 'quit'\n> select lines? :wq\n> invalid hunk line ':wq'\n> select lines? help\n> invalid hunk line 'help'\n> \n> Just doing Ctrl+D or RET exits it. Instead \"?\" should print some help\n> related to this sub-mode showing what the syntax is, and how to exit the\n> sub-mode. I think it would make sense for \"q\" to by synonymous with\n> \"RET\", i.e. you'd need \"qq\" to fully exit, but I don't know...\n\nThanks for point this out I meant to add some help and then forgot. As\nfor exiting adding \"q\" could be a good idea, at the moment it follows\nthe behavior of the other sub-prompts of 'add -i' which isn't that user\nfriendly.\n\n> \n> 2) I think it's confusing UI that selecting some of the lines won't\n> re-present the hunk to you again in line mode, but I see this is\n> consistent with how e.g. \"e\" works, it won't re-present the hunk to you\n> if there's still something to do, you need to exit and run \"git add -p\"\n> again.\n> \n> I think it makes sense to change that and you'd either \"e\" or \"l\" and\n> then \"n\" to proceed, or continue, but that's per-se unrelated to this\n> feature. Just something I ran into...\n\nThat's something I thought about but in the end I didn't do it, maybe I\nshould look at it again as it makes it easier to get around the\nlimitations of the selection grouping. On the other hand if I've just\nselected the lines I want it is convenient to move directly to the next\nhunk without having to somehow quit the current one.\n\nThere are some questions about how the remaining lines of the hunk\nshould be presented - should they be broken up if you end up with a big\nblock of context in the middle after the first edit for instance.\nThere's also a practical problem of coloring the hunk if the user has\nset interactive.diffFilter as it appears reading and writing to a\nprocess is tricky on Windows. Part of the commit message of 01143847db\n(\"add--interactive: allow custom diff highlighting programs\", 2016-2-27)\nreads\n\n 2. add--interactive will re-colorize a diff which has been\n hand-edited, but it won't have run through the filter.\n Fixing this is conceptually easy (just pipe the diff\n through the filter), but practically hard to do without\n using tempfiles (it would need to feed data to and read\n the result from the filter without deadlocking; this\n raises portability questions with respect to Windows).\n\n> \n> 3) I don't see any way around this, but we need to carefully explain\n> that selecting a list of things in one session is *not* the same thing\n> as selecting them incrementally in multiple sessions. I.e. consider this\n> diff:\n> \n> @@ -1,3 +1,3 @@\n> -a\n> -b\n> -c\n> +1\n> +2\n> +3\n> \n> If I select 1,4 I get, as expected:\n> \n> @@ -1,3 +1,3 @@\n> -a\n> +1\n> b\n> c\n> \n> And then in the next session:\n> \n> @@ -1,3 +1,3 @@\n> 1\n> 1 -b\n> 2 -c\n> 3 +2\n> 4 +3\n> select lines? 1,3\n> \n> Yields, as expected:\n> \n> @@ -1,3 +1,3 @@\n> -a\n> -b\n> +1\n> +2\n> c\n> \n> But this is not the same as redoing the whole thing as:\n> \n> select lines? 1,4\n> select lines? 1\n> select lines? 3\n> \n> Which instead yields:\n> \n> @@ -1,3 +1,3 @@\n> -a\n> -b\n> +1\n> c\n> +3\n> \n> Now, rummaging through my wetware and that E-Mail from back in March I\n> don't see how it could work differently, and you *also* want to be able\n> to select one line at a time like that.\n> \n> Just something that's not very intuative / hard to explain, and maybe\n> there should be a different syntax (e.g. 1:4) for this \"swap 1 for 4\"\n> operation, as opposed to selecting lines 1 and 4 as they appear in the\n> diff.\n\nI did wonder about forcing the user to explicitly pair lines but shied\naway from it as I assumed that is what would be wanted most of the time\nand it is awkward for the user to have to pair them up and it involved\nmore error checking on the input to make sure they don't say 1:4 2:3.\n\n> \n> 4) With that abc 123 diff noted above, why am I in two sessions allowed\n> to do:\n> \n> @@ -1,3 +1,3 @@\n> 1 -a\n> 2 -b\n> 3 -c\n> 4 +1\n> 5 +2\n> 6 +3\n> select lines? 1,4\n> select lines? 1,4\n> \n> To end up with this staged:\n> \n> @@ -1,3 +1,3 @@\n> -a\n> -b\n> +1\n> +3\n> c\n> \n> But not allowed to do the same thing in one operation via:\n> \n> @@ -1,3 +1,3 @@\n> 1 -a\n> 2 -b\n> 3 -c\n> 4 +1\n> 5 +2\n> 6 +3\n> select lines? 1,4,2,6\n> unable to pair up insertions and deletions\n> \n> But I am allowed to do e.g.:\n> \n> select lines? 1,4,2,5\n> \n> To end up with:\n> \n> @@ -1,3 +1,3 @@\n> -a\n> -b\n> +1\n> +2\n> c\n> \n> I can do this in two steps.\n\nYes that needs explaining. To implement staging modified lines the code\nneeds to pair up each deleted line with its replacement. It does this by\ngrouping consecutive selected lines together, so it has a list of groups\nof deleted lines and another list of inserted lines, it then pairs the\ndeletions and insertions by their index in the list. So in the last\nexample lines 1 and 2 are grouped together so there is a single group of\ndeletions and two groups of insertions. There is no way for it to tell\nif lines 1 and 2 should be replaced by line 4 and line 5 should inserted\nafter 'c' or if 1 and 2 should be replaced by 4 and 5 with 'c' coming after.\n\nIt seems clear for your comment and Junio's that I need to improve the\ndocumentation, I'm not sure if that will be enough though or do we need\nto change the behavior? [I'm beginning to see why all the other programs\nI tried while writing this (tig, gitg, gitk and mercurial's version of\nadd -i) don't make any attempt to stage modified lines correctly, though\nI think git should have some way of doing it.]\n\nBest Wishes\n\nPhillip\n"},{"id":"354409","messageId":"xmqqy3dnxujl.fsf@gitster-ct.c.googlers.com","threadId":"47873","inReplyTo":"d622a95b-7302-43d4-4ec9-b2cf3388c653@talktalk.net","subject":"Re: [RFC PATCH v5 0/4] add -p: select individual hunk lines","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2018-08-03T16:51:42Z","receivedAt":"2018-08-03T16:51:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood writes:\n\n> ... [I'm beginning to see why all the other programs I tried while\n> writing this (tig, gitg, gitk and mercurial's version of add -i)\n> don't make any attempt to stage modified lines correctly, though I\n> think git should have some way of doing it.]\n\nYes, this is a kind of feature that one can propose and implement\nsomething that works well for some of the limited cases one uses,\nbut not in other cases, and it becomes very hard to explain how to\nwork around the implementation limitation---that is why I stopped\nat \"split this hunk?\" and did not go beyond it when I designed the\noriginal \"incremental add\" feature.\n\nI think the real reason why it is hard is that there is no good\ndefinition of \"modified\" in \"stage modified lines\", and worse, there\nis no good way to mechanically figure it out, because a patch only\ngives you \"these were deleted\" and \"these are added\", without giving\nyou \"this line in the deleted block corresponds to these two lines\nin the added block\" (i.e. \"this original one line was modified into\nthis thing in the result\").\n"},{"id":"354420","messageId":"874lgbtjok.fsf@evledraar.gmail.com","threadId":"47873","inReplyTo":"d622a95b-7302-43d4-4ec9-b2cf3388c653@talktalk.net","subject":"Re: [RFC PATCH v5 0/4] add -p: select individual hunk lines","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2018-08-03T17:59:55Z","receivedAt":"2018-08-03T18:00:00Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Aug 03 2018, Phillip Wood wrote:\n\n[snip (all made sense)]\n\n> It seems clear for your comment and Junio's that I need to improve the\n> documentation, I'm not sure if that will be enough though or do we need\n> to change the behavior? [I'm beginning to see why all the other programs\n> I tried while writing this (tig, gitg, gitk and mercurial's version of\n> add -i) don't make any attempt to stage modified lines correctly, though\n> I think git should have some way of doing it.]\n\nI think this is ready to move forward conceptually, i.e. in these two\nreview rounds I've been focusing on the UI this exposes, and whether\nthis is even a viable thing to have conceptually.\n\nI'm convinced that it is, even though there's osme confusing edge cases\n(as noted in my mail) I don't see how those could be entire avoided, a\nfeature like this will inherently have such edge cases.\n\nI've avoided going deep into how the actual code itself works / is\nimplemented in these review rounds, figuring that given the topic it was\nbetter to move past RFC for that type of review once we had docs.\n\nThanks for working on this.\n"}]}