git/add-interactive.c, branch v2.51.2

add-interactive: manually fall back color config to color.ui

2025-09-08T21:00:32Z

Color options like color.interactive and color.diff should fall back to the value of color.ui if they aren't set. In add-interactive, we check the specific options (e.g., color.diff) via repo_config_get_value(), which does not depend on the main command having loaded any color config via the git_config() callback mechanism. But then we call want_color() on the result; if our specific config is unset then that function uses the value of git_use_color_default. That variable is typically set from color.ui by the git_color_config() callback, which is called by the main command in its own git_config() callback function. This works fine for "add -p", whose add_config() callback calls into git_color_config(). But it doesn't work for other commands like "checkout -p", which is otherwise unaware of color at all. People tend not to notice because the default is "auto", and that's what they'd set color.ui to as well. But something like: git -c color.ui=false checkout -p should disable color, and it doesn't. This regression goes back to 0527ccb1b5 (add -i: default to the built-in implementation, 2021-11-30). In the perl version we got the color config from "git config --get-colorbool", which did the full lookup for us. The obvious fix is for git-checkout to add a call to git_color_config() to its own config callback. But we'd have to do so for every command with this problem, which is error-prone. Let's see if we can fix it more centrally. It is tempting to teach want_color() to look up the value of repo_config_get_value("color.ui") itself. But I think that would have disastrous consequences. Plumbing commands, especially older ones, avoid porcelain config like "color.*" by simply not parsing it in their config callbacks. Looking up the value of color.ui under the hood would undermine that. Instead, let's do that lookup in the add-interactive setup code. We're already demand-loading other color config there, which is probably fine (even in a plumbing command like "git reset", the interactive mode is inherently porcelain-ish). That catches all commands that use the interactive code, whether they were calling git_color_config() themselves or not. Reported-by: Isaac Oscar Gariano Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

add-interactive: respect color.diff for diff coloring

2025-09-08T21:00:32Z

The old perl git-add--interactive.perl script used the color.diff config option to decide whether to color diffs (and if not set, it fell back to the value of color.ui via git-config's --get-colorbool option). When we switched to the builtin version, this was lost: we respect only color.ui. So for example: git -c color.diff=false add -p would color the diff, even when it should not. The culprit is this line in add-interactive.c's parse_diff(): if (want_color_fd(1, -1)) That "-1" means "no config has been set", which causes it to fall back to the color.ui setting. We should instead be passing the value of color.diff. But the problem is that we never even parse that config option! Instead the builtin interactive code parses only the value of color.interactive, which is used for prompts and other messages. One could perhaps argue that this should cover interactive diff coloring, too, but historically it did not. The perl script treated color.interactive and color.diff separately. So we should grab the values for both, keeping separate fields in our add_i_state variable, rather than a single use_color field. We also load individual color slots (e.g., color.interactive.prompt), leaving them as the empty string when color is disabled. This happens via the init_color() helper in add-interactive, which checks that use_color field. Now that there are two such fields, we need to pass the appropriate one for each color. The colors are mostly easy to divide up; color.interactive.* follows color.interactive, and color.diff.* follows color.diff. But the "reset" color is tricky. It is used for both types of coloring, but the two can be configured independently. So we introduce two separate reset colors, and use each in the appropriate spot. There are two new tests. The first enables interactive prompt colors but disables color.diff. We should see a colored prompt but not a colored diff, showing that we are now respecting color.diff (and not color.interactive or color.ui). The second does the opposite. We disable color.interactive but turn on color.diff with a custom fragment color. When we split a hunk, the interactive code has to re-color the hunk header, which lets us check that we correctly loaded the color.diff.frag config based on color.diff, not color.interactive. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano

add-patch: add diff.context command line overrides

2025-07-29T15:52:45Z

This patch compliments the previous commit, where builtins that use add-patch infrastructure now respect diff.context and diff.interHunkContext file configurations. In particular, this patch helps users who don't want to set persistent context configurations or just want a way to override them on a one-time basis, by allowing the relevant builtins to accept corresponding command line options that override the file configurations. This mimics commands such as diff and log, which allow for both context file configuration and command line overrides. Signed-off-by: Leon Michalak Signed-off-by: Junio C Hamano

add-patch: respect diff.context configuration

2025-07-29T15:52:45Z

Various builtins that use add-patch infrastructure do not respect the user's diff.context and diff.interHunkContext file configurations. The user may be used to seeing their diffs with customized context size, but not in the patches "git add -p" shows them to pick from. Teach add-patch infrastructure to read these configuration variables and pass their values when spawning the underlying plumbing commands as their command line option. Signed-off-by: Leon Michalak Signed-off-by: Junio C Hamano

add-interactive: stop using `the_repository`

2024-12-18T18:44:31Z

Stop using `the_repository` in the "add-interactive" subsystem by reusing the repository we already have available via parameters or in the `add_i_state` structure. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

global: mark code units that generate warnings with `-Wsign-compare`

2024-12-06T11:20:02Z

Mark code units that generate warnings with `-Wsign-compare`. This allows for a structured approach to get rid of all such warnings over time in a way that can be easily measured. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

global: introduce `USE_THE_REPOSITORY_VARIABLE` macro

2024-06-14T17:26:33Z

Use of the `the_repository` variable is deprecated nowadays, and we slowly but steadily convert the codebase to not use it anymore. Instead, callers should be passing down the repository to work on via parameters. It is hard though to prove that a given code unit does not use this variable anymore. The most trivial case, merely demonstrating that there is no direct use of `the_repository`, is already a bit of a pain during code reviews as the reviewer needs to manually verify claims made by the patch author. The bigger problem though is that we have many interfaces that implicitly rely on `the_repository`. Introduce a new `USE_THE_REPOSITORY_VARIABLE` macro that allows code units to opt into usage of `the_repository`. The intent of this macro is to demonstrate that a certain code unit does not use this variable anymore, and to keep it from new dependencies on it in future changes, be it explicit or implicit For now, the macro only guards `the_repository` itself as well as `the_hash_algo`. There are many more known interfaces where we have an implicit dependency on `the_repository`, but those are not guarded at the current point in time. Over time though, we should start to add guards as required (or even better, just remove them). Define the macro as required in our code units. As expected, most of our code still relies on the global variable. Nearly all of our builtins rely on the variable as there is no way yet to pass `the_repository` to their entry point. For now, declare the macro in "biultin.h" to keep the required changes at least a little bit more contained. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

hash: require hash algorithm in `empty_tree_oid_hex()`

2024-06-14T17:26:33Z

The `empty_tree_oid_hex()` function use `the_repository` to derive the hash function that shall be used. Require callers to pass in the hash algorithm to get rid of this implicit dependency. While at it, remove the unused `empty_blob_oid_hex()` function. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

cocci: apply rules to rewrite callers of "refs" interfaces

2024-05-07T17:06:59Z

Apply the rules that rewrite callers of "refs" interfaces to explicitly pass `struct ref_store`. The resulting patch has been applied with the `--whitespace=fix` option. Signed-off-by: Patrick Steinhardt Signed-off-by: Junio C Hamano

add-interactive: plug a leak in get_untracked_files

2024-04-22T23:27:42Z

Plug a leak we have since ab1e1cccaf (built-in add -i: re-implement `add-untracked` in C, 2019-11-29). This leak can be triggered with: $ echo a | git add -i As a curiosity, we have a somewhat similar function in builtin/stash.c, which correctly frees the memory. Signed-off-by: Rubén Justo Signed-off-by: Junio C Hamano