git add -i: add a rudimentary version in C (supporting only `status` and `help` so far) #170

dscho · 2019-03-28T16:04:55Z

This is the first leg on the long journey to a fully built-in git add -i (next up: parts 2, 3, 4, 5, and 6). Note: the latter PRs are not necessarily up to date, and will be re-targeted to the appropriate branches in https://github.com/gitster/git as soon as Junio picks them up.

This here patch series reflects the part that was submitted a couple of times (see #103) during the Outreachy project by Slavica Ðukic that continued the journey based on an initial patch series by Daniel Ferreira.

It only implements the status and the help part, in the interest of making the review remotely more reviewable.

As I am a heavy user of git add -p myself and use a patched version for several months already (it is so nice to not suffer over one second startup until the MSYS2 Perl finally shows me anything, instead it feels instantaneous), I integrated these patch series into Git for Windows already, as an opt-in feature guarded by the config variable add.interactive.useBuiltin (and Git for Windows' installer knows to detect this version and offer the option in the graphical user interface).

Changes since v6:

Fixed a potential buffer overrun when parsing numbers/number ranges.

Changes since v5:

Reworded two commit messages.
Clarified code that does not affect patch_mode.
Restricted scope of the very local variable endp.

Changes since v4:

Rebased onto current master to make use of Thomas Gummerer's repo_refresh_and_write_index() as well as to avoid merge conflicts with Eric Wong's work on struct hashmap.
Instead of rolling a dedicated data struct to simulate a Trie, we now use string-list extensively (an unsorted copy and a sorted one, the latter to determine unique prefixes). This had massive ramifications on the rest of the patches... For example, the struct command_item structure no longer contains the name field, but is intended to be a util in a string_list.
Changed the commit messages and author lines to reflect Slavica's name correctly.
Touched up a couple commit messages.

Changes since v3:

Rebased to v2.23.0 to reduce friction.
free_diffstat_info() is now made public as well, and used, to avoid a memory leak.
Prepared the patches for ew/hashmap (which is strict about the hashmap entries' type in hashmap_entry_init() and friends).
The private data types have been moved from prefix-map.h to prefix-map.c.
A lot of int types were converted to more appropriate size_t in prefix-map.c.
A misleading parameter name list was renamed to the correct array.
The code comment above find_unique_prefixes() was (hopefully) improved.
The run_help() function's signature now reflects that most of the parameters are actually unused.

Changes since v2:

Rebased to master to avoid merge conflicts.
Renumbered the prefix-map test to avoid conflicts with two patch series that are currently in-flight in pu.

Changes since v1:

The config machinery was reworked completely, to not use a callback to git_config(), but instead to query the config via the repo_config_get_*() functions. This also prevents a future "Huh???" moment: the internal add --interactive API accepts a parameter of type struct repository *r, but the previous configuration did not use that to query the config (and could in the future be a repository other than the_repository).
As a consequence, the color sequences are no longer stored in file-local variables, but passed around via a struct.
Instead of using the magical constant -2 to quit the main loop, it is now defined as LIST_AND_CHOOSE_QUIT (and likewise, LIST_AND_CHOOSE_ERROR is defined as -1 and used where appropriate).
Improved the add_prefix_item() function by avoiding buffer overruns, not reusing the struct that is used for lookup also for adding the new item, and by strengthening the bug check.

Cc: Jeff Hostetler git@jeffhostetler.com, Jeff King peff@peff.net

add-interactive.c

prefix-map.c

dscho · 2019-04-10T14:41:10Z

@slavicaDj thank you for your review! Most helpful.

dscho · 2019-04-10T17:37:15Z

/submit

gitgitgadget · 2019-04-10T17:38:13Z

Submitted as pull.170.git.gitgitgadget@gmail.com

slavicaDj · 2019-04-11T11:22:54Z

@slavicaDj thank you for your review! Most helpful.

You're welcome! Thank you.

Documentation/config/add.txt

add-interactive.c

Makefile

dscho · 2019-05-13T17:27:18Z

/submit

gitgitgadget · 2019-05-13T17:28:20Z

Submitted as pull.170.v2.git.gitgitgadget@gmail.com

gitgitgadget · 2019-11-14T03:36:04Z

This patch series was integrated into pu via git@bc2831e.

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 <johannes.schindelin@gmx.de>

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. To determine the unique prefixes, as well as to look up the command in question, we use a copy of the list and sort it. While this might seem like overkill for a single command, it will make much more sense when all the commands are implemented, and when we reuse the same logic to present a list of files to edit, with convenient unique prefixes. At the start of the development of this patch series, a dedicated data structure was introduced that imitated the Trie that the Perl version implements. However, this was deemed overkill, and we now simply sort the list before determining the length of the unique prefixes by looking at each item's neighbor. As a bonus, we now use the same sorted list to perform a binary search using the user-provided prefix as search key. Original-patch-by: Slavica Đukić <slawica92@hotmail.com> Helped-by: SZEDER Gábor <szeder.dev@gmail.com> Signed-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>

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 <johannes.schindelin@gmx.de>

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 Đukić <slawica92@hotmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

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 Đukić <slawica92@hotmail.com> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>

dscho · 2019-11-15T11:10:16Z

/submit

gitgitgadget · 2019-11-15T11:11:45Z

Submitted as pull.170.v7.git.1573816280.gitgitgadget@gmail.com

gitgitgadget · 2019-11-18T06:31:04Z

This patch series was integrated into pu via git@13df35f.

gitgitgadget · 2019-11-19T07:11:53Z

This patch series was integrated into pu via git@b99a171.

gitgitgadget · 2019-11-20T05:50:32Z

This patch series was integrated into pu via git@0d718c6.

gitgitgadget · 2019-11-20T12:00:01Z

This patch series was integrated into pu via git@cea9447.

gitgitgadget · 2019-11-21T04:56:22Z

This patch series was integrated into pu via git@cb15e0a.

gitgitgadget · 2019-11-21T07:41:10Z

This patch series was integrated into pu via git@b15d70c.

gitgitgadget · 2019-11-21T07:41:12Z

This patch series was integrated into next via git@caefa55.

gitgitgadget · 2019-11-22T08:35:40Z

This patch series was integrated into pu via git@a6bf991.

gitgitgadget · 2019-11-25T08:54:39Z

This patch series was integrated into pu via git@9544a70.

gitgitgadget · 2019-11-27T10:12:12Z

This patch series was integrated into pu via git@a1eb0ff.

gitgitgadget · 2019-12-02T16:15:07Z

This patch series was integrated into pu via git@964dd06.

gitgitgadget · 2019-12-03T01:13:02Z

This patch series was integrated into pu via git@dbb878f.

gitgitgadget · 2019-12-05T22:41:54Z

This patch series was integrated into pu via git@f7998d9.

gitgitgadget · 2019-12-05T22:41:55Z

This patch series was integrated into next via git@f7998d9.

gitgitgadget · 2019-12-05T22:41:57Z

This patch series was integrated into master via git@f7998d9.

gitgitgadget · 2019-12-05T22:42:00Z

Closed via f7998d9.

dscho changed the title ~~Add i in c status and help~~ git add -i: add a rudimentary version in C (supporting only status and help so far) Mar 28, 2019

dscho force-pushed the add-i-in-c-status-and-help branch 6 times, most recently from 11991db to 3d02c5b Compare April 3, 2019 09:40