Skip to content

Commit 801b4b7

Browse files
committed
Merge branch 'add-i-in-c-status-and-help'
The first part of the long journey to a fully built-in `git add -i`. It reflects the part that was submitted a couple of times (see gitgitgadget#103) during the Outreachy project by Slavica Đukić that continued the journey based on an initial patch series by Daniel Ferreira. This part only implements the `status` and the `help` part, like Slavica's last iteration did, in the interest of making the review remotely more reviewable. I fear that this attempt of making it a bit more reviewable is pretty futile, as so many things changed. So I will ask the reviewers for forgiveness: please be kind, and give this sort of a fresh review. I threw in a couple of major changes on top of that iteration, though: - The original plan was to add a helper (`git add--helper`) that takes over more and more responsibility from the Perl script over the course of the conversion. This plan is no longer in effect, as I encountered a serious problem with that: the MSYS2 runtime used by the Perl interpreter which Git for Windows employs to run `git add -i` has a curious bug (that is safely outside the purview of this here patch series) where it fails to read from standard input after it spawned a non-MSYS2 program that reads from standard input. To keep my `git add -i` in a working state, I therefore adopted a different strategy: Just like `git difftool` was converted by starting with a built-in that did nothing but handing off to the scripted version, guarded by the (opt-in) `difftool.useBuiltin` config setting, I start this patch series by a built-in `add -i` that does nothing else but state that it is not implemented yet, guarded by the (opt-in) `add.interactive.useBuiltin` config setting. In contrast to the `git difftool` situation, it is quite a bit easier here, as we do not even have to rename the script to `git-legacy-add--interactive.perl`: the `add--interactive` command is an implementation detail that users are not even supposed to know about. Therefore, we can implement that road fork between the built-in and the scripted version in `builtin/add.c`, i.e. in the user-facing `git add` command. This will also naturally help with the transition to a fully built-in `git add -i`/`git add -p`, as we saw with the built-in `git rebase` how important it is for end users to have an escape hatch (and for that reason, tried our best to provide the same with the built-in `git stash`). - The `help` command was actually not hooked up in `git add -i`, but was only available as a special option of the `git add--helper` command. As that command no longer exists, I kind of *had* to implement some way to let the built-in `git add -i` show the help text. - The main loop of `git add -i` (i.e. the thing that lets you choose `status` or `help`) is now implemented (but only lists `status` and `help`, of course), as it makes use of that feature that took the main chunk of the Outreachy project: the function to determine unique prefixes of a list of strings. - Speaking of the unique prefixes: the functionality to determine those is now encapsulated in the `prefix-map.c` file, and I also added a regression test. - Speaking of the tests: I also implemented support for the environment variable `GIT_TEST_ADD_I_USE_BUILTIN`: by setting it, the test suite can be forced to use the built-in, or the Perl script, version of `git add -i`. Needless to say: by the end of this patch series, running the test suite with `GIT_TEST_ADD_I_USE_BUILTIN=true` will still result in a ton of test failures due to not-yet-implemented commands, but it will also demonstrate what *already* works. - Since the main loop starts not only by showing the status, but refreshes the index before that, I added that, and I actually refactored that code into a new function (`repo_refresh_and_write_index()`), as it will be used a couple of times by the end of the complete conversion of `git add -i` into a built-in command. Signed-off-by: Johannes Schindelin <[email protected]>
2 parents 08f5629 + 096196d commit 801b4b7

32 files changed

+2915
-94
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
/git-interpret-trailers
8484
/git-instaweb
8585
/git-legacy-rebase
86+
/git-legacy-stash
8687
/git-log
8788
/git-ls-files
8889
/git-ls-remote

Documentation/config/add.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,3 +5,8 @@ add.ignore-errors (deprecated)::
55
option of linkgit:git-add[1]. `add.ignore-errors` is deprecated,
66
as it does not follow the usual naming convention for configuration
77
variables.
8+
9+
add.interactive.useBuiltin::
10+
[EXPERIMENTAL] Set to `true` to use the experimental built-in
11+
implementation of the interactive version of linkgit:git-add[1]
12+
instead of the Perl script version. Is `false` by default.

Documentation/git-stash.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ SYNOPSIS
99
--------
1010
[verse]
1111
'git stash' list [<options>]
12-
'git stash' show [<stash>]
12+
'git stash' show [<options>] [<stash>]
1313
'git stash' drop [-q|--quiet] [<stash>]
1414
'git stash' ( pop | apply ) [--index] [-q|--quiet] [<stash>]
1515
'git stash' branch <branchname> [<stash>]
@@ -106,7 +106,7 @@ stash@{1}: On master: 9cc0589... Add git-stash
106106
The command takes options applicable to the 'git log'
107107
command to control what is shown and how. See linkgit:git-log[1].
108108

109-
show [<stash>]::
109+
show [<options>] [<stash>]::
110110

111111
Show the changes recorded in the stash entry as a diff between the
112112
stashed contents and the commit back when the stash entry was first

Makefile

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -633,9 +633,9 @@ SCRIPT_SH += git-merge-resolve.sh
633633
SCRIPT_SH += git-mergetool.sh
634634
SCRIPT_SH += git-quiltimport.sh
635635
SCRIPT_SH += git-legacy-rebase.sh
636+
SCRIPT_SH += git-legacy-stash.sh
636637
SCRIPT_SH += git-remote-testgit.sh
637638
SCRIPT_SH += git-request-pull.sh
638-
SCRIPT_SH += git-stash.sh
639639
SCRIPT_SH += git-submodule.sh
640640
SCRIPT_SH += git-web--browse.sh
641641

@@ -754,6 +754,7 @@ TEST_BUILTINS_OBJS += test-online-cpus.o
754754
TEST_BUILTINS_OBJS += test-parse-options.o
755755
TEST_BUILTINS_OBJS += test-path-utils.o
756756
TEST_BUILTINS_OBJS += test-pkt-line.o
757+
TEST_BUILTINS_OBJS += test-prefix-map.o
757758
TEST_BUILTINS_OBJS += test-prio-queue.o
758759
TEST_BUILTINS_OBJS += test-reach.o
759760
TEST_BUILTINS_OBJS += test-read-cache.o
@@ -848,6 +849,7 @@ LIB_H = $(shell $(FIND) . \
848849
-name '*.h' -print)
849850

850851
LIB_OBJS += abspath.o
852+
LIB_OBJS += add-interactive.o
851853
LIB_OBJS += advice.o
852854
LIB_OBJS += alias.o
853855
LIB_OBJS += alloc.o
@@ -966,6 +968,7 @@ LIB_OBJS += patch-ids.o
966968
LIB_OBJS += path.o
967969
LIB_OBJS += pathspec.o
968970
LIB_OBJS += pkt-line.o
971+
LIB_OBJS += prefix-map.o
969972
LIB_OBJS += preload-index.o
970973
LIB_OBJS += pretty.o
971974
LIB_OBJS += prio-queue.o
@@ -1138,6 +1141,7 @@ BUILTIN_OBJS += builtin/shortlog.o
11381141
BUILTIN_OBJS += builtin/show-branch.o
11391142
BUILTIN_OBJS += builtin/show-index.o
11401143
BUILTIN_OBJS += builtin/show-ref.o
1144+
BUILTIN_OBJS += builtin/stash.o
11411145
BUILTIN_OBJS += builtin/stripspace.o
11421146
BUILTIN_OBJS += builtin/submodule--helper.o
11431147
BUILTIN_OBJS += builtin/symbolic-ref.o

0 commit comments

Comments
 (0)