Merge branch 'pw/add-p-recount'
authorJunio C Hamano <gitster@pobox.com>
Thu, 28 Jun 2018 19:53:32 +0000 (12:53 -0700)
committerJunio C Hamano <gitster@pobox.com>
Thu, 28 Jun 2018 19:53:32 +0000 (12:53 -0700)
When user edits the patch in "git add -p" and the user's editor is
set to strip trailing whitespaces indiscriminately, an empty line
that is unchanged in the patch would become completely empty
(instead of a line with a sole SP on it). The code introduced in
Git 2.17 timeframe failed to parse such a patch, but now it learned
to notice the situation and cope with it.

* pw/add-p-recount:
add -p: fix counting empty context lines in edited patches

1  2 
git-add--interactive.perl
t/t3701-add-interactive.sh
index 36f38ced902e1902a626933bdcda09819acf39a7,8361ef45e722f84140995e48f26ff2fa3c1b520b..20eb81cc92f947d872b31a179d43d97772ff25e4
@@@ -205,15 -205,8 +205,15 @@@ my $status_head = sprintf($status_fmt, 
        }
  }
  
 -sub get_empty_tree {
 -      return '4b825dc642cb6eb9a060e54bf8d69288fbee4904';
 +{
 +      my $empty_tree;
 +      sub get_empty_tree {
 +              return $empty_tree if defined $empty_tree;
 +
 +              $empty_tree = run_cmd_pipe(qw(git hash-object -t tree /dev/null));
 +              chomp $empty_tree;
 +              return $empty_tree;
 +      }
  }
  
  sub get_diff_reference {
@@@ -269,7 -262,7 +269,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);
@@@ -712,14 -705,6 +712,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 => [],
@@@ -1062,7 -1047,7 +1062,7 @@@ sub recount_edited_hunk 
                        $o_cnt++;
                } elsif ($mode eq '+') {
                        $n_cnt++;
-               } elsif ($mode eq ' ') {
+               } elsif ($mode eq ' ' or $mode eq "\n") {
                        $o_cnt++;
                        $n_cnt++;
                }
@@@ -1263,13 -1248,7 +1263,13 @@@ 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
@@@ -1387,39 -1366,39 +1387,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,?]? "),
        },
  );
  
@@@ -1475,7 -1454,7 +1475,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}) {
                                }
                                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;
 -                              if ($1 eq "") {
 +                              unless ($other =~ m|/|) {
 +                                      error_msg __("No other hunks to search\n");
 +                                      next;
 +                              }
 +                              if ($regex eq "") {
                                        print colored $prompt_color, __("search for regex? ");
                                        $regex = <STDIN>;
                                        if (defined $regex) {
                                        next;
                                }
                        }
 -                      elsif ($other =~ /s/ && $line =~ /^s/) {
 +                      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,f1bb879ea43321a1ebc3b933a2a190a8e23eed86..3e9139dca88e83165fc21e6dce06f3243bd49cd7
@@@ -175,6 -175,49 +175,49 @@@ test_expect_success 'real edit works' 
        diff_cmp expected output
  '
  
+ test_expect_success 'setup file' '
+       test_write_lines a "" b "" c >file &&
+       git add file &&
+       test_write_lines a "" d "" c >file
+ '
+ test_expect_success 'setup patch' '
+       SP=" " &&
+       NULL="" &&
+       cat >patch <<-EOF
+       @@ -1,4 +1,4 @@
+        a
+       $NULL
+       -b
+       +f
+       $SP
+       c
+       EOF
+ '
+ test_expect_success 'setup expected' '
+       cat >expected <<-EOF
+       diff --git a/file b/file
+       index b5dd6c9..f910ae9 100644
+       --- a/file
+       +++ b/file
+       @@ -1,5 +1,5 @@
+        a
+       $SP
+       -f
+       +d
+       $SP
+        c
+       EOF
+ '
+ test_expect_success 'edit can strip spaces from empty context lines' '
+       test_write_lines e n q | git add -p 2>error &&
+       test_must_be_empty error &&
+       git diff >output &&
+       diff_cmp expected output
+ '
  test_expect_success 'skip files similarly as commit -a' '
        git reset &&
        echo file >.gitignore &&
@@@ -397,26 -440,6 +440,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 -541,6 +561,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 &&