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>
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>
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>
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>
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>
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>
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>
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>
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
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
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>
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
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>
In 42f7d45428 (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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
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>
"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
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>
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>
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>
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
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>
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
"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
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>
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>
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>
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>
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>
When a hunk is skipped by add -i the offsets of subsequent hunks are
not adjusted to account for any missing insertions due to the skipped
hunk. Most of the time this does not matter as apply uses the context
lines to apply the subsequent hunks in the correct place, however in
pathological cases the context lines will match at the now incorrect
offset and the hunk will be applied in the wrong place. The offsets of
hunks following an edited hunk that has had the number of insertions
or deletions changed also need to be updated in the same way. Add
failing tests to demonstrate this.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Use a filter when comparing diffs to fix the value of non-zero hashes
in diff index lines so we're not hard coding sha1 hash values in the
expected output. This makes it easier to change the expected output if
a test is edited as we don't need to worry about the exact hash value
and means the tests will work when the hash algorithm is transitioned
away from sha1.
Thanks-to: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Simplify things slightly by using the above helpers.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
Indent here documents in line with the current style for tests.
While at it, quote the end marker of here-docs that do not use
variable interpolation.
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
For 'add -i' and 'add -p', the only action we can take on a dirty
submodule entry is update the index with a new value from its HEAD. The
content changes inside (from its own index, untracked files...) do not
matter, at least until 'git add -i' learns about launching a new
interactive add session inside a submodule.
Ignore all other submodules changes except HEAD. This reduces the number
of entries the user has to check through in 'git add -i', and the number
of 'no' they have to answer to 'git add -p' when dirty submodules are
present.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
It can be handy to use `--color=always` (or it's synonym
`--color`) on the command-line to convince a command to
produce color even if it's stdout isn't going to the
terminal or a pager.
What's less clear is whether it makes sense to set config
variables like color.ui to `always`. For a one-shot like:
git -c color.ui=always ...
it's potentially useful (especially if the command doesn't
directly support the `--color` option). But setting `always`
in your on-disk config is much muddier, as you may be
surprised when piped commands generate colors (and send them
to whatever is consuming the pipe downstream).
Some people have done this anyway, because:
1. The documentation for color.ui makes it sound like
using `always` is a good idea, when you almost
certainly want `auto`.
2. Traditionally not every command (and especially not
plumbing) respected color.ui in the first place. So
the confusion came up less frequently than it might
have.
The situation changed in 136c8c8b8f (color: check color.ui
in git_default_config(), 2017-07-13), which negated point
(2): now scripts using only plumbing commands (like
add-interactive) are broken by this setting.
That commit was fixing real issues (e.g., by making
`color.ui=never` work, since `auto` is the default), so we
don't want to just revert it. We could turn `always` into a
noop in plumbing commands, but that creates a hard-to-explain
inconsistency between the plumbing and other commands.
Instead, let's just turn `always` into `auto` for all config.
This does break the "one-shot" config shown above, but again,
we're probably better to have simple and consistent rules than
to try to special-case command-line config.
There is one place where `always` should retain its meaning:
on the command line, `--color=always` should continue to be
the same as `--color`, overriding any isatty checks. Since the
command-line parser also depends on git_config_colorbool(), we
can use the existence of the "var" string to deterine whether
we are serving the command-line or the config.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
When testing whether "add -p" can generate colors, we set
color.ui to "always". This isn't a very good test, as in the
real-world a user typically has "auto" coupled with stdout
going to a terminal (and it's plausible that this could mask
a real bug in add--interactive if we depend on plumbing's
isatty check).
Let's switch to test_terminal, which gives us a more
realistic environment. This also prepare us for future
changes to the "always" color option.
Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
"git add -p" were updated in 2.12 timeframe to cope with custom
core.commentchar but the implementation was buggy and a
metacharacter like $ and * did not work.
* jk/add-p-commentchar-fix:
add--interactive: quote commentChar regex
add--interactive: handle EOF in prompt_yesno
Johannes Schindelin
Junio C Hamano
Phillip Wood
Jeff King
Denton Liu
Andrei Rybak
Slavica Đukić
Kunal Tyagi
SZEDER Gábor
Eric Sunshine
Nguyễn Thái Ngọc Duy