path: root/git-add--interactive.perl
AgeCommit message (Collapse)Author
2019-12-06git add -p: use non-zero exit code when the diff generation failedJohannes Schindelin
The first thing `git add -p` does is to generate a diff. If this diff cannot be generated, `git add -p` should not continue as if nothing happened, but instead fail. What we *actually* do here is much broader: we now verify for *every* `run_cmd_pipe()` call that the spawned process actually succeeded. Note that we have to change two callers in this patch, as we need to store the spawned process' output in a local variable, which means that the callers can no longer decide whether to interpret the `return <$fh>` in array or in scalar context. This bug was noticed while writing a test case for the diff.algorithm feature, and we let that test case double as a regression test for this fixed bug, too. Signed-off-by: Johannes Schindelin <> Signed-off-by: Junio C Hamano <>
2019-10-03add -i: show progress counter in the promptKunal Tyagi
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 <> Signed-off-by: Junio C Hamano <>
2019-07-09Merge branch 'nd/switch-and-restore'Junio C Hamano
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'Junio C Hamano
"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 contextPhillip Wood
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 <> Signed-off-by: Junio C Hamano <>
2019-05-07restore: support --patchNguyễn Thái Ngọc Duy
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 <> Signed-off-by: Junio C Hamano <>
2018-06-28Merge branch 'pw/add-p-recount'Junio C Hamano
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 patchesPhillip Wood
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] Reported-by: Mahmoud Al-Qudsi <> Reported-by: Oliver Joseph Ash <> Reported-by: Jeff Felchner <> Signed-off-by: Phillip Wood <> Signed-off-by: Junio C Hamano <>
2018-05-02add--interactive: compute the empty tree valuebrian m. carlson
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 <> Signed-off-by: Junio C Hamano <>
2018-04-02Merge branch 'pw/add-p-single'Junio C Hamano
Hotfix. * pw/add-p-single: add -p: fix 2.17.0-rc* regression due to moved code
2018-04-01add -p: fix 2.17.0-rc* regression due to moved codeÆvar Arnfjörð Bjarmason
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' $ ./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 <> Signed-off-by: Junio C Hamano <>
2018-03-14Merge branch 'jk/add-i-diff-filter'Junio C Hamano
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'Junio C Hamano
"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 outputJeff King
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 <> Signed-off-by: Junio C Hamano <>
2018-03-05add -p: don't rely on apply's '--recount' optionPhillip Wood
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 <> Signed-off-by: Junio C Hamano <>
2018-03-05add -p: fix counting when splitting and coalescingPhillip Wood
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 <> Signed-off-by: Junio C Hamano <>
2018-03-05add -p: calculate offset delta for edited patchesPhillip Wood
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 <> Signed-off-by: Junio C Hamano <>
2018-03-01add -p: adjust offsets of subsequent hunks when one is skippedPhillip Wood
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 <> Signed-off-by: Junio C Hamano <>
2018-02-20add -i: add function to format hunk headerPhillip Wood
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 <> Signed-off-by: Junio C Hamano <>
2018-02-13add -p: improve error messagesPhillip Wood
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 <> Signed-off-by: Junio C Hamano <>
2018-02-13add -p: only bind search key if there's more than one hunkPhillip Wood
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 <> Signed-off-by: Junio C Hamano <>
2018-02-13add -p: only display help for active keysPhillip Wood
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 <> Signed-off-by: Junio C Hamano <>
2018-01-16add--interactive: ignore submodule changes except HEADNguyễn Thái Ngọc Duy
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 <> Signed-off-by: Junio C Hamano <>
2017-07-10Merge branch 'pw/unquote-path-in-git-pm'Junio C Hamano
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
2017-06-30add -i: move unquote_path() to Git.pmPhillip Wood
Move unquote_path() from git-add--interactive to 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 <>
2017-06-26Merge branch 'jk/add-p-commentchar-fix'Junio C Hamano
"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 regexJeff King
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 <>
2017-06-21add--interactive: handle EOF in prompt_yesnoJeff King
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 <>
2017-05-09add--interactive: drop diff.indentHeuristic handlingJeff King
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 <>
2017-04-20Merge branch 'va/i18n-perl-scripts'Junio C Hamano
Message fix. * va/i18n-perl-scripts: git-add--interactive.perl: add missing dot in a message
2017-04-14git-add--interactive.perl: add missing dot in a messageRalf Thielow
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 <>
2017-03-17Merge branch 'jk/add-i-use-pathspecs'Junio C Hamano
"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-filesJeff King
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: 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 <>
2017-03-02add--interactive: fix missing file prompt for patch mode with "-i"Jeff King
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 <> Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>
2017-02-24Merge branch 'rt/align-add-i-help-text'Junio C Hamano
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 messageRalf Thielow
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 <> Signed-off-by: Junio C Hamano <>
2017-01-10Merge branch 'jc/retire-compaction-heuristics'Junio C Hamano
"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'Junio C Hamano
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 add subroutines for commenting lines
2016-12-23diff: retire "compaction" heuristicsJunio C Hamano
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 <> Signed-off-by: Junio C Hamano <>
2016-12-14i18n: add--interactive: mark status words for translationVasco Almeida
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 <> Signed-off-by: Junio C Hamano <>
2016-12-14i18n: add--interactive: remove %patch_modes entriesVasco Almeida
Remove unnecessary entries from %patch_modes. After the i18n conversion, these entries are not used anymore. Signed-off-by: Vasco Almeida <> Signed-off-by: Junio C Hamano <>
2016-12-14i18n: add--interactive: mark edit_hunk_manually message for translationVasco Almeida
Mark message of edit_hunk_manually displayed in the editing file when user chooses 'e' option. The message had to be unfolded to allow translation of the $participle verb. Some messages end up being exactly the same for some use cases, but left it for easier change in the future, e.g., wanting to change wording of one particular use case. The comment character is now used according to the git configuration core.commentchar. Signed-off-by: Vasco Almeida <> Signed-off-by: Junio C Hamano <>
2016-12-14i18n: add--interactive: i18n of help_patch_cmdVasco Almeida
Mark help message of help_patch_cmd for translation. The message must be unfolded to be free of variables so we can have high quality translations. Signed-off-by: Vasco Almeida <> Signed-off-by: Junio C Hamano <>
2016-12-14i18n: add--interactive: mark patch prompt for translationVasco Almeida
Mark prompt message assembled in place for translation, unfolding each use case for each entry in the %patch_modes hash table. Previously, this script relied on whether $patch_mode was set to run the command patch_update_cmd() or show status and loop the main loop. Now, it uses $cmd to indicate we must run patch_update_cmd() and $patch_mode is used to tell which flavor of the %patch_modes are we on. This is introduced in order to be able to mark and unfold the message prompt knowing in which context we are. The tracking of context was done previously by point %patch_mode_flavour hash table to the correct entry of %patch_modes, focusing only on value of %patch_modes. Now, we are also interested in the key ('staged', 'stash', 'checkout_head', ...). Signed-off-by: Vasco Almeida <> Signed-off-by: Junio C Hamano <>
2016-12-14i18n: add--interactive: mark plural stringsVasco Almeida
Mark plural strings for translation. Unfold each action case in one entire sentence. Pass new keyword for xgettext to extract. Update test to include new subroutine __n() for plural strings handling. Update documentation to include a description of the new __n() subroutine. Signed-off-by: Vasco Almeida <> Signed-off-by: Junio C Hamano <>
2016-12-14i18n: add--interactive: mark strings with interpolation for translationVasco Almeida
Since at this point Git::I18N.perl lacks support for Perl i18n placeholder substitution, use of sprintf following die or error_msg is necessary for placeholder substitution take place. Signed-off-by: Vasco Almeida <> Signed-off-by: Junio C Hamano <>
2016-12-14i18n: add--interactive: mark simple here-documents for translationVasco Almeida
Mark messages in here-documents without interpolation for translation. The here-document delimiter \EOF, which is the same as 'EOF', indicates that the text is to be treated literally without interpolation of its content. Unfortunately xgettext is not able to extract here-documents delimited with \EOF but it is with delimiter enclosed in single quotes. So change \EOF to 'EOF', although in this case does not make difference what variation of here-document to use since there is nothing to interpolate. Signed-off-by: Vasco Almeida <> Signed-off-by: Junio C Hamano <>
2016-12-14i18n: add--interactive: mark strings for translationVasco Almeida
Mark simple strings (without interpolation) for translation. Brackets around first parameter of ternary operator is necessary because otherwise xgettext fails to extract strings marked for translation from the rest of the file. Signed-off-by: Vasco Almeida <> Signed-off-by: Junio C Hamano <>
2016-09-19diff: improve positioning of add/delete blocks in diffsMichael Haggerty
Some groups of added/deleted lines in diffs can be slid up or down, because lines at the edges of the group are not unique. Picking good shifts for such groups is not a matter of correctness but definitely has a big effect on aesthetics. For example, consider the following two diffs. The first is what standard Git emits: --- a/9c572b21dd090a1e5c5bb397053bf8043ffe7fb4:git-send-email.perl +++ b/6dcfa306f2b67b733a7eb2d7ded1bc9987809edb:git-send-email.perl @@ -231,6 +231,9 @@ if (!defined $initial_reply_to && $prompting) { } if (!$smtp_server) { + $smtp_server = $repo->config('sendemail.smtpserver'); +} +if (!$smtp_server) { foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) { if (-x $_) { $smtp_server = $_; The following diff is equivalent, but is obviously preferable from an aesthetic point of view: --- a/9c572b21dd090a1e5c5bb397053bf8043ffe7fb4:git-send-email.perl +++ b/6dcfa306f2b67b733a7eb2d7ded1bc9987809edb:git-send-email.perl @@ -230,6 +230,9 @@ if (!defined $initial_reply_to && $prompting) { $initial_reply_to =~ s/(^\s+|\s+$)//g; } +if (!$smtp_server) { + $smtp_server = $repo->config('sendemail.smtpserver'); +} if (!$smtp_server) { foreach (qw( /usr/sbin/sendmail /usr/lib/sendmail )) { if (-x $_) { This patch teaches Git to pick better positions for such "diff sliders" using heuristics that take the positions of nearby blank lines and the indentation of nearby lines into account. The existing Git code basically always shifts such "sliders" as far down in the file as possible. The only exception is when the slider can be aligned with a group of changed lines in the other file, in which case Git favors depicting the change as one add+delete block rather than one add and a slightly offset delete block. This naive algorithm often yields ugly diffs. Commit d634d61ed6 improved the situation somewhat by preferring to position add/delete groups to make their last line a blank line, when that is possible. This heuristic does more good than harm, but (1) it can only help if there are blank lines in the right places, and (2) always picks the last blank line, even if there are others that might be better. The end result is that it makes perhaps 1/3 as many errors as the default Git algorithm, but that still leaves a lot of ugly diffs. This commit implements a new and much better heuristic for picking optimal "slider" positions using the following approach: First observe that each hypothetical positioning of a diff slider introduces two splits: one between the context lines preceding the group and the first added/deleted line, and the other between the last added/deleted line and the first line of context following it. It tries to find the positioning that creates the least bad splits. Splits are evaluated based only on the presence and locations of nearby blank lines, and the indentation of lines near the split. Basically, it prefers to introduce splits adjacent to blank lines, between lines that are indented less, and between lines with the same level of indentation. In more detail: 1. It measures the following characteristics of a proposed splitting position in a `struct split_measurement`: * the number of blank lines above the proposed split * whether the line directly after the split is blank * the number of blank lines following that line * the indentation of the nearest non-blank line above the split * the indentation of the line directly below the split * the indentation of the nearest non-blank line after that line 2. It combines the measured attributes using a bunch of empirically-optimized weighting factors to derive a `struct split_score` that measures the "badness" of splitting the text at that position. 3. It combines the `split_score` for the top and the bottom of the slider at each of its possible positions, and selects the position that has the best `split_score`. I determined the initial set of weighting factors by collecting a corpus of Git histories from 29 open-source software projects in various programming languages. I generated many diffs from this corpus, and determined the best positioning "by eye" for about 6600 diff sliders. I used about half of the repositories in the corpus (corresponding to about 2/3 of the sliders) as a training set, and optimized the weights against this corpus using a crude automated search of the parameter space to get the best agreement with the manually-determined values. Then I tested the resulting heuristic against the full corpus. The results are summarized in the following table, in column `indent-1`: | repository | count | Git 2.9.0 | compaction | compaction-fixed | indent-1 | indent-2 | | --------------------- | ----- | -------------- | -------------- | ---------------- | -------------- | -------------- | | afnetworking | 109 | 89 (81.7%) | 37 (33.9%) | 37 (33.9%) | 2 (1.8%) | 2 (1.8%) | | alamofire | 30 | 18 (60.0%) | 14 (46.7%) | 15 (50.0%) | 0 (0.0%) | 0 (0.0%) | | angular | 184 | 127 (69.0%) | 39 (21.2%) | 23 (12.5%) | 5 (2.7%) | 5 (2.7%) | | animate | 313 | 2 (0.6%) | 2 (0.6%) | 2 (0.6%) | 2 (0.6%) | 2 (0.6%) | | ant | 380 | 356 (93.7%) | 152 (40.0%) | 148 (38.9%) | 15 (3.9%) | 15 (3.9%) | * | bugzilla | 306 | 263 (85.9%) | 109 (35.6%) | 99 (32.4%) | 14 (4.6%) | 15 (4.9%) | * | corefx | 126 | 91 (72.2%) | 22 (17.5%) | 21 (16.7%) | 6 (4.8%) | 6 (4.8%) | | couchdb | 78 | 44 (56.4%) | 26 (33.3%) | 28 (35.9%) | 6 (7.7%) | 6 (7.7%) | * | cpython | 937 | 158 (16.9%) | 50 (5.3%) | 49 (5.2%) | 5 (0.5%) | 5 (0.5%) | * | discourse | 160 | 95 (59.4%) | 42 (26.2%) | 36 (22.5%) | 18 (11.2%) | 13 (8.1%) | | docker | 307 | 194 (63.2%) | 198 (64.5%) | 253 (82.4%) | 8 (2.6%) | 8 (2.6%) | * | electron | 163 | 132 (81.0%) | 38 (23.3%) | 39 (23.9%) | 6 (3.7%) | 6 (3.7%) | | git | 536 | 470 (87.7%) | 73 (13.6%) | 78 (14.6%) | 16 (3.0%) | 16 (3.0%) | * | gitflow | 127 | 0 (0.0%) | 0 (0.0%) | 0 (0.0%) | 0 (0.0%) | 0 (0.0%) | | ionic | 133 | 89 (66.9%) | 29 (21.8%) | 38 (28.6%) | 1 (0.8%) | 1 (0.8%) | | ipython | 482 | 362 (75.1%) | 167 (34.6%) | 169 (35.1%) | 11 (2.3%) | 11 (2.3%) | * | junit | 161 | 147 (91.3%) | 67 (41.6%) | 66 (41.0%) | 1 (0.6%) | 1 (0.6%) | * | lighttable | 15 | 5 (33.3%) | 0 (0.0%) | 2 (13.3%) | 0 (0.0%) | 0 (0.0%) | | magit | 88 | 75 (85.2%) | 11 (12.5%) | 9 (10.2%) | 1 (1.1%) | 0 (0.0%) | | neural-style | 28 | 0 (0.0%) | 0 (0.0%) | 0 (0.0%) | 0 (0.0%) | 0 (0.0%) | | nodejs | 781 | 649 (83.1%) | 118 (15.1%) | 111 (14.2%) | 4 (0.5%) | 5 (0.6%) | * | phpmyadmin | 491 | 481 (98.0%) | 75 (15.3%) | 48 (9.8%) | 2 (0.4%) | 2 (0.4%) | * | react-native | 168 | 130 (77.4%) | 79 (47.0%) | 81 (48.2%) | 0 (0.0%) | 0 (0.0%) | | rust | 171 | 128 (74.9%) | 30 (17.5%) | 27 (15.8%) | 16 (9.4%) | 14 (8.2%) | | spark | 186 | 149 (80.1%) | 52 (28.0%) | 52 (28.0%) | 2 (1.1%) | 2 (1.1%) | | tensorflow | 115 | 66 (57.4%) | 48 (41.7%) | 48 (41.7%) | 5 (4.3%) | 5 (4.3%) | | test-more | 19 | 15 (78.9%) | 2 (10.5%) | 2 (10.5%) | 1 (5.3%) | 1 (5.3%) | * | test-unit | 51 | 34 (66.7%) | 14 (27.5%) | 8 (15.7%) | 2 (3.9%) | 2 (3.9%) | * | xmonad | 23 | 22 (95.7%) | 2 (8.7%) | 2 (8.7%) | 1 (4.3%) | 1 (4.3%) | * | --------------------- | ----- | -------------- | -------------- | ---------------- | -------------- | -------------- | | totals | 6668 | 4391 (65.9%) | 1496 (22.4%) | 1491 (22.4%) | 150 (2.2%) | 144 (2.2%) | | totals (training set) | 4552 | 3195 (70.2%) | 1053 (23.1%) | 1061 (23.3%) | 86 (1.9%) | 88 (1.9%) | | totals (test set) | 2116 | 1196 (56.5%) | 443 (20.9%) | 430 (20.3%) | 64 (3.0%) | 56 (2.6%) | In this table, the numbers are the count and percentage of human-rated sliders that the corresponding algorithm got *wrong*. The columns are * "repository" - the name of the repository used. I used the diffs between successive non-merge commits on the HEAD branch of the corresponding repository. * "count" - the number of sliders that were human-rated. I chose most, but not all, sliders to rate from those among which the various algorithms gave different answers. * "Git 2.9.0" - the default algorithm used by `git diff` in Git 2.9.0. * "compaction" - the heuristic used by `git diff --compaction-heuristic` in Git 2.9.0. * "compaction-fixed" - the heuristic used by `git diff --compaction-heuristic` after the fixes from earlier in this patch series. Note that the results are not dramatically different than those for "compaction". Both produce non-ideal diffs only about 1/3 as often as the default `git diff`. * "indent-1" - the new `--indent-heuristic` algorithm, using the first set of weighting factors, determined as described above. * "indent-2" - the new `--indent-heuristic` algorithm, using the final set of weighting factors, determined as described below. * `*` - indicates that repo was part of training set used to determine the first set of weighting factors. The fact that the heuristic performed nearly as well on the test set as on the training set in column "indent-1" is a good indication that the heuristic was not over-trained. Given that fact, I ran a second round of optimization, using the entire corpus as the training set. The resulting set of weights gave the results in column "indent-2". These are the weights included in this patch. The final result gives consistently and significantly better results across the whole corpus than either `git diff` or `git diff --compaction-heuristic`. It makes only about 1/30 as many errors as the former and about 1/10 as many errors as the latter. (And a good fraction of the remaining errors are for diffs that involve weirdly-formatted code, sometimes apparently machine-generated.) The tools that were used to do this optimization and analysis, along with the human-generated data values, are recorded in a separate project [1]. This patch adds a new command-line option `--indent-heuristic`, and a new configuration setting `diff.indentHeuristic`, that activate this heuristic. This interface is only meant for testing purposes, and should be finalized before including this change in any release. [1] Signed-off-by: Michael Haggerty <> Signed-off-by: Junio C Hamano <>
2016-06-16add--interactive: respect diff.compactionHeuristicJeff King
We use plumbing to generate the diff, so it doesn't automatically pick up UI config like compactionHeuristic. Let's forward it on, since interactive adding is porcelain. Note that we only need to handle the "true" case. There's no point in passing --no-compaction-heuristic when the variable is false, since nothing else could have turned it on. Signed-off-by: Jeff King <> Signed-off-by: Junio C Hamano <>