summaryrefslogtreecommitdiff
path: root/t/t3701-add-interactive.sh
AgeCommit message (Collapse)AuthorFilesLines
2022-01-12builtin add -p: fix hunk splittingLibravatar Phillip Wood1-4/+42
The C reimplementation of "add -p" fails to split the last hunk in a file if hunk ends with an addition or deletion without any post context line unless it is the last file to be processed. To determine whether a hunk can be split a counter is incremented each time a context line follows an insertion or deletion. If at the end of the hunk the value of this counter is greater than one then the hunk can be split into that number of smaller hunks. If the last hunk in a file ends with an insertion or deletion then there is no following context line and the counter will not be incremented. This case is already handled at the end of the loop where counter is incremented if the last hunk ended with an insertion or deletion. Unfortunately there is no similar check between files (likely because the perl version only ever parses one diff at a time). Fix this by checking if the last hunk ended with an insertion or deletion when we see the diff header of a new file and extend the existing regression test. Reproted-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2022-01-12t3701: clean up hunk splitting testsLibravatar Phillip Wood1-4/+4
Clean up some test constructs in preparation for extending the tests in the next commit. There are three small changes, I've grouped them together as they're so small it didn't seem worth creating three separate commits. 1 - "cat file | sed expression" is better written as "sed expression file". 2 - Follow our usual practice of redirecting the output of git commands to a file rather than piping it into another command. 3 - Use test_write_lines rather than 'printf "%s\n"'. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-10tests: remove most uses of test_i18ncmpLibravatar Ævar Arnfjörð Bjarmason1-1/+1
As a follow-up to d162b25f956 (tests: remove support for GIT_TEST_GETTEXT_POISON, 2021-01-20) remove most uses of test_i18ncmp via a simple s/test_i18ncmp/test_cmp/g search-replacement. I'm leaving t6300-for-each-ref.sh out due to a conflict with in-flight changes between "master" and "seen", as well as the prerequisite itself due to other changes between "master" and "next/seen" which add new test_i18ncmp uses. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-02-10tests: remove most uses of C_LOCALE_OUTPUTLibravatar Ævar Arnfjörð Bjarmason1-1/+1
As a follow-up to d162b25f956 (tests: remove support for GIT_TEST_GETTEXT_POISON, 2021-01-20) remove those uses of the now always true C_LOCALE_OUTPUT prerequisite from those tests which declare it as an argument to test_expect_{success,failure}. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2021-01-25Merge branch 'js/default-branch-name-tests-final-stretch'Libravatar Junio C Hamano1-2/+5
Prepare tests not to be affected by the name of the default branch "git init" creates. * js/default-branch-name-tests-final-stretch: (28 commits) tests: drop prereq `PREPARE_FOR_MAIN_BRANCH` where no longer needed t99*: adjust the references to the default branch name "main" tests(git-p4): transition to the default branch name `main` t9[5-7]*: adjust the references to the default branch name "main" t9[0-4]*: adjust the references to the default branch name "main" t8*: adjust the references to the default branch name "main" t7[5-9]*: adjust the references to the default branch name "main" t7[0-4]*: adjust the references to the default branch name "main" t6[4-9]*: adjust the references to the default branch name "main" t64*: preemptively adjust alignment to prepare for `master` -> `main` t6[0-3]*: adjust the references to the default branch name "main" t5[6-9]*: adjust the references to the default branch name "main" t55[4-9]*: adjust the references to the default branch name "main" t55[23]*: adjust the references to the default branch name "main" t551*: adjust the references to the default branch name "main" t550*: adjust the references to the default branch name "main" t5503: prepare aligned comment for replacing `master` with `main` t5[0-4]*: adjust the references to the default branch name "main" t5323: prepare centered comment for `master` -> `main` t4*: adjust the references to the default branch name "main" ...
2021-01-25Merge branch 'sj/untracked-files-in-submodule-directory-is-not-dirty'Libravatar Junio C Hamano1-0/+6
"git diff" showed a submodule working tree with untracked cruft as "Submodule commit <objectname>-dirty", but a natural expectation is that the "-dirty" indicator would align with "git describe --dirty", which does not consider having untracked files in the working tree as source of dirtiness. The inconsistency has been fixed. * sj/untracked-files-in-submodule-directory-is-not-dirty: diff: do not show submodule with untracked files as "-dirty"
2020-12-08diff: do not show submodule with untracked files as "-dirty"Libravatar Sangeeta Jain1-0/+6
Git diff reports a submodule directory as -dirty even when there are only untracked files in the submodule directory. This is inconsistent with what `git describe --dirty` says when run in the submodule directory in that state. Make `--ignore-submodules=untracked` the default for `git diff` when there is no configuration variable or command line option, so that the command would not give '-dirty' suffix to a submodule whose working tree has untracked files, to make it consistent with `git describe --dirty` that is run in the submodule working tree. And also make `--ignore-submodules=none` the default for `git status` so that the user doesn't end up deleting a submodule that has uncommitted (untracked) files. Signed-off-by: Sangeeta Jain <sangunb09@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19t3[5-9]*: adjust the references to the default branch name "main"Libravatar Johannes Schindelin1-3/+3
This trick was performed via $ (cd t && sed -i -e 's/master/main/g' -e 's/MASTER/MAIN/g' \ -e 's/Master/Main/g' -- t3[5-9]*.sh) This allows us to define `GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main` for those tests. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-19tests: mark tests relying on the current default for `init.defaultBranch`Libravatar Johannes Schindelin1-0/+3
In addition to the manual adjustment to let the `linux-gcc` CI job run the test suite with `master` and then with `main`, this patch makes sure that GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME is set in all test scripts that currently rely on the initial branch name being `master by default. To determine which test scripts to mark up, the first step was to force-set the default branch name to `master` in - all test scripts that contain the keyword `master`, - t4211, which expects `t/t4211/history.export` with a hard-coded ref to initialize the default branch, - t5560 because it sources `t/t556x_common` which uses `master`, - t8002 and t8012 because both source `t/annotate-tests.sh` which also uses `master`) This trick was performed by this command: $ sed -i '/^ *\. \.\/\(test-lib\|lib-\(bash\|cvs\|git-svn\)\|gitweb-lib\)\.sh$/i\ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\ ' $(git grep -l master t/t[0-9]*.sh) \ t/t4211*.sh t/t5560*.sh t/t8002*.sh t/t8012*.sh After that, careful, manual inspection revealed that some of the test scripts containing the needle `master` do not actually rely on a specific default branch name: either they mention `master` only in a comment, or they initialize that branch specificially, or they do not actually refer to the current default branch. Therefore, the aforementioned modification was undone in those test scripts thusly: $ git checkout HEAD -- \ t/t0027-auto-crlf.sh t/t0060-path-utils.sh \ t/t1011-read-tree-sparse-checkout.sh \ t/t1305-config-include.sh t/t1309-early-config.sh \ t/t1402-check-ref-format.sh t/t1450-fsck.sh \ t/t2024-checkout-dwim.sh \ t/t2106-update-index-assume-unchanged.sh \ t/t3040-subprojects-basic.sh t/t3301-notes.sh \ t/t3308-notes-merge.sh t/t3423-rebase-reword.sh \ t/t3436-rebase-more-options.sh \ t/t4015-diff-whitespace.sh t/t4257-am-interactive.sh \ t/t5323-pack-redundant.sh t/t5401-update-hooks.sh \ t/t5511-refspec.sh t/t5526-fetch-submodules.sh \ t/t5529-push-errors.sh t/t5530-upload-pack-error.sh \ t/t5548-push-porcelain.sh \ t/t5552-skipping-fetch-negotiator.sh \ t/t5572-pull-submodule.sh t/t5608-clone-2gb.sh \ t/t5614-clone-submodules-shallow.sh \ t/t7508-status.sh t/t7606-merge-custom.sh \ t/t9302-fast-import-unpack-limit.sh We excluded one set of test scripts in these commands, though: the range of `git p4` tests. The reason? `git p4` stores the (foreign) remote branch in the branch called `p4/master`, which is obviously not the default branch. Manual analysis revealed that only five of these tests actually require a specific default branch name to pass; They were modified thusly: $ sed -i '/^ *\. \.\/lib-git-p4\.sh$/i\ GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=master\ export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME\ ' t/t980[0167]*.sh t/t9811*.sh Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-16add -i: verify in the tests that colors can be overriddenLibravatar Johannes Schindelin1-0/+84
Now that the Perl version produces the same output as the built-in version (mostly fixing bugs in the latter), let's add a regression test to verify that it stays this way. Note that we only `grep` for the colored error message instead of verifying that the entire `stderr` consists of just this one line: when running the test script using the `-x` option to trace the commands, the sub-shell in `force_color` causes those commands to be traced into `err.raw` (unless running in Bash where we set the `BASH_XTRACEFD` variable to avoid that). Also note that the color reset in the `<BLUE>+<RESET><BLUE>new<RESET>` line might look funny and unnecessary, as the corresponding `old` line does not reset the color after the diff marker only to turn the color back on right away. However, this is a (necessary) side effect of the white-space check: in `emit_line_ws_markup()`, we first emit the diff marker via `emit_line_0()` and then the rest of the line via `ws_check_emit()`. To leave them somewhat decoupled, the color has to be reset after the diff marker to allow for the rest of the line to start with another color (or inverted, in case of white-space issues). Finally, we have to simulate hunk editing: the `git add -p` command cannot rely on the internal diff machinery for coloring after letting the user edit a hunk; It has to "re-color" the edited hunk. This is the primary reason why that command is interested in the exact values of the `color.diff.*` settings in the first place. To test this re-coloring, we therefore have to pretend to edit a hunk and then show that hunk in the regression test. Co-authored-by: Jeff King <peff@peff.net> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-22Merge branch 'pw/add-p-edit-ita-path'Libravatar Junio C Hamano1-0/+38
"add -p" now allows editing paths that were only added in intent. * pw/add-p-edit-ita-path: add -p: fix editing of intent-to-add paths
2020-09-18Merge branch 'jk/add-i-fixes'Libravatar Junio C Hamano1-0/+8
"add -i/-p" fixes. * jk/add-i-fixes: add--interactive.perl: specify --no-color explicitly add-patch: fix inverted return code of repo_read_index()
2020-09-09add -p: fix editing of intent-to-add pathsLibravatar Phillip Wood1-0/+38
A popular way of partially staging a new file is to run `git add -N <path>` and then use the hunk editing of `git add -p` to select the part of the file that the user wishes to stage. Since 85953a3187 ("diff-files --raw: show correct post-image of intent-to-add files", 2020-07-01) this has stopped working as intent-to-add paths are now show as new files rather than changes to an empty blob and `git apply` refused to apply a creation patch for a path that was marked as intent-to-add. 7cfde3fa0f ("apply: allow "new file" patches on i-t-a entries", 2020-08-06) fixed the problem with apply but it still wasn't possible to edit the added hunk properly. 2c8bd8471a ("checkout -p: handle new files correctly", 2020-05-27) had previously changed `add -p` to handle new files but it did not implement patch editing correctly. The perl version simply forbade editing and the C version opened the editor with the full diff rather that just the hunk which meant that the user had to edit the hunk header manually to get it to work. The root cause of the problem is that added files store the diff header with the hunk data rather than separating the two as we do for other changes. Changing added files to store the diff header separately fixes the editing problem at the expense of having to special case empty additions as they no longer have any hunks associated with them, only the diff header. The changes move some existing code into a conditional changing the indentation, they are best viewed with --color-moved-ws=allow-indentation-change (or --ignore-space-change works well to get an overview of the changes) Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Reported-by: Thomas Sullivan <tom@msbit.com.au> Reported-by: Yuchen Ying <ych@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-08add--interactive.perl: specify --no-color explicitlyLibravatar Jeff King1-0/+1
Our color tests of "git add -p" do something a bit different from how a normal user would behave: we pretend there's a pager in use, so that Git thinks it's OK to write color to a non-tty stdout. This comes from 8539b46534 (t3701: avoid depending on the TTY prerequisite, 2019-12-06), which allows us to avoid a lot of complicated mock-tty code. However, those environment variables also make their way down to sub-processes of add--interactive, including the "diff-files" we run to generate the patches. As a result, it thinks it should output color, too. So in t3701.50, for example, the machine-readable version of the diff we get unexpectedly has color in it. We fail to parse it as a diff and think there are zero hunks. The test does still pass, though, because even with zero hunks we'll dump the diff header (and we consider those unparseable bits to be part of the header!), and so the output still has the expected color codes in it. We don't notice that the command was totally broken and failed to apply anything. And in fact we're not really testing what we think we are about the color, either. While add--interactive does correctly show the version we got from running "diff-files --color", we'd also pass the test if we had accidentally shown the machine-readable version, too, since it (erroneously) has color codes in it. One could argue that the test isn't very realistic; it's setting up this "pretend there's a pager" situation to get around the tty restrictions of the test environment. So one option would be to move back towards using a real tty. But the behavior of add--interactive really is user-visible here. If a user, for whatever reason, did run "git --paginate add --patch" (perhaps because their pager is really a filter or something), the command would totally fail to do anything useful. Since we know that we don't want color in this output, let's just make add--interactive more defensive, and say "--no-color" explicitly. It doesn't hurt anything in the common case, but it fixes this odd case and lets our test function properly again. Note that the C builtin run_add_p() already passes --no-color, so it doesn't need a similar fix. That will eventually replace this perl code anyway, but the test change here will be valuable for ensuring that. Signed-off-by: Jeff King <peff@peff.net> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-08add-patch: fix inverted return code of repo_read_index()Libravatar Jeff King1-0/+7
After applying hunks to a file with "add -p", the C patch_update_file() function tries to refresh the index (just like the perl version does). We can only refresh the index if we're able to read it in, so we first check the return value of repo_read_index(). But unlike many functions, where "0" is success, that function is documented to return the number of entries in the index. Hence we should be checking for success with a non-negative return value. Neither the tests nor any users seem to have noticed this, probably due to a combination of: - this affects only the C version, which is not yet the default - following it up with any porcelain command like "git diff" or "git commit" would refresh the index automatically. But you can see the problem by running the plumbing "git diff-files" immediately after "add -p" stages all hunks. Running the new test with GIT_TEST_ADD_I_USE_BUILTIN=1 fails without the matching code change. Signed-off-by: Jeff King <peff@peff.net> Acked-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-07-07t3701: stop using `env` in force_color()Libravatar Denton Liu1-2/+11
In a future patch, we plan on making the test_must_fail()-family of functions accept only git commands. Even though force_color() wraps an invocation of `env git`, test_must_fail() will not be able to figure this out since it will assume that force_color() is just some random function which is disallowed. Instead of using `env` in force_color() (which does not support shell functions), export the environment variables in a subshell. Write the invocation as `force_color test_must_fail git ...` since shell functions are now supported. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-06-08Merge branch 'js/checkout-p-new-file'Libravatar Junio C Hamano1-0/+19
"git checkout -p" did not handle a newly added path at all. * js/checkout-p-new-file: checkout -p: handle new files correctly
2020-05-27checkout -p: handle new files correctlyLibravatar Johannes Schindelin1-0/+19
The original patch selection code was written for `git add -p`, and the fundamental unit on which it works is a hunk. We hacked around that to handle deletions back in 24ab81ae4d (add-interactive: handle deletion of empty files, 2009-10-27). But `git add -p` would never see a new file, since we only consider the set of tracked files in the index. However, since the same machinery was used for `git checkout -p` & friends, we can see new files. Handle this case specifically, adding a new prompt for it that is modeled after the `deleted file` case. This also fixes the problem where added _empty_ files could not be staged via `git checkout -p`. Reported-by: Merlin Büge <toni@bluenox07.de> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-03-22t: fix whitespace around &&Libravatar Andrei Rybak1-1/+1
Add missing spaces before '&&' and switch tabs around '&&' to spaces. Also fix the space after redirection operator in t3701 while we're here. These issues were found using `git grep '[^ ]&&$'` and `git grep -P '&&\t' t/`. Signed-off-by: Andrei Rybak <rybak.a.v@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-02-14Merge branch 'jk/diff-honor-wserrhighlight-in-plumbing'Libravatar Junio C Hamano1-0/+13
The diff-* plumbing family of subcommands now pay attention to the diff.wsErrorHighlight configuration, which has been ignored before; this allows "git add -p" to also show the whitespace problems to the end user. * jk/diff-honor-wserrhighlight-in-plumbing: diff: move diff.wsErrorHighlight to "basic" config
2020-02-05Merge branch 'js/add-p-leftover-bits'Libravatar Junio C Hamano1-1/+1
The final leg of rewriting "add -i/-p" in C. * js/add-p-leftover-bits: ci: include the built-in `git add -i` in the `linux-gcc` job built-in add -p: handle Escape sequences more efficiently built-in add -p: handle Escape sequences in interactive.singlekey mode built-in add -p: respect the `interactive.singlekey` config setting terminal: add a new function to read a single keystroke terminal: accommodate Git for Windows' default terminal terminal: make the code of disable_echo() reusable built-in add -p: handle diff.algorithm built-in add -p: support interactive.diffFilter t3701: adjust difffilter test
2020-01-31diff: move diff.wsErrorHighlight to "basic" configLibravatar Jeff King1-0/+13
We parse diff.wsErrorHighlight in git_diff_ui_config(), meaning that it doesn't take effect for plumbing commands, only for porcelains like git-diff itself. This is mildly annoying as it means scripts like add--interactive, which produce a user-visible diff with color, don't respect the option. We could teach that script to parse the config and pass it along as --ws-error-highlight to the diff plumbing. But there's a simpler solution. It should be reasonably safe for plumbing to respect this option, as it only kicks in when color is otherwise enabled. And anybody parsing colorized output must already deal with the fact that color.diff.* may change the exact output they see; those options have been part of git_diff_basic_config() since its inception in 9a1805a872 (add a "basic" diff config callback, 2008-01-04). So we can just move it to the "basic" config, which fixes add--interactive, along with any other script in the same boat, with a very low risk of hurting any plumbing users. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-30Merge branch 'js/builtin-add-i-cmds'Libravatar Junio C Hamano1-0/+9
Minor bugfixes to "git add -i" that has recently been rewritten in C. * js/builtin-add-i-cmds: built-in add -i: accept open-ended ranges again built-in add -i: do not try to `patch`/`diff` an empty list of files
2020-01-16built-in add -i: accept open-ended ranges againLibravatar Johannes Schindelin1-0/+9
The interactive `add` command allows selecting multiple files for some of its sub-commands, via unique prefixes, indices or index ranges. When re-implementing `git add -i` in C, we even added a code comment talking about ranges with a missing end index, such as `2-`, but the code did not actually accept those, as pointed out in https://github.com/git-for-windows/git/issues/2466#issuecomment-574142760. Let's fix this, and add a test case to verify that this stays fixed forever. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-01-15t3701: adjust difffilter testLibravatar Johannes Schindelin1-1/+1
In 42f7d45428e (add--interactive: detect bogus diffFilter output, 2018-03-03), we added a test case that verifies that the diffFilter feature complains appropriately when the output is too short. In preparation for the upcoming change where the built-in `add -p` is taught to respect that setting, let's adjust that test a little. The problem is that `echo too-short` is configured as diffFilter, and it does not read the `stdin`. When calling it through `pipe_command()`, it is therefore possible that we try to feed the `diff` to it while it is no longer listening, and we receive a `SIGPIPE`. The Perl code apparently handles this in a way similar to an end-of-file, but taking a step back, we realize that a diffFilter that does not even _look_ at its standard input is very unrealistic. The entire point of this feature is to transform the diff, not to ignore it altogether. So let's modify the test case to reflect that insight: instead of printing some bogus text, let's use a diffFilter that deletes the first line of the diff instead. This still tests for the same thing, but it does not confuse the built-in `add -p` with that `SIGPIPE`. Helped-by: SZEDER Gábor <szeder.dev@gmail.com> Helped-by: Jeff King <peff@peff.net> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-13built-in add -p: implement the '/' ("search regex") commandLibravatar Johannes Schindelin1-0/+14
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-13built-in add -p: implement the 'g' ("goto") commandLibravatar Johannes Schindelin1-0/+16
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-13built-in add -p: implement the hunk splitting featureLibravatar Johannes Schindelin1-0/+12
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06git add -p: use non-zero exit code when the diff generation failedLibravatar Johannes Schindelin1-1/+1
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06t3701: verify that the diff.algorithm config setting is handledLibravatar Johannes Schindelin1-0/+10
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06t3701: verify the shown messages when nothing can be addedLibravatar Johannes Schindelin1-0/+11
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06t3701: add a test for the different `add -p` promptsLibravatar Johannes Schindelin1-1/+18
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06t3701: avoid depending on the TTY prerequisiteLibravatar Johannes Schindelin1-7/+21
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-12-06t3701: add a test for advanced split-hunk editingLibravatar Johannes Schindelin1-0/+22
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 <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-11-18built-in add -i: implement the `help` commandLibravatar Slavica Đukić1-0/+25
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ć <slawica92@hotmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-10-04add -i: show progress counter in the promptLibravatar Kunal Tyagi1-1/+1
Report the current hunk count and total number of hunks for the current file in the prompt. Also adjust the expected output in some tests to match. Signed-off-by: Kunal Tyagi <tyagi.kunal@live.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2019-07-09Merge branch 'pw/add-p-recount'Libravatar Junio C Hamano1-0/+8
"git checkout -p" needs to selectively apply a patch in reverse, which did not work well. * pw/add-p-recount: add -p: fix checkout -p with pathological context
2019-06-13add -p: fix checkout -p with pathological contextLibravatar Phillip Wood1-0/+8
Commit fecc6f3a68 ("add -p: adjust offsets of subsequent hunks when one is skipped", 2018-03-01) fixed adding hunks in the correct place when a previous hunk has been skipped. However it did not address patches that are applied in reverse. In that case we need to adjust the pre-image offset so that when apply reverses the patch the post-image offset is adjusted correctly. We subtract rather than add the delta as the patch is reversed (the easiest way to think about it is to consider a hunk of deletions that is skipped - in that case we want to reduce offset so we need to subtract). Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-09-24Merge branch 'sg/t3701-tighten-trace'Libravatar Junio C Hamano1-1/+1
Test update. * sg/t3701-tighten-trace: t3701-add-interactive: tighten the check of trace output
2018-09-11t3701-add-interactive: tighten the check of trace outputLibravatar SZEDER Gábor1-1/+1
The test 'add -p does not expand argument lists' in 't3701-add-interactive.sh', added in 7288e12cce (add--interactive: do not expand pathspecs with ls-files, 2017-03-14), checks the GIT_TRACE of 'git add -p' to ensure that the name of a tracked file wasn't passed around as argument to any of the commands executed as a result of undesired pathspec expansion. This check is done with 'grep' using the filename on its own as the pattern, which is too loose a pattern, and would match any occurrences of the filename in the trace output, not just those as command arguments. E.g. if a developer were to litter the index handling code with trace_printf()s printing, among other things, the name of the just processed cache entry, then that pattern would mistakenly match these as well, and would fail the test. Tighten this 'grep' pattern to only match trace lines that show the executed commands. Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-07-03t: use test_write_lines() instead of series of 'echo' commandsLibravatar Eric Sunshine1-8/+8
These tests employ a noisy subshell (with missing &&-chain) to feed input into Git commands or files: (echo a; echo b; echo c) | git some-command ... Simplify by taking advantage of test_write_lines(): test_write_lines a b c | git some-command ... Signed-off-by: Eric Sunshine <sunshine@sunshineco.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-28Merge branch 'pw/add-p-recount'Libravatar Junio C Hamano1-0/+43
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
2018-06-11add -p: fix counting empty context lines in edited patchesLibravatar Phillip Wood1-0/+43
recount_edited_hunk() introduced in commit 2b8ea7f3c7 ("add -p: calculate offset delta for edited patches", 2018-03-05) required all context lines to start with a space, empty lines are not counted. This was intended to avoid any recounting problems if the user had introduced empty lines at the end when editing the patch. However this introduced a regression into 'git add -p' as it seems it is common for editors to strip the trailing whitespace from empty context lines when patches are edited thereby introducing empty lines that should be counted. 'git apply' knows how to deal with such empty lines and POSIX states that whether or not there is an space on an empty context line is implementation defined [1]. Fix the regression by counting lines that consist solely of a newline as well as lines starting with a space as context lines and add a test to prevent future regressions. [1] http://pubs.opengroup.org/onlinepubs/9699919799/utilities/diff.html Reported-by: Mahmoud Al-Qudsi <mqudsi@neosmart.net> Reported-by: Oliver Joseph Ash <oliverjash@gmail.com> Reported-by: Jeff Felchner <jfelchner1@gmail.com> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-14Merge branch 'jk/add-i-diff-filter'Libravatar Junio C Hamano1-0/+20
The "interactive.diffFilter" used by "git add -i" must retain one-to-one correspondence between its input and output, but it was not enforced and caused end-user confusion. We now at least make sure the filtered result has the same number of lines as its input to detect a broken filter. * jk/add-i-diff-filter: add--interactive: detect bogus diffFilter output t3701: add a test for interactive.diffFilter
2018-03-14Merge branch 'pw/add-p-recount'Libravatar Junio C Hamano1-130/+165
"git add -p" has been lazy in coalescing split patches before passing the result to underlying "git apply", leading to corner case bugs; the logic to prepare the patch to be applied after hunk selections has been tightened. * pw/add-p-recount: add -p: don't rely on apply's '--recount' option add -p: fix counting when splitting and coalescing add -p: calculate offset delta for edited patches add -p: adjust offsets of subsequent hunks when one is skipped t3701: add failing test for pathological context lines t3701: don't hard code sha1 hash values t3701: use test_write_lines and write_script t3701: indent here documents add -i: add function to format hunk header
2018-03-05add--interactive: detect bogus diffFilter outputLibravatar Jeff King1-0/+8
It's important that the diff-filter only filter the individual lines, and that there remain a one-to-one mapping between the input and output lines. Otherwise, things like hunk-splitting will behave quite unexpectedly (e.g., you think you are splitting at one point, but it has a different effect in the text patch we apply). We can't detect all problematic cases, but we can at least catch the obvious case where we don't even have the correct number of lines. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-05t3701: add a test for interactive.diffFilterLibravatar Jeff King1-0/+12
This feature was added in 01143847db (add--interactive: allow custom diff highlighting programs, 2016-02-27) but never tested. Let's add a basic test. Note that we only apply the filter when color is enabled, so we have to use test_terminal. This is an open limitation explicitly mentioned in the original commit. So take this commit as testing the status quo, and not making a statement on whether we'd want to enhance that in the future. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-05add -p: fix counting when splitting and coalescingLibravatar Phillip Wood1-8/+23
When a file has no trailing new line at the end diff records this by appending "\ No newline at end of file" below the last line of the file. This line should not be counted in the hunk header. Fix the splitting and coalescing code to count files without a trailing new line properly and change one of the tests to test splitting without a trailing new line. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-05add -p: calculate offset delta for edited patchesLibravatar Phillip Wood1-1/+1
Recount the number of preimage and postimage lines in a hunk after it has been edited so any change in the number of insertions or deletions can be used to adjust the offsets of subsequent hunks. If an edited hunk is subsequently split then the offset correction will be lost. It would be possible to fix this if it is a problem, however the code here is still an improvement on the status quo for the common case where an edited hunk is applied without being split. This is also a necessary step to removing '--recount' and '--allow-overlap' from the invocation of 'git apply'. Before '--recount' can be removed the splitting and coalescing counting needs to be fixed to handle a missing newline at the end of a file. In order to remove '--allow-overlap' there needs to be i) some way of verifying the offset data in the edited hunk (probably by correlating the preimage (or postimage if the patch is going to be applied in reverse) lines of the edited and unedited versions to see if they are offset or if any leading/trailing context lines have been removed) and ii) a way of dealing with edited hunks that change context lines that are shared with neighbouring hunks. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-01add -p: adjust offsets of subsequent hunks when one is skippedLibravatar Phillip Wood1-1/+1
Since commit 8cbd431082 ("git-add--interactive: replace hunk recounting with apply --recount", 2008-7-2) if a hunk is skipped then we rely on the context lines to apply subsequent hunks in the right place. While this works most of the time it is possible for hunks to end up being applied in the wrong place. To fix this adjust the offset of subsequent hunks to correct for any change in the number of insertions or deletions due to the skipped hunk. The change in offset due to edited hunks that have the number of insertions or deletions changed is ignored here, it will be fixed in the next commit. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>