From 8c159044625e46de67cd8467f07424f38eb8301e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Slavica=20=C4=90uki=C4=87?= Date: Fri, 15 Nov 2019 11:11:20 +0000 Subject: built-in add -i: implement the `help` command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This imitates the code to show the help text from the Perl script `git-add--interactive.perl` in the built-in version. To make sure that it renders exactly like the Perl version of `git add -i`, we also add a test case for that to `t3701-add-interactive.sh`. Signed-off-by: Slavica Đukić Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t3701-add-interactive.sh | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 't/t3701-add-interactive.sh') diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index d50e165ca8..d4f9386621 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -647,4 +647,29 @@ test_expect_success 'checkout -p works with pathological context lines' ' test_write_lines a b a b a a b a b a >expect && test_cmp expect a ' + +test_expect_success 'show help from add--helper' ' + git reset --hard && + cat >expect <<-EOF && + + *** Commands *** + 1: status 2: update 3: revert 4: add untracked + 5: patch 6: diff 7: quit 8: help + What now> status - show paths with changes + update - add working tree state to the staged set of changes + revert - revert staged set of changes back to the HEAD version + patch - pick hunks and update selectively + diff - view diff between HEAD and index + add untracked - add contents of untracked files to the staged set of changes + *** Commands *** + 1: status 2: update 3: revert 4: add untracked + 5: patch 6: diff 7: quit 8: help + What now>$SP + Bye. + EOF + test_write_lines h | GIT_PAGER_IN_USE=true TERM=vt100 git add -i >actual.colored && + test_decode_color actual && + test_i18ncmp expect actual +' + test_done -- cgit v1.2.3 From 0f0fba2cc87219bf0c182201b7798ceb74c24857 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 6 Dec 2019 13:08:19 +0000 Subject: t3701: add a test for advanced split-hunk editing In this developer's workflows, it often happens that a hunk needs to be edited in a way that adds lines, and sometimes even reduces the number of context lines. Let's add a regression test for this. Note that just like the preceding test case, the new test case is *not* handled gracefully by the current `git add -p`. It will be handled correctly by the upcoming built-in `git add -p`, though. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t3701-add-interactive.sh | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 't/t3701-add-interactive.sh') diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index d4f9386621..4da99e27af 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -403,6 +403,28 @@ test_expect_failure 'split hunk "add -p (no, yes, edit)"' ' ! grep "^+31" actual ' +test_expect_failure 'edit, adding lines to the first hunk' ' + test_write_lines 10 11 20 30 40 50 51 60 >test && + git reset && + tr _ " " >patch <<-EOF && + @@ -1,5 +1,6 @@ + _10 + +11 + +12 + _20 + +21 + +22 + _30 + EOF + # test sequence is s(plit), e(dit), n(o) + # q n q q is there to make sure we exit at the end. + printf "%s\n" s e n q n q q | + EDITOR=./fake_editor.sh git add -p 2>error && + test_must_be_empty error && + git diff --cached >actual && + grep "^+22" actual +' + test_expect_success 'patch mode ignores unmerged entries' ' git reset --hard && test_commit conflict && -- cgit v1.2.3 From 8539b465341cc475f219ed46273a1c157bddafa0 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 6 Dec 2019 13:08:20 +0000 Subject: t3701: avoid depending on the TTY prerequisite The TTY prerequisite is a rather heavy one: it not only requires Perl to work, but also the IO/Pty.pm module (with native support, and it requires pseudo terminals, too). In particular, test cases marked with the TTY prerequisite would be skipped in Git for Windows' SDK. In the case of `git add -p`, we do not actually need that big a hammer, as we do not want to test any functionality that requires a pseudo terminal; all we want is for the interactive add command to use color, even when being called from within the test suite. And we found exactly such a trick earlier already: when we added a test case to verify that the main loop of `git add -i` is colored appropriately. Let's use that trick instead of the TTY prerequisite. While at it, we avoid the pipes, as we do not want a SIGPIPE to break the regression test cases (which will be much more likely when we do not run everything through Perl because that is inherently slower). Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t3701-add-interactive.sh | 28 +++++++++++++++++++++------- 1 file changed, 21 insertions(+), 7 deletions(-) (limited to 't/t3701-add-interactive.sh') diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 4da99e27af..793ce28297 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -23,6 +23,17 @@ diff_cmp () { test_cmp "$1.filtered" "$2.filtered" } +# This function uses a trick to manipulate the interactive add to use color: +# the `want_color()` function special-cases the situation where a pager was +# spawned and Git now wants to output colored text: to detect that situation, +# the environment variable `GIT_PAGER_IN_USE` is set. However, color is +# suppressed despite that environment variable if the `TERM` variable +# indicates a dumb terminal, so we set that variable, too. + +force_color () { + env GIT_PAGER_IN_USE=true TERM=vt100 "$@" +} + test_expect_success 'setup (initial)' ' echo content >file && git add file && @@ -451,35 +462,38 @@ test_expect_success 'patch mode ignores unmerged entries' ' diff_cmp expected diff ' -test_expect_success TTY 'diffs can be colorized' ' +test_expect_success 'diffs can be colorized' ' git reset --hard && echo content >test && - printf y | test_terminal git add -p >output 2>&1 && + printf y >y && + force_color git add -p >output 2>&1 test && test_config interactive.diffFilter "sed s/^/foo:/" && - printf y | test_terminal git add -p >output 2>&1 && + printf y >y && + force_color git add -p >output 2>&1 test && test_config interactive.diffFilter "echo too-short" && - printf y | test_must_fail test_terminal git add -p + printf y >y && + test_must_fail force_color git add -p What now>$SP Bye. EOF - test_write_lines h | GIT_PAGER_IN_USE=true TERM=vt100 git add -i >actual.colored && + test_write_lines h | force_color git add -i >actual.colored && test_decode_color actual && test_i18ncmp expect actual ' -- cgit v1.2.3 From 24be352d52f96b2cace4d3e5f01f02917b7d649b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 6 Dec 2019 13:08:21 +0000 Subject: t3701: add a test for the different `add -p` prompts The `git add -p` command offers different prompts for regular diff hunks vs mode change pseudo hunks vs diffs deleting files. Let's cover this in the regresion test suite, in preparation for re-implementing `git add -p` in C. For the mode change prompt, we use a trick that lets this test case pass even on systems without executable bit, i.e. where `core.filemode = false` (such as Windows): we first add the file to the index with `git add --chmod=+x`, and then call `git add -p` with `core.filemode` forced to `true`. The file on disk has no executable bit set, therefore we will see a mode change. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t3701-add-interactive.sh | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) (limited to 't/t3701-add-interactive.sh') diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 793ce28297..c90aaa25b0 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -105,7 +105,6 @@ test_expect_success 'revert works (commit)' ' grep "unchanged *+3/-0 file" output ' - test_expect_success 'setup expected' ' cat >expected <<-\EOF EOF @@ -274,6 +273,24 @@ test_expect_success FILEMODE 'stage mode and hunk' ' # end of tests disabled when filemode is not usable +test_expect_success 'different prompts for mode change/deleted' ' + git reset --hard && + >file && + >deleted && + git add --chmod=+x file deleted && + echo changed >file && + rm deleted && + test_write_lines n n n | + git -c core.filemode=true add -p >actual && + sed -n "s/^\(([0-9/]*) Stage .*?\).*/\1/p" actual >actual.filtered && + cat >expect <<-\EOF && + (1/1) Stage deletion [y,n,q,a,d,?]? + (1/2) Stage mode change [y,n,q,a,d,j,J,g,/,?]? + (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? + EOF + test_cmp expect actual.filtered +' + test_expect_success 'setup again' ' git reset --hard && test_chmod +x file && -- cgit v1.2.3 From 0c3222c4f322c586099d2773e180dabf6d4f6568 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 6 Dec 2019 13:08:22 +0000 Subject: t3701: verify the shown messages when nothing can be added In preparation for re-implementing `git add -p` in pure C (where we will purposefully keep the implementation of `git add -p` separate from the implementation of `git add -i`), let's verify that the user is told the same things as in the Perl version when the diff file is either empty or contains only entries about binary files. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t3701-add-interactive.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 't/t3701-add-interactive.sh') diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index c90aaa25b0..797610e96d 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -291,6 +291,17 @@ test_expect_success 'different prompts for mode change/deleted' ' test_cmp expect actual.filtered ' +test_expect_success 'correct message when there is nothing to do' ' + git reset --hard && + git add -p 2>err && + test_i18ngrep "No changes" err && + printf "\\0123" >binary && + git add binary && + printf "\\0abc" >binary && + git add -p 2>err && + test_i18ngrep "Only binary files changed" err +' + test_expect_success 'setup again' ' git reset --hard && test_chmod +x file && -- cgit v1.2.3 From e91162be9ce7195309dc2b7e3c03988481cee850 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 6 Dec 2019 13:08:23 +0000 Subject: t3701: verify that the diff.algorithm config setting is handled Without this patch, there is actually no test in Git's test suite that covers the diff.algorithm feature. Let's add one. We do this by passing a bogus value and then expecting `git diff-files` to produce the appropriate error message. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t3701-add-interactive.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 't/t3701-add-interactive.sh') diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 797610e96d..f43634102e 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -524,6 +524,16 @@ test_expect_success 'detect bogus diffFilter output' ' test_must_fail force_color git add -p file && + git add file && + echo changed >file && + git -c diff.algorithm=bogus add -p 2>err && + test_i18ngrep "error: option diff-algorithm accepts " err +' + test_expect_success 'patch-mode via -i prompts for files' ' git reset --hard && -- cgit v1.2.3 From 89c8559367aae771006cc0956b6f5e54cc8c614c Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 6 Dec 2019 13:08:24 +0000 Subject: git add -p: use non-zero exit code when the diff generation failed The first thing `git add -p` does is to generate a diff. If this diff cannot be generated, `git add -p` should not continue as if nothing happened, but instead fail. What we *actually* do here is much broader: we now verify for *every* `run_cmd_pipe()` call that the spawned process actually succeeded. Note that we have to change two callers in this patch, as we need to store the spawned process' output in a local variable, which means that the callers can no longer decide whether to interpret the `return <$fh>` in array or in scalar context. This bug was noticed while writing a test case for the diff.algorithm feature, and we let that test case double as a regression test for this fixed bug, too. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t3701-add-interactive.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 't/t3701-add-interactive.sh') diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index f43634102e..5db6432e33 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -530,7 +530,7 @@ test_expect_success 'diff.algorithm is passed to `git diff-files`' ' >file && git add file && echo changed >file && - git -c diff.algorithm=bogus add -p 2>err && + test_must_fail git -c diff.algorithm=bogus add -p 2>err && test_i18ngrep "error: option diff-algorithm accepts " err ' -- cgit v1.2.3 From 510aeca199c4feeb38d318cc151ecf5464a3a865 Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 13 Dec 2019 08:07:58 +0000 Subject: built-in add -p: implement the hunk splitting feature If this developer's workflow is any indication, then this is *the* most useful feature of Git's interactive `add `command. Note: once again, this is not a verbatim conversion from the Perl code to C: the `hunk_splittable()` function, for example, essentially did all the work of splitting the hunk, just to find out whether more than one hunk would have been the result (and then tossed that result into the trash). In C we instead count the number of resulting hunks (without actually doing the work of splitting, but just counting the transitions from non-context lines to context lines), and store that information with the hunk, and we do that *while* parsing the diff in the first place. Another deviation: the built-in `git add -p` was designed with a single strbuf holding the diff (and another one holding the colored diff, if that one was asked for) in mind, and hunks essentially store just the start and end offsets pointing into that strbuf. As a consequence, when we split hunks, we now use a special mode where the hunk header is generated dynamically, and only the rest of the hunk is stored using such start/end offsets. This way, we also avoid the frequent formatting/re-parsing of the hunk header of the Perl version. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t3701-add-interactive.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 't/t3701-add-interactive.sh') diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 5db6432e33..fe383be50e 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -442,6 +442,18 @@ test_expect_failure 'split hunk "add -p (no, yes, edit)"' ' ! grep "^+31" actual ' +test_expect_success 'split hunk with incomplete line at end' ' + git reset --hard && + printf "missing LF" >>test && + git add test && + test_write_lines before 10 20 30 40 50 60 70 >test && + git grep --cached missing && + test_write_lines s n y q | git add -p && + test_must_fail git grep --cached missing && + git grep before && + test_must_fail git grep --cached before +' + test_expect_failure 'edit, adding lines to the first hunk' ' test_write_lines 10 11 20 30 40 50 51 60 >test && git reset && -- cgit v1.2.3 From 9254bdfb4f971be4be5364341a0387e8a1f3734a Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 13 Dec 2019 08:08:02 +0000 Subject: built-in add -p: implement the 'g' ("goto") command With this patch, it is now possible to see a summary of the available hunks and to navigate between them (by number). A test is added to verify that this behavior matches the one of the Perl version of `git add -p`. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t3701-add-interactive.sh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) (limited to 't/t3701-add-interactive.sh') diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index fe383be50e..57c656a20c 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -413,6 +413,22 @@ test_expect_success 'split hunk setup' ' test_write_lines 10 15 20 21 22 23 24 30 40 50 60 >test ' +test_expect_success 'goto hunk' ' + test_when_finished "git reset" && + tr _ " " >expect <<-EOF && + (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? + 1: -1,2 +1,3 +15 + _ 2: -2,4 +3,8 +21 + go to which hunk? @@ -1,2 +1,3 @@ + _10 + +15 + _20 + (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?_ + EOF + test_write_lines s y g 1 | git add -p >actual && + tail -n 7 actual.trimmed && + test_cmp expect actual.trimmed +' + test_expect_success 'split hunk "add -p (edit)"' ' # Split, say Edit and do nothing. Then: # -- cgit v1.2.3 From d6cf873340703098f2d7f54be457e4db1b32cf7b Mon Sep 17 00:00:00 2001 From: Johannes Schindelin Date: Fri, 13 Dec 2019 08:08:03 +0000 Subject: built-in add -p: implement the '/' ("search regex") command This patch implements the hunk searching feature in the C version of `git add -p`. A test is added to verify that this behavior matches the one of the Perl version of `git add -p`. Note that this involves a change of behavior: the Perl version uses (of course) the Perl flavor of regular expressions, while this patch uses the regcomp()/regexec(), i.e. POSIX extended regular expressions. In practice, this behavior change is unlikely to matter. Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- t/t3701-add-interactive.sh | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 't/t3701-add-interactive.sh') diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh index 57c656a20c..12ee321707 100755 --- a/t/t3701-add-interactive.sh +++ b/t/t3701-add-interactive.sh @@ -429,6 +429,20 @@ test_expect_success 'goto hunk' ' test_cmp expect actual.trimmed ' +test_expect_success 'navigate to hunk via regex' ' + test_when_finished "git reset" && + tr _ " " >expect <<-EOF && + (2/2) Stage this hunk [y,n,q,a,d,K,g,/,e,?]? @@ -1,2 +1,3 @@ + _10 + +15 + _20 + (1/2) Stage this hunk [y,n,q,a,d,j,J,g,/,e,?]?_ + EOF + test_write_lines s y /1,2 | git add -p >actual && + tail -n 5 actual.trimmed && + test_cmp expect actual.trimmed +' + test_expect_success 'split hunk "add -p (edit)"' ' # Split, say Edit and do nothing. Then: # -- cgit v1.2.3