From: Junio C Hamano Date: Fri, 30 Mar 2018 19:42:06 +0000 (-0700) Subject: Merge branch 'pw/add-p-select' into next X-Git-Url: https://git.lorimer.id.au/gitweb.git/diff_plain/eae69f5ded514f6c7a961372f0dd67e6429e861c?hp=-c Merge branch 'pw/add-p-select' into next "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 --- eae69f5ded514f6c7a961372f0dd67e6429e861c diff --combined git-add--interactive.perl index d190469cd8,86e373a101..cb48fc3e8e --- a/git-add--interactive.perl +++ b/git-add--interactive.perl @@@ -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 = ; + 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", __ < { - 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}) { @@@ -1485,6 -1637,9 +1651,9 @@@ if ($hunk[$ix]{TYPE} eq 'hunk') { $other .= ',e'; } + if (label_hunk_lines($hunk[$ix])) { + $other .= ',l'; + } for (@{$hunk[$ix]{DISPLAY}}) { print; } @@@ -1509,12 -1664,8 +1678,12 @@@ } 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); @@@ -1560,10 -1711,6 +1729,10 @@@ } 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 = ; @@@ -1632,11 -1779,19 +1801,23 @@@ 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/) { + 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( @@@ -1648,11 -1803,7 +1829,11 @@@ $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; diff --combined t/t3701-add-interactive.sh index b170fb02b8,b842b3b4c5..8954481995 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@@ -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 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