built-in add -i/-p: update to latest iteration #2402
Merged
Conversation
This file contains hidden or 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
[TODO! Cherry-pick the latest changes from gitgitgadget#170] Start to implement a built-in version of `git add --interactive` Unlike previous conversions to C, where we started with a built-in helper, we start this conversion by adding an interception in the `run_add_interactive()` function when the new opt-in `add.interactive.useBuiltin` config knob is turned on (or the corresponding environment variable `GIT_TEST_ADD_I_USE_BUILTIN`), and calling the new internal API function `run_add_i()` that is implemented directly in libgit.a. At this point, the built-in version of `git add -i` only states that it cannot do anything yet. In subsequent patches/patch series, the `run_add_i()` function will gain more and more functionality, until it is feature complete. The whole arc of the conversion can be found in the PRs msysgit#170-175 at https://github.com/gitgitgadget/git. The "--helper approach" can unfortunately not be used here: on Windows 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`. Which prevents us from implementing the main loop in C and still trying to hand off to the Perl script. The very real downside of the approach we have to take here is that the test suite won't pass with `GIT_TEST_ADD_I_USE_BUILTIN=true` until the conversion is complete (the `--helper` approach would have let it pass, even at each of the incremental conversion steps). Signed-off-by: Johannes Schindelin <[email protected]>
built-in add -i: implement the main loop 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. In contrast to the Perl version, in the built-in interactive `add`, we will keep the `list()` function (which only displays items) and the `list_and_choose()` function (which uses `list()` to display the items, and only takes care of the "and choose" part) separate. The `list_and_choose()` function, as implemented in `git-add--interactive.perl` knows a few more tricks than the function we introduce in this patch: - There is a flag to let the user select multiple items. - In multi-select mode, the list of items is prefixed with a marker indicating what items have been selected. - Initially, for each item 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 selection. These features will be implemented in the C version later. This patch does not add any new main loop command, of course, the built-in `git add -i` still only supports the `status` command. The remaining commands to follow over the course of the next commits. To accommodate for listing the commands in columns, preparing for the commands that will be implemented over the course of the next patches/patch series, we teach the `list()` function to do precisely that. 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]>
A long time ago, Phillip Wood pointed out that this would be easier to read, and I agreed, but failed to backport the fix to Git for Windows' `master` branch. Signed-off-by: Johannes Schindelin <[email protected]>
Phillip Wood pointed out, long ago, that `temp` -> `merged` would make it easier to read this code. Somehow, this change only made it into gitgitgadget#173 but not into Git for Windows' `master`. Signed-off-by: Johannes Schindelin <[email protected]>
This helper supports the scenario where Git has a populated `strbuf` and wants to let the user edit it interactively. In `git add -p`, we will use this to allow interactive hunk editing: the diff hunks are already in memory, but we need to write them out to a file so that an editor can be launched, then read everything back once the user is done editing. Signed-off-by: Johannes Schindelin <[email protected]>
Phillip Wood pointed out, long ago, that the code is underdocumented and could use a helper for calling the editor. Somehow, this change only made it into gitgitgadget#173 but not into Git for Windows' `master`. Signed-off-by: Johannes Schindelin <[email protected]>
Let's not rename the `unused_opts` parameter; It is _still_ unused. Signed-off-by: Johannes Schindelin <[email protected]>
Whoops, a buffer overrun. Signed-off-by: Johannes Schindelin <[email protected]>
During a final review before submitting it, this developer stumbled over the fact that the result of `strtoul()` was decremented by one for `from`, but not for `to`. Let's add a code comment to stave off confusion. Signed-off-by: Johannes Schindelin <[email protected]>
dscho
changed the title
add-i-in-c-status-and-help
dscho
added a commit
that referenced
this pull request
Nov 16, 2019
built-in add -i/-p: update to latest iteration
dscho
added a commit
that referenced
this pull request
Nov 16, 2019
built-in add -i/-p: update to latest iteration
dscho
added a commit
that referenced
this pull request
Nov 16, 2019
built-in add -i/-p: update to latest iteration
dscho
added a commit
that referenced
this pull request
Nov 16, 2019
built-in add -i/-p: update to latest iteration
dscho
added a commit
that referenced
this pull request
Nov 20, 2019
built-in add -i/-p: update to latest iteration
dscho
added a commit
that referenced
this pull request
Nov 21, 2019
built-in add -i/-p: update to latest iteration
dscho
added a commit
that referenced
this pull request
Nov 25, 2019
built-in add -i/-p: update to latest iteration
dscho
added a commit
that referenced
this pull request
Nov 25, 2019
built-in add -i/-p: update to latest iteration
git-for-windows-ci
pushed a commit
that referenced
this pull request
Nov 25, 2019
built-in add -i/-p: update to latest iteration
git-for-windows-ci
pushed a commit
that referenced
this pull request
Nov 25, 2019
built-in add -i/-p: update to latest iteration
dscho
added a commit
that referenced
this pull request
Nov 26, 2019
built-in add -i/-p: update to latest iteration
dscho
added a commit
that referenced
this pull request
Nov 27, 2019
built-in add -i/-p: update to latest iteration
dscho
added a commit
that referenced
this pull request
Dec 2, 2019
built-in add -i/-p: update to latest iteration
git-for-windows-ci
pushed a commit
that referenced
this pull request
Dec 2, 2019
built-in add -i/-p: update to latest iteration
dscho
added a commit
that referenced
this pull request
Dec 6, 2019
built-in add -i/-p: update to latest iteration
dscho
added a commit
that referenced
this pull request
Dec 6, 2019
built-in add -i/-p: update to latest iteration
dscho
added a commit
that referenced
this pull request
Dec 7, 2019
built-in add -i/-p: update to latest iteration
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.
Yes, another iteration ;-)