git/git-add--interactive.perl, branch v2.14.0

Merge branch 'pw/unquote-path-in-git-pm'

2017-07-10T20:42:50Z

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

add -i: move unquote_path() to Git.pm

2017-06-30T15:02:20Z

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 Signed-off-by: Junio C Hamano

Merge branch 'jk/add-p-commentchar-fix'

2017-06-26T21:09:31Z

"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

add--interactive: quote commentChar regex

2017-06-21T21:06:20Z

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 Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

add--interactive: handle EOF in prompt_yesno

2017-06-21T21:06:09Z

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 Signed-off-by: Junio C Hamano

add--interactive: drop diff.indentHeuristic handling

2017-05-09T03:24:35Z

Now that diff.indentHeuristic is handled automatically by the plumbing commands, there's no need to propagate it manually. Signed-off-by: Jeff King Signed-off-by: Marc Branchaud Signed-off-by: Junio C Hamano

Merge branch 'va/i18n-perl-scripts'

2017-04-20T04:37:17Z

Message fix. * va/i18n-perl-scripts: git-add--interactive.perl: add missing dot in a message

git-add--interactive.perl: add missing dot in a message

2017-04-14T01:01:15Z

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 Signed-off-by: Junio C Hamano

Merge branch 'jk/add-i-use-pathspecs'

2017-03-17T20:50:26Z

"git add -p " unnecessarily expanded the pathspec to a list of individual files that matches the pathspec by running "git ls-files ", 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

add--interactive: do not expand pathspecs with ls-files

2017-03-14T20:27:23Z

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 Reported-by: Mislav Marohnić Signed-off-by: Jeff King Signed-off-by: Junio C Hamano