Offer a built-in version of git add -i and git add -p
#2150
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Companion PRs over at GitGitGadget: |
dscho
force-pushed
the
add-p-gfw
branch
2 times, most recently
from
61e49a5 to
d177a6a
Compare
This is hardly the first conversion of a Git command that is implemented as a script to a built-in. So far, the most successful strategy for such conversions has been to add a built-in helper and call that for more and more functionality from the script, as more and more parts are converted. With the interactive add, we choose a different strategy. The sole reason for this is that on Windows (where such a conversion has the most benefits in terms of speed and robustness) we face the very specific problem that a `system()` call in Perl seems to close `stdin` in the parent process when the spawned process consumes even one character from `stdin`. And that just does not work for us here, as it would stop the main loop as soon as any interactive command was performed by the helper. Which is almost all of the commands in `git add -i`. It is almost as if Perl told us once again that it does not want us to use it on Windows. Instead, we follow the opposite route where we start with a bare-bones version of the built-in interactive add, guarded by the new `add.interactive.useBuiltin` config variable, and then add more and more functionality to it, until it is feature complete. At this point, the built-in version of `git add -i` only states that it cannot do anything yet ;-) Signed-off-by: Johannes Schindelin <[email protected]>
Make the diffstat interface (namely, the diffstat_t struct and compute_diffstat) no longer be internal to diff.c and allow it to be used by other parts of git. This is helpful for code that may want to easily extract information from files using the diff machinery, while flushing it differently from how the show_* functions used by diff_flush() do it. One example is the builtin implementation of git-add--interactive's status. Signed-off-by: Daniel Ferreira <[email protected]> Signed-off-by: Slavica Djukic <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
This implements the `status` command of `git add -i`. The data structures introduced in this commit will be extended as needed later. At this point, we re-implement only part of the `list_and_choose()` function of the Perl script `git-add--interactive.perl` and call it `list()`. It does not yet color anything, or do columns, or allow user input. Over the course of the next commits, we will introduce a `list_and_choose()` function that uses `list()` to display the list of options and let the user choose one or more of the displayed items. This will be used to implement the main loop of the built-in `git add -i`, at which point the new `status` command can actually be used. Note that we pass the list of items as a `struct item **` as opposed to a `struct item *`, to allow for the actual items to contain much more information than merely the name. Signed-off-by: Daniel Ferreira <[email protected]> Signed-off-by: Slavica Djukic <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
This is what the Perl version does, and therefore it is what the built-in version should do, too. Signed-off-by: Johannes Schindelin <[email protected]>
The `add -p` changes conflict with this commit, so let's revert it for now. Besides, it would appear that the rationale of the change is not sound: in a quick test in a PowerShell, it seems that echo *is* the default. To verify, test, run this in a PowerShell session: git -c alias.s="! unset GIT_CONFIG_PARAMETERS; printf `"hostname=123\\n\\n`" | GIT_ASKPASS= ./git \ -c credential.helper= --exec-path=`$PWD credential fill " s This reverts commit b9990a3. Signed-off-by: Johannes Schindelin <[email protected]>
We do not actually need this, as `bash` is not a BusyBox applet, and in the unlikely event anybody would call `git.exe` from within WSL, they would use `/cmd/git.exe` which puts the MSYS2 `bash.exe` into the `PATH` before the WSL `bash`. This reverts commit 040d9db. Signed-off-by: Johannes Schindelin <[email protected]>
The "hack" to use `bash`'s `read -r` function to read reliably from `/dev/tty` conflicts with the upcoming built-in `add -p`, to support single key stroke commands. Let's back out the changes, and then re-apply them on top of `add -p`, consolidated. Signed-off-by: Johannes Schindelin <[email protected]>
This conflicts with the `add-p` series and will be reapplied on top of the merge later. This reverts commit dc72b08. Signed-off-by: Johannes Schindelin <[email protected]>
…eractively" This conflicts with the `add-p` series and will be reapplied on top of the merge later. This reverts commit c8c1c27. Signed-off-by: Johannes Schindelin <[email protected]>
For simplicity, we only implemented the `status` command without colors. This patch starts adding color, matching what the Perl script `git-add--interactive.perl` does. Original-Patch-By: Daniel Ferreira <[email protected]> Signed-off-by: Slavica Djukic <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
The reason why we did not start with the main loop to begin with is that it is the first user of `list_and_choose()`, which uses the `list()` function that we conveniently introduced for use by the `status` command. Apart from the "and choose" part, there are more differences between the way the `status` command calls the `list_and_choose()` function in the Perl version of `git add -i` compared to the other callers of said function. The most important ones: - The list is not only shown, but the user is also asked to make a choice, possibly selecting multiple entries. - The list of items is prefixed with a marker indicating what items have been selected, if multi-selection is allowed. - Initially, for each item a unique prefix (if there exists any within the given parameters) is determined, and shown in the list, and accepted as a shortcut for the selection. These features will be implemented later, except the part where the user can choose a command. At this stage, though, the built-in `git add -i` still only supports the `status` command, with the remaining commands to follow over the course of the next commits. In addition, we also modify `list()` to support displaying the commands in columns, even if there is currently only one. The Perl script `git-add--interactive.perl` mixed the purposes of the "list" and the "and choose" part into the same function. In the C version, we will keep them separate instead, calling the `list()` function from the `list_and_choose()` function. Note that we only have a prompt ending in a single ">" at this stage; later commits will add commands that display a double ">>" to indicate that the user is in a different loop than the main one. Signed-off-by: Johannes Schindelin <[email protected]>
In the `git add -i` command, we show unique prefixes of the commands and files, to give an indication what prefix would select them. Naturally, the C implementation looks a lot different than the Perl implementation: in Perl, a trie is much easier implemented, while we already have a pretty neat hashmap implementation in C that we use for the purpose of storing (not necessarily unique) prefixes. The idea: for each item that we add, we generate prefixes starting with the first letter, then the first two letters, then three, etc, until we find a prefix that is unique (or until the prefix length would be longer than we want). If we encounter a previously-unique prefix on the way, we adjust that item's prefix to make it unique again (or we mark it as having no unique prefix if we failed to find one). These partial prefixes are stored in a hash map (for quick lookup times). To make sure that this function works as expected, we add a test using a special-purpose test helper that was added for that purpose. Note: We expect the list of prefix items to be passed in as a list of pointers rather than as regular list to avoid having to copy information (the actual items will most likely contain more information than just the name and the length of the unique prefix, but passing in `struct prefix_item *` would not allow for that). Signed-off-by: Slavica Djukic <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
Just like in the Perl script `git-add--interactive.perl`, for each command a unique prefix is determined (if there exists any within the given parameters), and shown in the list, and accepted as a shortcut for the command. We use the prefix map implementation that we just added in the previous commit for that purpose. Signed-off-by: Slavica Djukic <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
With this change, we print out the same colored help text that the Perl-based `git add -i` prints in the main loop when question mark is entered. Signed-off-by: Johannes Schindelin <[email protected]>
The error messages as well as the unique prefixes are colored in `git add -i` by default; We need to do the same in the built-in version. Signed-off-by: Slavica Djukic <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
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 Djukic <[email protected]> Signed-off-by: Johannes Schindelin <[email protected]>
In `update` command of `git add -i`, we are primarily interested in the list of modified files that have worktree (i.e. unstaged) changes. The Perl script version of `git add -i` has a parameter of the `list_modified()` function for that matter. In C, we can be a lot more precise, using an `enum`. The C implementation of the filter also has an easier time to avoid unnecessary work, simply by using an adaptive order of the `diff-index` and `diff-files` calls, and then not adding unnecessary entries in the first place. Signed-off-by: Johannes Schindelin <[email protected]>
The `upgrade`, `revert` and `add-untracked` commands allow selecting multiple entries. Let's extend the `list_and_choose()` function to accommodate those use cases. Signed-off-by: Johannes Schindelin <[email protected]>
After `status` and `help`, it is now turn to port the `update` command to C, the second command that is shown in the main loop menu of `git add -i`. This `git add -i` command is the first one which lets the user choose a subset of a list of files, and as such, this patch lays the groundwork for the other commands of that category: - It teaches the `print_file_item()` function to show a unique prefix if we found any (the code to find it had been added already in the previous patch where we colored the unique prefixes of the main loop commands, but that patch uses the `print_command_item()` function to display the menu items). - This patch also adds the help text that is shown when the user input to select items from the shown list could not be parsed. Signed-off-by: Johannes Schindelin <[email protected]>
This is a relatively straight-forward port from the Perl version, with the notable exception that we imitate `git reset -- <paths>` in the C version rather than the convoluted `git ls-tree HEAD -- <paths> | git update-index --index-info` followed by `git update-index --force-remove -- <paths>` for the missed ones. While at it, we fix the pretty obvious bug where the `revert` command offers to unstage files that do not have staged changes. Signed-off-by: Johannes Schindelin <[email protected]>
This is yet another command, ported to C. It builds nicely on the support functions introduced for other commands, with the notable difference that only names are displayed for untracked files, no file type or diff summary. Signed-off-by: Johannes Schindelin <[email protected]>
Well, it is not a full implementation yet. In the interest of making this easy to review (and easy to keep bugs out), we still hand off to the Perl script to do the actual work. The `patch` functionality actually makes up for more than half of the 1,800+ lines of `git-add--interactive.perl`. It will be ported from Perl to C incrementally, later. Signed-off-by: Johannes Schindelin <[email protected]>
It is not only laziness that we simply spawn `git diff -p --cached` here: this command needs to use the pager, and the pager needs to exit when the diff is done. Currently we do not have any way to make that happen if we run the diff in-process. So let's just spawn. Signed-off-by: Johannes Schindelin <[email protected]>
We do not really want to `exit()` here, of course, as this is safely libified code. Signed-off-by: Johannes Schindelin <[email protected]>
dscho
added a commit
to git-for-windows/build-extra
that referenced
this pull request
May 8, 2019
See git-for-windows/git#2150. This is exciting... Signed-off-by: Johannes Schindelin <[email protected]>
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
git-for-windows-ci
pushed a commit
that referenced
this pull request
May 8, 2019
Offer a built-in version of `git add -i` and `git add -p`
dscho
added a commit
that referenced
this pull request
May 8, 2019
Offer a built-in version of `git add -i` and `git add -p`
dscho
added a commit
that referenced
this pull request
May 9, 2019
Offer a built-in version of `git add -i` and `git add -p`
dscho
added a commit
that referenced
this pull request
May 9, 2019
Offer a built-in version of `git add -i` and `git add -p`
dscho
added a commit
that referenced
this pull request
May 9, 2019
Offer a built-in version of `git add -i` and `git add -p`
dscho
added a commit
to git-for-windows/build-extra
that referenced
this pull request
May 9, 2019
There are now [experimental built-in versions of `git add -i` and `git add -p`](git-for-windows/git#2150), i.e. those modes are now a lot faster (in particular at startup). You can opt into using them on the last installer page. Signed-off-by: Johannes Schindelin <[email protected]>
git-for-windows-ci
pushed a commit
that referenced
this pull request
May 9, 2019
Offer a built-in version of `git add -i` and `git add -p`
dscho
added a commit
that referenced
this pull request
May 10, 2019
Offer a built-in version of `git add -i` and `git add -p`
dscho
added a commit
that referenced
this pull request
May 10, 2019
Offer a built-in version of `git add -i` and `git add -p`
dscho
added a commit
that referenced
this pull request
May 10, 2019
Offer a built-in version of `git add -i` and `git add -p`
git-for-windows-ci
pushed a commit
that referenced
this pull request
May 10, 2019
Offer a built-in version of `git add -i` and `git add -p`
git-for-windows-ci
pushed a commit
that referenced
this pull request
May 10, 2019
Offer a built-in version of `git add -i` and `git add -p`
git-for-windows-ci
pushed a commit
that referenced
this pull request
May 10, 2019
Offer a built-in version of `git add -i` and `git add -p`
dscho
added a commit
that referenced
this pull request
May 13, 2019
Offer a built-in version of `git add -i` and `git add -p`
dscho
added a commit
that referenced
this pull request
May 13, 2019
Offer a built-in version of `git add -i` and `git add -p`
|
Is there a way to enable this feature if I'm using the portable distribution of Git for Windows? A global configuration option or some such? Or does the installer deploy different executables/scripts if you enable this feature in the installer? Please advise. |
|
Yes! The config option |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reduces not only our dependency on Perl even further, but it also reduces the startup time of
git add -psubstantially.Note: this PR is currently meant primarily as a way to test this feature thoroughly, with plans to merge it when the patches have been reviewed on the Git mailing list for a bit.