On 06/03/2018 11:17, Phillip Wood wrote:
> From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
>
> When I end up editing hunks it is almost always because I want to
> stage a subset of the lines in the hunk. Doing this by editing the
> hunk is inconvenient and error prone (especially so if the patch is
> going to be reversed before being applied). Instead offer an option
> for add -p to stage individual lines. When the user presses 'l' the
> hunk is redrawn with labels by the insertions and deletions and they
> are prompted to enter a list of the lines they wish to stage. Ranges
> of lines may be specified using 'a-b' where either 'a' or 'b' may be
> omitted to mean all lines from 'a' to the end of the hunk or all lines
> from 1 upto and including 'b'.
>
> Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
> ---
> Documentation/git-add.txt | 7 +++
> git-add--interactive.perl | 143 +++++++++++++++++++++++++++++++++++++++++++++
> t/t3701-add-interactive.sh | 65 +++++++++++++++++++++
> 3 files changed, 215 insertions(+)
>
> diff --git a/Documentation/git-add.txt b/Documentation/git-add.txt
> index d50fa339dc..ad33fda9a2 100644
> --- a/Documentation/git-add.txt
> +++ b/Documentation/git-add.txt
> @@ -332,10 +332,17 @@ patch::
> J - leave this hunk undecided, see next hunk
> k - leave this hunk undecided, see previous undecided hunk
> K - leave this hunk undecided, see previous hunk
> + l - select hunk lines to use
Might be more surrounding context aligned to say "stage hunk lines"
here (phrase "stage", instead of "select to use").
> s - split the current hunk into smaller hunks
> e - manually edit the current hunk
> ? - print help
> +
> +If you press "l" then the hunk will be reprinted with each insertion
> +or deletion labelled with a number and you will be prompted to enter
> +which lines you wish to select. Individual line numbers should be
Likewise, s/you wish to select/you wish to stage/.
> +separated by a space or comma, to specify a range of lines use a dash
> +between them.
> ++
> After deciding the fate for all hunks, if there is any hunk
> that was chosen, the index is updated with the selected hunks.
> +
> diff --git a/git-add--interactive.perl b/git-add--interactive.perl
> index f83e7450ad..a273b41e95 100755
> --- a/git-add--interactive.perl
> +++ b/git-add--interactive.perl
> @@ -1013,6 +1013,133 @@ sub color_diff {
> } @_;
> }
>
> +sub label_hunk_lines {
> + local $_;
> + my $hunk = shift;
> + my $i = 0;
> + my $labels = [ map { /^[-+]/ ? ++$i : 0 } @{$hunk->{TEXT}} ];
> + if ($i > 1) {
> + @{$hunk}{qw(LABELS MAX_LABEL)} = ($labels, $i);
> + return 1;
> + }
> + return 0;
> +}
> +
> +sub select_hunk_lines {
This is just something I`ve spotted, but I have no actual idea if
renaming this to "stage_hunk_lines" might be better, too, or not
(depending on the surrounding code context), so please take this with
a big grain of salt.
> + my ($hunk, $selected) = @_;
> + my ($text, $labels) = @{$hunk}{qw(TEXT LABELS)};
> + my ($i, $o_cnt, $n_cnt) = (0, 0, 0);
> + my ($push_eol, @newtext);
> + # Lines with this mode will become context lines if they are
> + # not selected
> + my $context_mode = $patch_mode_flavour{IS_REVERSE} ? '+' : '-';
> + for $i (1..$#{$text}) {
> + my $mode = substr($text->[$i], 0, 1);
> + if ($mode eq '\\') {
> + push @newtext, $text->[$i] if ($push_eol);
> + undef $push_eol;
> + } elsif ($labels->[$i] and $selected->[$labels->[$i]]) {
> + push @newtext, $text->[$i];
> + if ($mode eq '+') {
> + $n_cnt++;
> + } else {
> + $o_cnt++;
> + }
> + $push_eol = 1;
> + } elsif ($mode eq ' ' or $mode eq $context_mode) {
> + push @newtext, ' ' . substr($text->[$i], 1);
> + $o_cnt++; $n_cnt++;
> + $push_eol = 1;
> + } else {
> + undef $push_eol;
> + }
> + }
> + my ($o_ofs, $orig_o_cnt, $n_ofs, $orig_n_cnt) =
> + parse_hunk_header($text->[0]);
> + unshift @newtext, format_hunk_header($o_ofs, $o_cnt, $n_ofs, $n_cnt);
> + my $newhunk = {
> + TEXT => \@newtext,
> + DISPLAY => [ color_diff(@newtext) ],
> + OFS_DELTA => $orig_o_cnt - $orig_n_cnt - $o_cnt + $n_cnt,
> + TYPE => $hunk->{TYPE},
> + USE => 1,
> + };
> + # If this hunk has previously been edited add the offset delta
> + # of the old hunk to get the real delta from the original
> + # hunk.
> + if ($hunk->{OFS_DELTA}) {
> + $newhunk->{OFS_DELTA} += $hunk->{OFS_DELTA};
> + }
> + return $newhunk;
> +}
> +
> +sub check_hunk_label {
> + my ($max_label, $label) = ($_[0]->{MAX_LABEL}, $_[1]);
> + if ($label < 1 or $label > $max_label) {
> + error_msg sprintf(__("invalid hunk line '%d'\n"), $label);
> + return 0;
> + }
> + return 1;
> +}
> +
> +sub parse_hunk_selection {
> + local $_;
> + my ($hunk, $line) = @_;
> + my $max_label = $hunk->{MAX_LABEL};
> + my @selected = (0) x ($max_label + 1);
> + my @fields = split(/[,\s]+/, $line);
> + for (@fields) {
> + if (/^([0-9]*)-([0-9]*)$/) {
> + if ($1 eq '' and $2 eq '') {
> + error_msg __("range '-' missing upper or lower bound\n");
> + return undef;
> + }
> + my $lo = $1 eq '' ? 1 : $1;
> + my $hi = $2 eq '' ? $max_label : $2;
> + check_hunk_label($hunk, $lo) or return undef;
> + check_hunk_label($hunk, $hi) or return undef;
> + if ($hi < $lo) {
> + ($lo, $hi) = ($hi, $lo);
> + }
> + @selected[$lo..$hi] = (1) x (1 + $hi - $lo);
> + } elsif (/^([0-9]+)$/) {
> + check_hunk_label($hunk, $1) or return undef;
> + $selected[$1] = 1;
> + } else {
> + error_msg sprintf(__("invalid hunk line '%s'\n"), $_);
> + return undef;
> + }
> + }
> + return \@selected;
> +}
> +
> +sub display_hunk_lines {
> + my ($display, $labels, $max_label) =
> + @{$_[0]}{qw(DISPLAY LABELS MAX_LABEL)};
> + my $width = int(log($max_label) / log(10)) + 1;
> + my $padding = ' ' x ($width + 1);
> + for my $i (0..$#{$display}) {
> + if ($labels->[$i]) {
> + printf '%*d %s', $width, $labels->[$i], $display->[$i];
> + } else {
> + print $padding . $display->[$i];
> + }
> + }
> +}
> +
> +sub select_lines_loop {
> + my $hunk = shift;
> + display_hunk_lines($hunk);
> + my $selection = undef;
> + until (defined $selection) {
> + print colored $prompt_color, __("select lines? ");
> + my $text = <STDIN>;
> + defined $text and $text =~ /\S/ or return undef;
> + $selection = parse_hunk_selection($hunk, $text);
> + }
> + return select_hunk_lines($hunk, $selection);
> +}
> +
> my %edit_hunk_manually_modes = (
> stage => N__(
> "If the patch applies cleanly, the edited hunk will immediately be
> @@ -1255,6 +1382,7 @@ j - leave this hunk undecided, see next undecided hunk
> J - leave this hunk undecided, see next hunk
> k - leave this hunk undecided, see previous undecided hunk
> K - leave this hunk undecided, see previous hunk
> +l - select hunk lines to use
s/select hunk lines to use/stage hunk lines/
> s - split the current hunk into smaller hunks
> e - manually edit the current hunk
> ? - print help
> @@ -1471,6 +1599,9 @@ sub patch_update_file {
> if ($hunk[$ix]{TYPE} eq 'hunk') {
> $other .= ',e';
> }
> + if (label_hunk_lines($hunk[$ix])) {
> + $other .= ',l';
> + }
> for (@{$hunk[$ix]{DISPLAY}}) {
> print;
> }
> @@ -1610,6 +1741,18 @@ sub patch_update_file {
> next;
> }
> }
> + elsif ($line =~ /^l/) {
> + unless ($other =~ /l/) {
> + error_msg __("Cannot select line by line\n");
> + next;
> + }
> + my $newhunk = select_lines_loop($hunk[$ix]);
> + if ($newhunk) {
> + splice @hunk, $ix, 1, $newhunk;
> + } else {
> + next;
> + }
> + }
> elsif ($other =~ /s/ && $line =~ /^s/) {
> my @split = split_hunk($hunk[$ix]{TEXT}, $hunk[$ix]{DISPLAY});
> if (1 < @split) {
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index a9a9478a29..65c8c3354b 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -360,6 +360,63 @@ test_expect_failure 'split hunk "add -p (no, yes, edit)"' '
> ! grep "^+31" actual
> '
>
> +test_expect_success 'setup expected diff' '
> + cat >expected <<-\EOF
> + diff --git a/test b/test
> + index 0889435..341cc6b 100644
> + --- a/test
> + +++ b/test
> + @@ -1,6 +1,9 @@
> + +5
> + 10
> + 20
> + +21
> + 30
> + 40
> + 50
> + 60
> + +61
> + \ No newline at end of file
> + EOF
> +'
> +
> +test_expect_success 'can stage individual lines of patch' '
Here, you`re actually using "stage lines" yourself, good ;)
> + git reset &&
> + printf 61 >>test &&
> + printf "%s\n" l "-2 4" |
> + EDITOR=: git add -p 2>error &&
> + test_must_be_empty error &&
> + git diff --cached HEAD >actual &&
> + diff_cmp expected actual
> +'
> +
> +test_expect_success 'setup expected diff' '
> + cat >expected <<-\EOF
> + diff --git a/test b/test
> + index 0889435..cc6163b 100644
> + --- a/test
> + +++ b/test
> + @@ -1,6 +1,8 @@
> + +5
> + 10
> + 20
> + 30
> + 40
> + 50
> + 60
> + +61
> + \ No newline at end of file
> + EOF
> +'
> +
> +test_expect_success 'can reset individual lines of patch' '
> + printf "%s\n" l 2 |
> + EDITOR=: git reset -p 2>error &&
> + test_must_be_empty error &&
> + git diff --cached HEAD >actual &&
> + diff_cmp expected actual
> +'
> +
> test_expect_success 'patch mode ignores unmerged entries' '
> git reset --hard &&
> test_commit conflict &&
> @@ -576,4 +633,12 @@ test_expect_success 'add -p patch editing works with pathological context lines'
> test_cmp expected-2 actual
> '
>
> +test_expect_success 'add -p selecting lines works with pathological context lines' '
Maybe s/selecting lines/staging lines/ ?
> + git reset &&
> + printf "%s\n" l 2 y |
> + GIT_EDITOR=./editor git add -p &&
> + git cat-file blob :a >actual &&
> + test_cmp expected-2 actual
> +'
> +
> test_done
>
