Merge branch 'pw/add-p-select' into next
authorJunio C Hamano <gitster@pobox.com>
Fri, 30 Mar 2018 19:42:06 +0000 (12:42 -0700)
committerJunio C Hamano <gitster@pobox.com>
Fri, 30 Mar 2018 19:42:06 +0000 (12:42 -0700)
"git add -p" interactive interface learned to let users choose
individual added/removed lines to be used in the operation, instead
of accepting or rejecting a whole hunk.

* pw/add-p-select:
add -p: optimize line selection for short hunks
add -p: allow line selection to be inverted
add -p: select individual hunk lines

1  2 
git-add--interactive.perl
t/t3701-add-interactive.sh
index d190469cd8b5e1dc427b4029d0d1fb937faef584,86e373a1011e012c2fd349fca2b85866ef599dfb..cb48fc3e8e1e046a1076826bddc04ae298a9a9e5
@@@ -262,7 -262,7 +262,7 @@@ sub list_modified 
                }
        }
  
 -      for (run_cmd_pipe(qw(git diff-files --numstat --summary --raw --), @ARGV)) {
 +      for (run_cmd_pipe(qw(git diff-files --ignore-submodules=dirty --numstat --summary --raw --), @ARGV)) {
                if (($add, $del, $file) =
                    /^([-\d]+)  ([-\d]+)        (.*)/) {
                        $file = unquote_path($file);
@@@ -705,14 -705,6 +705,14 @@@ sub parse_diff 
        }
        my (@hunk) = { TEXT => [], DISPLAY => [], TYPE => 'header' };
  
 +      if (@colored && @colored != @diff) {
 +              print STDERR
 +                "fatal: mismatched output from interactive.diffFilter\n",
 +                "hint: Your filter must maintain a one-to-one correspondence\n",
 +                "hint: between its input and output lines.\n";
 +              exit 1;
 +      }
 +
        for (my $i = 0; $i < @diff; $i++) {
                if ($diff[$i] =~ /^@@ /) {
                        push @hunk, { TEXT => [], DISPLAY => [],
@@@ -1021,6 -1013,171 +1021,171 @@@ 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 {
+       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 split_hunk_selection {
+       local $_;
+       my @fields = @_;
+       my @ret;
+       for (@fields) {
+               while ($_ ne '') {
+                       if (/^[0-9]-$/) {
+                               push @ret, $_;
+                               last;
+                       } elsif (/^([0-9](?:-[0-9])?)(.*)/) {
+                               push @ret, $1;
+                               $_ = $2;
+                       } else {
+                               error_msg sprintf
+                                   __("invalid hunk line '%s'\n"),
+                                   substr($_, 0, 1);
+                               return ();
+                       }
+               }
+       }
+       return @ret;
+ }
+ sub parse_hunk_selection {
+       local $_;
+       my ($hunk, $line) = @_;
+       my ($max_label, $invert) = ($hunk->{MAX_LABEL}, undef);
+       my @selected = (0) x ($max_label + 1);
+       my @fields = split(/[,\s]+/, $line);
+       if ($fields[0] =~ /^-(.*)/) {
+               $invert = 1;
+               if ($1 ne '') {
+                       $fields[0] = $1;
+               } else {
+                       shift @fields;
+                       unless (@fields) {
+                               error_msg __("no lines to invert\n");
+                               return undef;
+                       }
+               }
+       }
+       if ($max_label < 10) {
+               @fields = split_hunk_selection(@fields) or return undef;
+       }
+       for (@fields) {
+               if (my ($lo, $hi) = /^([0-9]+)-([0-9]*)$/) {
+                       if ($hi eq '') {
+                               $hi = $max_label;
+                       }
+                       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;
+               }
+       }
+       if ($invert) {
+               @selected = map { !$_ } @selected;
+       }
+       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
@@@ -1256,19 -1413,14 +1421,20 @@@ d - do not apply this hunk or any of th
  );
  
  sub help_patch_cmd {
 -      print colored $help_color, __($help_patch_modes{$patch_mode}), "\n", __ <<EOF ;
 +      local $_;
 +      my $other = $_[0] . ",?";
 +      print colored $help_color, __($help_patch_modes{$patch_mode}), "\n",
 +              map { "$_\n" } grep {
 +                      my $c = quotemeta(substr($_, 0, 1));
 +                      $other =~ /,$c/
 +              } split "\n", __ <<EOF ;
  g - select a hunk to go to
  / - search for a hunk matching the given regex
  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 - split the current hunk into smaller hunks
  e - manually edit the current hunk
  ? - print help
@@@ -1380,39 -1532,39 +1546,39 @@@ sub display_hunks 
  
  my %patch_update_prompt_modes = (
        stage => {
 -              mode => N__("Stage mode change [y,n,q,a,d,/%s,?]? "),
 -              deletion => N__("Stage deletion [y,n,q,a,d,/%s,?]? "),
 -              hunk => N__("Stage this hunk [y,n,q,a,d,/%s,?]? "),
 +              mode => N__("Stage mode change [y,n,q,a,d%s,?]? "),
 +              deletion => N__("Stage deletion [y,n,q,a,d%s,?]? "),
 +              hunk => N__("Stage this hunk [y,n,q,a,d%s,?]? "),
        },
        stash => {
 -              mode => N__("Stash mode change [y,n,q,a,d,/%s,?]? "),
 -              deletion => N__("Stash deletion [y,n,q,a,d,/%s,?]? "),
 -              hunk => N__("Stash this hunk [y,n,q,a,d,/%s,?]? "),
 +              mode => N__("Stash mode change [y,n,q,a,d%s,?]? "),
 +              deletion => N__("Stash deletion [y,n,q,a,d%s,?]? "),
 +              hunk => N__("Stash this hunk [y,n,q,a,d%s,?]? "),
        },
        reset_head => {
 -              mode => N__("Unstage mode change [y,n,q,a,d,/%s,?]? "),
 -              deletion => N__("Unstage deletion [y,n,q,a,d,/%s,?]? "),
 -              hunk => N__("Unstage this hunk [y,n,q,a,d,/%s,?]? "),
 +              mode => N__("Unstage mode change [y,n,q,a,d%s,?]? "),
 +              deletion => N__("Unstage deletion [y,n,q,a,d%s,?]? "),
 +              hunk => N__("Unstage this hunk [y,n,q,a,d%s,?]? "),
        },
        reset_nothead => {
 -              mode => N__("Apply mode change to index [y,n,q,a,d,/%s,?]? "),
 -              deletion => N__("Apply deletion to index [y,n,q,a,d,/%s,?]? "),
 -              hunk => N__("Apply this hunk to index [y,n,q,a,d,/%s,?]? "),
 +              mode => N__("Apply mode change to index [y,n,q,a,d%s,?]? "),
 +              deletion => N__("Apply deletion to index [y,n,q,a,d%s,?]? "),
 +              hunk => N__("Apply this hunk to index [y,n,q,a,d%s,?]? "),
        },
        checkout_index => {
 -              mode => N__("Discard mode change from worktree [y,n,q,a,d,/%s,?]? "),
 -              deletion => N__("Discard deletion from worktree [y,n,q,a,d,/%s,?]? "),
 -              hunk => N__("Discard this hunk from worktree [y,n,q,a,d,/%s,?]? "),
 +              mode => N__("Discard mode change from worktree [y,n,q,a,d%s,?]? "),
 +              deletion => N__("Discard deletion from worktree [y,n,q,a,d%s,?]? "),
 +              hunk => N__("Discard this hunk from worktree [y,n,q,a,d%s,?]? "),
        },
        checkout_head => {
 -              mode => N__("Discard mode change from index and worktree [y,n,q,a,d,/%s,?]? "),
 -              deletion => N__("Discard deletion from index and worktree [y,n,q,a,d,/%s,?]? "),
 -              hunk => N__("Discard this hunk from index and worktree [y,n,q,a,d,/%s,?]? "),
 +              mode => N__("Discard mode change from index and worktree [y,n,q,a,d%s,?]? "),
 +              deletion => N__("Discard deletion from index and worktree [y,n,q,a,d%s,?]? "),
 +              hunk => N__("Discard this hunk from index and worktree [y,n,q,a,d%s,?]? "),
        },
        checkout_nothead => {
 -              mode => N__("Apply mode change to index and worktree [y,n,q,a,d,/%s,?]? "),
 -              deletion => N__("Apply deletion to index and worktree [y,n,q,a,d,/%s,?]? "),
 -              hunk => N__("Apply this hunk to index and worktree [y,n,q,a,d,/%s,?]? "),
 +              mode => N__("Apply mode change to index and worktree [y,n,q,a,d%s,?]? "),
 +              deletion => N__("Apply deletion to index and worktree [y,n,q,a,d%s,?]? "),
 +              hunk => N__("Apply this hunk to index and worktree [y,n,q,a,d%s,?]? "),
        },
  );
  
@@@ -1468,7 -1620,7 +1634,7 @@@ sub patch_update_file 
                        $other .= ',J';
                }
                if ($num > 1) {
 -                      $other .= ',g';
 +                      $other .= ',g,/';
                }
                for ($i = 0; $i < $num; $i++) {
                        if (!defined $hunk[$i]{USE}) {
                if ($hunk[$ix]{TYPE} eq 'hunk') {
                        $other .= ',e';
                }
+               if (label_hunk_lines($hunk[$ix])) {
+                       $other .= ',l';
+               }
                for (@{$hunk[$ix]{DISPLAY}}) {
                        print;
                }
                                }
                                next;
                        }
 -                      elsif ($other =~ /g/ && $line =~ /^g(.*)/) {
 +                      elsif ($line =~ /^g(.*)/) {
                                my $response = $1;
 +                              unless ($other =~ /g/) {
 +                                      error_msg __("No other hunks to goto\n");
 +                                      next;
 +                              }
                                my $no = $ix > 10 ? $ix - 10 : 0;
                                while ($response eq '') {
                                        $no = display_hunks(\@hunk, $no);
                        }
                        elsif ($line =~ m|^/(.*)|) {
                                my $regex = $1;
 +                              unless ($other =~ m|/|) {
 +                                      error_msg __("No other hunks to search\n");
 +                                      next;
 +                              }
                                if ($1 eq "") {
                                        print colored $prompt_color, __("search for regex? ");
                                        $regex = <STDIN>;
                                        next;
                                }
                        }
 -                      elsif ($other =~ /s/ && $line =~ /^s/) {
+                       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 ($line =~ /^s/) {
 +                              unless ($other =~ /s/) {
 +                                      error_msg __("Sorry, cannot split this hunk\n");
 +                                      next;
 +                              }
                                my @split = split_hunk($hunk[$ix]{TEXT}, $hunk[$ix]{DISPLAY});
                                if (1 < @split) {
                                        print colored $header_color, sprintf(
                                $num = scalar @hunk;
                                next;
                        }
 -                      elsif ($other =~ /e/ && $line =~ /^e/) {
 +                      elsif ($line =~ /^e/) {
 +                              unless ($other =~ /e/) {
 +                                      error_msg __("Sorry, cannot edit this hunk\n");
 +                                      next;
 +                              }
                                my $newhunk = edit_hunk_loop($head, \@hunk, $ix);
                                if (defined $newhunk) {
                                        splice @hunk, $ix, 1, $newhunk;
index b170fb02b80356455d03dcf379636778149665a6,b842b3b4c592d557c092caee44244c2df3218c15..8954481995129292c99208b59b368e231d819605
@@@ -360,6 -360,63 +360,63 @@@ test_expect_failure 'split hunk "add -
        ! 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' '
+       git reset &&
+       printf 61 >>test &&
+       printf "%s\n" l "1,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 -13 |
+       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 &&
@@@ -397,26 -454,6 +454,26 @@@ test_expect_success TTY 'diffs can be c
        grep "$(printf "\\033")" output
  '
  
 +test_expect_success TTY 'diffFilter filters diff' '
 +      git reset --hard &&
 +
 +      echo content >test &&
 +      test_config interactive.diffFilter "sed s/^/foo:/" &&
 +      printf y | test_terminal git add -p >output 2>&1 &&
 +
 +      # avoid depending on the exact coloring or content of the prompts,
 +      # and just make sure we saw our diff prefixed
 +      grep foo:.*content output
 +'
 +
 +test_expect_success TTY 'detect bogus diffFilter output' '
 +      git reset --hard &&
 +
 +      echo content >test &&
 +      test_config interactive.diffFilter "echo too-short" &&
 +      printf y | test_must_fail test_terminal git add -p
 +'
 +
  test_expect_success 'patch-mode via -i prompts for files' '
        git reset --hard &&
  
@@@ -518,54 -555,6 +575,54 @@@ test_expect_success 'add -p works even 
        test_cmp expect actual
  '
  
 +test_expect_success 'setup different kinds of dirty submodules' '
 +      test_create_repo for-submodules &&
 +      (
 +              cd for-submodules &&
 +              test_commit initial &&
 +              test_create_repo dirty-head &&
 +              (
 +                      cd dirty-head &&
 +                      test_commit initial
 +              ) &&
 +              cp -R dirty-head dirty-otherwise &&
 +              cp -R dirty-head dirty-both-ways &&
 +              git add dirty-head &&
 +              git add dirty-otherwise dirty-both-ways &&
 +              git commit -m initial &&
 +
 +              cd dirty-head &&
 +              test_commit updated &&
 +              cd ../dirty-both-ways &&
 +              test_commit updated &&
 +              echo dirty >>initial &&
 +              : >untracked &&
 +              cd ../dirty-otherwise &&
 +              echo dirty >>initial &&
 +              : >untracked
 +      ) &&
 +      git -C for-submodules diff-files --name-only >actual &&
 +      cat >expected <<-\EOF &&
 +      dirty-both-ways
 +      dirty-head
 +      dirty-otherwise
 +      EOF
 +      test_cmp expected actual &&
 +      git -C for-submodules diff-files --name-only --ignore-submodules=dirty >actual &&
 +      cat >expected <<-\EOF &&
 +      dirty-both-ways
 +      dirty-head
 +      EOF
 +      test_cmp expected actual
 +'
 +
 +test_expect_success 'status ignores dirty submodules (except HEAD)' '
 +      git -C for-submodules add -i </dev/null >output &&
 +      grep dirty-head output &&
 +      grep dirty-both-ways output &&
 +      ! grep dirty-otherwise output
 +'
 +
  test_expect_success 'set up pathological context' '
        git reset --hard &&
        test_write_lines a a a a a a a a a a a >a &&
@@@ -596,4 -585,12 +653,12 @@@ test_expect_success 'add -p patch editi
        test_cmp expected-2 actual
  '
  
+ test_expect_success 'add -p selecting lines works with pathological context 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