summaryrefslogtreecommitdiff
path: root/git-add--interactive.perl
AgeCommit message (Collapse)AuthorFilesLines
2020-12-08Merge branch 'js/add-i-color-fix'Libravatar Junio C Hamano1-7/+5
"git add -i" failed to honor custom colors configured to show patches, which has been corrected. * js/add-i-color-fix: add -i: verify in the tests that colors can be overridden add -p: prefer color.diff.context over color.diff.plain add -i (Perl version): color header to match the C version add -i (built-in): use the same indentation as the Perl version add -p (built-in): do not color the progress indicator separately add -i (built-in): use correct names to load color.diff.* config add -i (built-in): prevent the `reset` "color" from being configured add -i: use `reset_color` consistently add -p (built-in): imitate `xdl_format_hunk_hdr()` generating hunk headers add -i (built-in): send error messages to stderr add -i (built-in): do show an error message for incorrect inputs
2020-11-16add -p: prefer color.diff.context over color.diff.plainLibravatar Johannes Schindelin1-3/+3
Git's diff machinery allows users to override the colors to use in diffs, even the plain-colored context lines. As of 8dbf3eb6850 (diff.h: rename DIFF_PLAIN color slot to DIFF_CONTEXT, 2015-05-27), the preferred name of the config setting is `color.diff.context`, although Git still allows `color.diff.plain`. In the context of `git add -p`, this logic is a bit hard to replicate: `git_diff_basic_config()` reads all config values sequentially and if it sees _any_ `color.diff.context` or `color.diff.plain`, it accepts the new color. The Perl version of `git add -p` needs to go through `git config --get-color`, though, which allows only one key to be specified. The same goes for the built-in version of `git add -p`, which has to go through `repo_config_get_value()`. The best we can do here is to look for `.context` and if none is found, fall back to looking for `.plain`, and if still not found, fall back to the hard-coded default (which in this case is simply the empty string, as context lines are typically rendered without colored). This still leads to inconsistencies when both config names are used: the initial diff will be colored by the diff machinery. Once edited by a user, a hunk has to be re-colored by `git add -p`, though, which would then use the other setting to color the context lines. In practice, this is not _all_ that bad. The `git config` manual says this in the `color.diff.<slot>`: `context` (context text - `plain` is a historical synonym) We should therefore assume that users use either one or the other, but not both names. Besides, it is relatively uncommon to look at a hunk after editing it because it is immediately staged by default. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-11-16add -i (Perl version): color header to match the C versionLibravatar Johannes Schindelin1-4/+2
Both versions of `add -i` indent non-flat lists by five spaces. However when using color the C version prints these spaces after the ANSI color codes whereas the Perl version prints them before the color codes. Change the Perl version to match the C version to allow for introducing a test that verifies that both versions produce the exact same output. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-10-27Merge branch 'dl/checkout-p-merge-base'Libravatar Junio C Hamano1-0/+7
"git checkout -p A...B [-- <path>]" did not work, even though the same command without "-p" correctly used the merge-base between commits A and B. * dl/checkout-p-merge-base: t2016: add a NEEDSWORK about the PERL prerequisite add-patch: add NEEDSWORK about comparing commits Doc: document "A...B" form for <tree-ish> in checkout and switch builtin/checkout: fix `git checkout -p HEAD...` bug
2020-10-07add-patch: add NEEDSWORK about comparing commitsLibravatar Denton Liu1-0/+7
The two versions of add-patch has special-casing for the literal revision "HEAD". However, we want to handle other ways of saying "HEAD" in the same way.[0] Add a NEEDSWORK to the add-patch code that does this so that it can be addressed later. [0]: https://lore.kernel.org/git/xmqqsgat7ttf.fsf@gitster.c.googlers.com/ Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2020-09-22Merge branch 'pw/add-p-edit-ita-path'Libravatar Junio C Hamano1-33/+56
"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-09add -p: fix editing of intent-to-add pathsLibravatar Phillip Wood1-33/+56
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-1/+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-05-27checkout -p: handle new files correctlyLibravatar Johannes Schindelin1-2/+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>
2019-12-06git add -p: use non-zero exit code when the diff generation failedLibravatar Johannes Schindelin1-3/+5
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-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 'nd/switch-and-restore'Libravatar Junio C Hamano1-0/+52
Two new commands "git switch" and "git restore" are introduced to split "checking out a branch to work on advancing its history" and "checking out paths out of the index and/or a tree-ish to work on advancing the current history" out of the single "git checkout" command. * nd/switch-and-restore: (46 commits) completion: disable dwim on "git switch -d" switch: allow to switch in the middle of bisect t2027: use test_must_be_empty Declare both git-switch and git-restore experimental help: move git-diff and git-reset to different groups doc: promote "git restore" user-manual.txt: prefer 'merge --abort' over 'reset --hard' completion: support restore t: add tests for restore restore: support --patch restore: replace --force with --ignore-unmerged restore: default to --source=HEAD when only --staged is specified restore: reject invalid combinations with --staged restore: add --worktree and --staged checkout: factor out worktree checkout code restore: disable overlay mode by default restore: make pathspec mandatory restore: take tree-ish from --source option instead checkout: split part of it to new command 'restore' doc: promote "git switch" ...
2019-07-09Merge branch 'pw/add-p-recount'Libravatar Junio C Hamano1-1/+5
"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-1/+5
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>
2019-05-07restore: support --patchLibravatar Nguyễn Thái Ngọc Duy1-0/+52
git-restore is different from git-checkout that it only restores the worktree by default, not both worktree and index. add--interactive needs some update to support this mode. Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-06-28Merge branch 'pw/add-p-recount'Libravatar Junio C Hamano1-1/+1
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-1/+1
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-05-02add--interactive: compute the empty tree valueLibravatar brian m. carlson1-2/+9
The interactive add script hard-codes the object ID of the empty tree. To avoid any problems when changing hashes, compute this value when used and cache it for any future uses. Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-04-02Merge branch 'pw/add-p-single'Libravatar Junio C Hamano1-1/+1
Hotfix. * pw/add-p-single: add -p: fix 2.17.0-rc* regression due to moved code
2018-03-31add -p: fix 2.17.0-rc* regression due to moved codeLibravatar Ævar Arnfjörð Bjarmason1-1/+1
Fix a regression in 88f6ffc1c2 ("add -p: only bind search key if there's more than one hunk", 2018-02-13) which is present in 2.17.0-rc*, but not 2.16.0. In Perl, regex variables like $1 always refer to the last regex match. When the aforementioned change added a new regex match between the old match and the corresponding code that was expecting $1, the $1 variable would always be undef, since the newly inserted regex match doesn't have any captures. As a result the "/" feature to search for a string in a hunk by regex completely broke, on git.git: $ perl -pi -e 's/Git/Tig/g' README.md $ ./git --exec-path=$PWD add -p [..] Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,?]? s Split into 4 hunks. [...] Stage this hunk [y,n,q,a,d,j,J,g,/,s,e,?]? /Many Use of uninitialized value $1 in string eq at /home/avar/g/git/git-add--interactive line 1568, <STDIN> line 1. search for regex? Many I.e. the initial "/regex" command wouldn't work, and would always emit a warning and ask again for a regex, now it works as intended again. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-14Merge branch 'jk/add-i-diff-filter'Libravatar Junio C Hamano1-0/+8
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-22/+86
"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-05add -p: don't rely on apply's '--recount' optionLibravatar Phillip Wood1-1/+1
Now that add -p counts patches properly it should be possible to turn off the '--recount' option when invoking 'git apply' Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-03-05add -p: fix counting when splitting and coalescingLibravatar Phillip Wood1-0/+11
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-9/+50
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-2/+13
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>
2018-02-20add -i: add function to format hunk headerLibravatar Phillip Wood1-10/+11
This code is duplicated in a couple of places so make it into a function as we're going to add some more callers shortly. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-13add -p: improve error messagesLibravatar Phillip Wood1-3/+15
If the user presses a key that isn't currently active then explain why it isn't active rather than just listing all the keys. It already did this for some keys, this patch does the same for the those that weren't already handled. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-13add -p: only bind search key if there's more than one hunkLibravatar Phillip Wood1-23/+27
If there is only a single hunk then disable searching as there is nothing to search for. Also print a specific error message if the user tries to search with '/' when there's only a single hunk rather than just listing the key bindings. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-02-13add -p: only display help for active keysLibravatar Phillip Wood1-1/+7
If the user presses a key that add -p wasn't expecting then it prints a list of key bindings. Although the prompt only lists the active bindings the help was printed for all bindings. Fix this by using the list of keys in the prompt to filter the help. Note that the list of keys was already passed to help_patch_cmd() by the caller so there is no change needed to the call site. Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2018-01-16add--interactive: ignore submodule changes except HEADLibravatar Nguyễn Thái Ngọc Duy1-1/+1
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>
2017-07-10Merge branch 'pw/unquote-path-in-git-pm'Libravatar Junio C Hamano1-42/+1
Code refactoring. * pw/unquote-path-in-git-pm: t9700: add tests for Git::unquote_path() Git::unquote_path(): throw an exception on bad path Git::unquote_path(): handle '\a' add -i: move unquote_path() to Git.pm
2017-06-30add -i: move unquote_path() to Git.pmLibravatar Phillip Wood1-42/+1
Move unquote_path() from git-add--interactive to Git.pm so it can be used by other scripts. Note this is a straight copy, it does not handle '\a'. That 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>
2017-06-26Merge branch 'jk/add-p-commentchar-fix'Libravatar Junio C Hamano1-1/+2
"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
2017-06-21add--interactive: quote commentChar regexLibravatar Jeff King1-1/+1
Since c9d961647 (i18n: add--interactive: mark edit_hunk_manually message for translation, 2016-12-14), when the user asks to edit a hunk manually, we respect core.commentChar in generating the edit instructions. However, when we then strip out comment lines, we use a simple regex like: /^$commentChar/ If your chosen comment character is a regex metacharacter, then that will behave in a confusing manner ("$", for instance, would only eliminate blank lines, not actual comment lines). We can fix that by telling perl not to respect metacharacters. Reported-by: Christian Rösch <christian@croesch.de> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-06-21add--interactive: handle EOF in prompt_yesnoLibravatar Jeff King1-0/+1
The prompt_yesno function loops indefinitely waiting for a "y" or "n" response. But it doesn't handle EOF, meaning that we can end up in an infinite loop of reading EOF from stdin. One way to simulate that is with: echo e | GIT_EDITOR='echo corrupt >' git add -p Let's break out of the loop and propagate the undef to the caller. Without modifying the callers that effectively turns it into a "no" response. This is reasonable for both of the current callers, and it leaves room for any future caller to check for undef explicitly. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-05-09add--interactive: drop diff.indentHeuristic handlingLibravatar Jeff King1-4/+0
Now that diff.indentHeuristic is handled automatically by the plumbing commands, there's no need to propagate it manually. Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Marc Branchaud <marcnarc@xiplink.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-04-19Merge branch 'va/i18n-perl-scripts'Libravatar Junio C Hamano1-1/+1
Message fix. * va/i18n-perl-scripts: git-add--interactive.perl: add missing dot in a message
2017-04-13git-add--interactive.perl: add missing dot in a messageLibravatar Ralf Thielow1-1/+1
One message appears twice in the translations and the only difference is a dot at the end. So add this dot to make the messages being identical. Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-17Merge branch 'jk/add-i-use-pathspecs'Libravatar Junio C Hamano1-11/+2
"git add -p <pathspec>" unnecessarily expanded the pathspec to a list of individual files that matches the pathspec by running "git ls-files <pathspec>", before feeding it to "git diff-index" to see which paths have changes, because historically the pathspec language supported by "diff-index" was weaker. These days they are equivalent and there is no reason to internally expand it. This helps both performance and avoids command line argument limit on some platforms. * jk/add-i-use-pathspecs: add--interactive: do not expand pathspecs with ls-files
2017-03-14add--interactive: do not expand pathspecs with ls-filesLibravatar Jeff King1-11/+2
When we want to get the list of modified files, we first expand any user-provided pathspecs with "ls-files", and then feed the resulting list of paths as arguments to "diff-index" and "diff-files". If your pathspec expands into a large number of paths, you may run into one of two problems: 1. The OS may complain about the size of the argument list, and refuse to run. For example: $ (ulimit -s 128 && git add -p drivers) Can't exec "git": Argument list too long at .../git-add--interactive line 177. Died at .../git-add--interactive line 177. That's on the linux.git repository, which has about 20K files in the "drivers" directory (none of them modified in this case). The "ulimit -s" trick is necessary to show the problem on Linux even for such a gigantic set of paths. Other operating systems have much smaller limits (e.g., a real-world case was seen with only 5K files on OS X). 2. Even when it does work, it's really slow. The pathspec code is not optimized for huge numbers of paths. Here's the same case without the ulimit: $ time git add -p drivers No changes. real 0m16.559s user 0m53.140s sys 0m0.220s We can improve this by skipping "ls-files" completely, and just feeding the original pathspecs to the diff commands. This solution was discussed in 2010: http://public-inbox.org/git/20100105041438.GB12574@coredump.intra.peff.net/ but at the time the diff code's pathspecs were more primitive than those used by ls-files (e.g., they did not support globs). Making the change would have caused a user-visible regression, so we didn't. Since then, the pathspec code has been unified, and the diff commands natively understand pathspecs like '*.c'. This patch implements that solution. That skips the argument-list limits, and the result runs much faster: $ time git add -p drivers No changes. real 0m0.149s user 0m0.116s sys 0m0.080s There are two new tests. The first just exercises the globbing behavior to confirm that we are not causing a regression there. The second checks the actual argument behavior using GIT_TRACE. We _could_ do it with the "ulimit -s" trick, as above. But that would mean the test could only run where "ulimit -s" works. And tests of that sort are expensive, because we have to come up with enough files to actually bust the limit (we can't just shrink the "128" down infinitely, since it is also the in-program stack size). Finally, two caveats and possibilities for future work: a. This fixes one argument-list expansion, but there may be others. In fact, it's very likely that if you run "git add -i" and select a large number of modified files that the script would try to feed them all to a single git command. In practice this is probably fine. The real issue here is that the argument list was growing with the _total_ number of files, not the number of modified or selected files. b. If the repository contains filenames with literal wildcard characters (e.g., "foo*"), the original code expanded them via "ls-files" and then fed those wildcard names to "diff-index", which would have treated them as wildcards. This was a bug, which is now fixed (though unless you really go through some contortions with ":(literal)", it's likely that your original pathspec would match whatever the accidentally-expanded wildcard would anyway). So this takes us one step closer to working correctly with files whose names contain wildcard characters, but it's likely that others remain (e.g., if "git add -i" feeds the selected paths to "git add"). Reported-by: Wincent Colaiuta <win@wincent.com> Reported-by: Mislav Marohnić <mislav.marohnic@gmail.com> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-03-02add--interactive: fix missing file prompt for patch mode with "-i"Libravatar Jeff King1-4/+4
When invoked as "git add -i", each menu interactive menu option prompts the user to select a list of files. This includes the "patch" option, which gets the list before starting the hunk-selection loop. As "git add -p", it behaves differently, and jumps straight to the hunk selection loop. Since 0539d5e6d (i18n: add--interactive: mark patch prompt for translation, 2016-12-14), the "add -i" case mistakenly jumps to straight to the hunk-selection loop. Prior to that commit the distinction between the two cases was managed by the $patch_mode variable. That commit used $patch_mode for something else, and moved the old meaning to the "$cmd" variable. But it forgot to update the $patch_mode check inside patch_update_cmd() which controls the file-list behavior. The simplest fix would be to change that line to check $cmd. But while we're here, let's use a less obscure name for this flag: $patch_mode_only, a boolean which tells whether we are in full-interactive mode or only in patch-mode. Reported-by: Henrik Grubbström <grubba@grubba.org> Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-02-24Merge branch 'rt/align-add-i-help-text'Libravatar Junio C Hamano1-1/+1
Doc update. * rt/align-add-i-help-text: git add -i: replace \t with blanks in the help message
2017-02-22git add -i: replace \t with blanks in the help messageLibravatar Ralf Thielow1-1/+1
Within the help message of 'git add -i', the 'diff' command uses one tab character and blanks to create the space between the name and the description while the others use blanks only. So if the tab size is not at 4 characters, this description will not be in range. Replace the tab character with blanks. Signed-off-by: Ralf Thielow <ralf.thielow@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2017-01-10Merge branch 'jc/retire-compaction-heuristics'Libravatar Junio C Hamano1-3/+0
"git diff" and its family had two experimental heuristics to shift the contents of a hunk to make the patch easier to read. One of them turns out to be better than the other, so leave only the "--indent-heuristic" option and remove the other one. * jc/retire-compaction-heuristics: diff: retire "compaction" heuristics
2016-12-27Merge branch 'va/i18n-perl-scripts'Libravatar Junio C Hamano1-114/+215
Porcelain scripts written in Perl are getting internationalized. * va/i18n-perl-scripts: i18n: difftool: mark warnings for translation i18n: send-email: mark composing message for translation i18n: send-email: mark string with interpolation for translation i18n: send-email: mark warnings and errors for translation i18n: send-email: mark strings for translation i18n: add--interactive: mark status words for translation i18n: add--interactive: remove %patch_modes entries i18n: add--interactive: mark edit_hunk_manually message for translation i18n: add--interactive: i18n of help_patch_cmd i18n: add--interactive: mark patch prompt for translation i18n: add--interactive: mark plural strings i18n: clean.c: match string with git-add--interactive.perl i18n: add--interactive: mark strings with interpolation for translation i18n: add--interactive: mark simple here-documents for translation i18n: add--interactive: mark strings for translation Git.pm: add subroutines for commenting lines
2016-12-23diff: retire "compaction" heuristicsLibravatar Junio C Hamano1-3/+0
When a patch inserts a block of lines, whose last lines are the same as the existing lines that appear before the inserted block, "git diff" can choose any place between these existing lines as the boundary between the pre-context and the added lines (adjusting the end of the inserted block as appropriate) to come up with variants of the same patch, and some variants are easier to read than others. We have been trying to improve the choice of this boundary, and Git 2.11 shipped with an experimental "compaction-heuristic". Since then another attempt to improve the logic further resulted in a new "indent-heuristic" logic. It is agreed that the latter gives better result overall, and the former outlived its usefulness. Retire "compaction", and keep "indent" as an experimental feature. The latter hopefully will be turned on by default in a future release, but that should be done as a separate step. Suggested-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-14i18n: add--interactive: mark status words for translationLibravatar Vasco Almeida1-6/+6
Mark words 'nothing', 'unchanged' and 'binary' used to display what has been staged or not, in "git add -i" status command. Alternatively one could mark N__('nothing') no-op in order to xgettext(1) extract the string and then trigger the translation at run time only with __($print->{FILE}), but that has the side effect of triggering retrieval of translations for the changes indicator too (e.g. +2/-1) which may or may not be a problem. To avoid that potential problem, mark only where there is certain to trigger translation only of those words but in this case we must also retrieve the translation for the eq tests, since the value assigned was of the translation, not the English source. Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>
2016-12-14i18n: add--interactive: remove %patch_modes entriesLibravatar Vasco Almeida1-21/+0
Remove unnecessary entries from %patch_modes. After the i18n conversion, these entries are not used anymore. Signed-off-by: Vasco Almeida <vascomalmeida@sapo.pt> Signed-off-by: Junio C Hamano <gitster@pobox.com>