From 9f3a11508719a244fa52d5418bfd29847a272211 Mon Sep 17 00:00:00 2001 From: Phillip Wood Date: Wed, 26 Nov 2025 14:33:37 +0000 Subject: [PATCH 01/14] replay: do not copy "gpgsign-sha256" header When "git replay" replays a commit it copies the extended headers across from the original commit. However, if the original commit was signed, we do not want to copy the header associated with the signature is it wont be valid for the new commit. The code already knows to avoid coping the "gpgsig" header but does not know to avoid copying the "gpgsig-sha256" header. Add that header to the list of exclusions to match what "git commit --amend" does. Signed-off-by: Phillip Wood Signed-off-by: Junio C Hamano --- builtin/replay.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builtin/replay.c b/builtin/replay.c index 6172c8aacc..d12e4d5487 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -67,7 +67,7 @@ static struct commit *create_commit(struct repository *repo, const char *message = repo_logmsg_reencode(repo, based_on, NULL, out_enc); const char *orig_message = NULL; - const char *exclude_gpgsig[] = { "gpgsig", NULL }; + const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL }; commit_list_insert(parent, &parents); extra = read_commit_extra_headers(based_on, exclude_gpgsig); -- GitLab From c5947f11ce20376951470abbed927d86058a2215 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 19 Aug 2025 09:22:39 +0200 Subject: [PATCH 02/14] Introduce git-history(1) command for easy history editing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Hi, over recent months I've been playing around with Jujutsu quite frequently. While I still prefer using Git, there's been a couple features in it that I really like and that I'd like to have in Git, as well. A copule of these features relate to history editing. Most importantly, I really dig the following commands: - jj-abandon(1) to drop a specific commit from your history. - jj-absorb(1) to take some changes and automatically apply them to commits in your history that last modified the respective hunks. - jj-split(1) to split a commit into two. - jj-new(1) to insert a new commit after or before a specific other commit. Not all of these commands can be ported directly into Git. jj-new(1) for example doesn't really make a ton of sense for us, I'd claim. But some of these commands _do_ make sense. This patch series is a starting point for such a command. I've significantly slimmed it down from the first couple revisions now following the discussions at the Contributor's Summit yesterday. This was my intent anyway, as I already mentioned on the last iteration. Changes in v7: - I've rebased the patch series on top of f0ef5b6d9b (The fifth batch, 2025-11-30) with pw/replay-exclude-gpgsig-fix at 9f3a115087 (replay: do not copy "gpgsign-sha256" header, 2025-11-26) merged into it to fix some conflicts. - I refactored a bunch of code to be shared between split and reword. - I am now using "--ancestry-path" and "--reverse" to enumerate commits. - `git history split` now asks for commit messages for both commits. - `git history split` no longer allows the user to edit hunks so that conflicts become impossible. This may be reintroduced at a later point in time. - A bunch of test improvements. - Several commit message and documentation improvements. - Link to v6: https://lore.kernel.org/r/20251027-b4-pks-history-builtin-v6-0-407dd3f57ad3@pks.im Changes in v6: - I've rebased the patch series again to pull in the latest updates from sa/replay-atomic-ref-updates and fix conflicts. It is now based on 4e98b730f1 (The twenty-fourth batch, 2025-10-24) with ab661bb1bb (replay: add replay.refAction config option, 2025-10-23) merged into it. - I've dropped the "-m" options for now, so commit messages are always asked for via the editor. These can be introduced in a subsequent patch series once discussion around them has settled. - We don't use the merge machinery anymore to pick the commits. - Drop the commit to parse commits in the replay machinery. It didn't seem to be necessary in v5 anymore, and now that we don't use the merge machinery at all we don't ever take that code path in the first place. - Link to v5: https://lore.kernel.org/r/20251021-b4-pks-history-builtin-v5-0-78d23f578fe6@pks.im Changes in v5: - I've changed the patch series to be based on top of 133d151831 (The twenty-first batch, 2025-10-20) with sa/replay-atomic-ref-updates at a1c22e627e (SQAUASH??? t0450 band-aid, 2025-10-14) merged into it. This is one the one hand to fix a conflict, but also to get some of the CI updates to make GitLab CI work again. - Some slight commit message improvements. - Deduplicate subcommand usage strings by using defines. - Fix the desendancy checks to properly verify that HEAD is a descendant of the commit to be rewritten. Also add some tests for this. - Fix the hint that mentions that lines starting with the comment character will be tripped after having written the commit message. - Move an include to the correct commit. - Link to v4: https://lore.kernel.org/r/20251001-b4-pks-history-builtin-v4-0-8e61ddb86317@pks.im Changes in v4: - I've rebuilt the patch series. It is now based on 821f583da6 (The thirteenth batcn, 2025-09-29) with sa/replay-atomic-ref-updates at 665c66a743 (replay: make atomic ref updates the default behavior, 2025-09-27) merged into it. This should fix all conflicts with seen. - I've reworked this patch series to use the same infra as git-replay(1), as discussed during the Contributor's Summit. - I've slimmed down the patch series to only tackle those commands that cannot result in a conflict to keep it simple. I also learned that Elijah has been working on a "git replay edit" command, so I dropped that command so that we can instead use his version. - During the Contributor's Summit we have agreed that for now, we won't care about hook execution just yet. This may be backfilled at a later point in time. - I dropped "commit.verbose" handling for now, as my understanding of it was wrong at first. This is something we should backfill. - Link to v3: https://lore.kernel.org/r/20250904-b4-pks-history-builtin-v3-0-509053514755@pks.im Changes in v3: - Add logic to drive the "post-rewrite" hook and add tests to verify that all hooks are executed as expected. - Deduplicate logic to turn a replay action into a todo command. - Move the addition of tests for the top-level git-history(1) command to the correct commit. - Some smaller commit message fixes. - Honor "commit.verbose". - Fix copy-paste error with an error message. - Link to v2: https://lore.kernel.org/r/20250824-b4-pks-history-builtin-v2-0-964ac12f65bd@pks.im Changes in v2: - Add a new "reword" subcommand. - List git-history(1) in "command-list.txt". - Add some missing error handling. - Simplify calling convention of `apply_commits()` to handle root commits internally instead of requiring every caller to do so. - Add tests to verify that git-history(1) refuses to work with changes in the worktree or index. - Mark git-history(1) as experimental. - Introduce commands to manage interrupted history edits. - A bunch of improvements to the manpage. - Link to v1: https://lore.kernel.org/r/20250819-b4-pks-history-builtin-v1-0-9b77c32688fe@pks.im Thanks! Patrick To: git@vger.kernel.org Cc: "D. Ben Knoble" Cc: Junio C Hamano Cc: Sergey Organov Cc: Jean-Noël AVILA Cc: Martin von Zweigbergk Cc: Kristoffer Haugsbakk Cc: Elijah Newren Cc: Karthik Nayak --- b4-submit-tracking --- # This section is used internally by b4 prep for tracking purposes. { "series": { "revision": 7, "change-id": "20250819-b4-pks-history-builtin-83398f9a05f0", "prefixes": [], "history": { "v1": [ "20250819-b4-pks-history-builtin-v1-0-9b77c32688fe@pks.im" ], "v2": [ "20250824-b4-pks-history-builtin-v2-0-964ac12f65bd@pks.im" ], "v3": [ "20250904-b4-pks-history-builtin-v3-0-509053514755@pks.im" ], "v4": [ "20251001-b4-pks-history-builtin-v4-0-8e61ddb86317@pks.im" ], "v5": [ "20251021-b4-pks-history-builtin-v5-0-78d23f578fe6@pks.im" ], "v6": [ "20251027-b4-pks-history-builtin-v6-0-407dd3f57ad3@pks.im" ] } } } -- GitLab From ccf19f9067d4788674a13eadf267dd6782358c1c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Sun, 24 Aug 2025 16:42:46 +0200 Subject: [PATCH 03/14] wt-status: provide function to expose status for trees The "wt-status" subsystem is responsible for printing status information around the current state of the working tree. This most importantly includes information around whether the working tree or the index have any changes. We're about to introduce a new command where the changes in neither of them are actually relevant to us. Instead, what we want is to format the changes between two different trees. While it is a little bit of a stretch to add this as functionality to _working tree_ status, it doesn't make any sense to open-code this functionality, either. Implement a new function `wt_status_collect_changes_trees()` that diffs two trees and formats the status accordingly. This function is not yet used, but will be in a subsequent commit. Signed-off-by: Patrick Steinhardt --- wt-status.c | 24 ++++++++++++++++++++++++ wt-status.h | 9 +++++++++ 2 files changed, 33 insertions(+) diff --git a/wt-status.c b/wt-status.c index e12adb26b9..95942399f8 100644 --- a/wt-status.c +++ b/wt-status.c @@ -612,6 +612,30 @@ static void wt_status_collect_updated_cb(struct diff_queue_struct *q, } } +void wt_status_collect_changes_trees(struct wt_status *s, + const struct object_id *old_treeish, + const struct object_id *new_treeish) +{ + struct diff_options opts = { 0 }; + + repo_diff_setup(s->repo, &opts); + opts.output_format = DIFF_FORMAT_CALLBACK; + opts.format_callback = wt_status_collect_updated_cb; + opts.format_callback_data = s; + opts.detect_rename = s->detect_rename >= 0 ? s->detect_rename : opts.detect_rename; + opts.rename_limit = s->rename_limit >= 0 ? s->rename_limit : opts.rename_limit; + opts.rename_score = s->rename_score >= 0 ? s->rename_score : opts.rename_score; + opts.flags.recursive = 1; + diff_setup_done(&opts); + + diff_tree_oid(old_treeish, new_treeish, "", &opts); + diffcore_std(&opts); + diff_flush(&opts); + wt_status_get_state(s->repo, &s->state, 0); + + diff_free(&opts); +} + static void wt_status_collect_changes_worktree(struct wt_status *s) { struct rev_info rev; diff --git a/wt-status.h b/wt-status.h index e40a27214a..e9fe32e98c 100644 --- a/wt-status.h +++ b/wt-status.h @@ -153,6 +153,15 @@ void wt_status_add_cut_line(struct wt_status *s); void wt_status_prepare(struct repository *r, struct wt_status *s); void wt_status_print(struct wt_status *s); void wt_status_collect(struct wt_status *s); + +/* + * Collect all changes between the two trees. Changes will be displayed as if + * they were staged into the index. + */ +void wt_status_collect_changes_trees(struct wt_status *s, + const struct object_id *old_treeish, + const struct object_id *new_treeish); + /* * Frees the buffers allocated by wt_status_collect. */ -- GitLab From 39c5a05ee1be8085ca263f59712c7b4f88283218 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 2 Dec 2025 21:09:08 +0100 Subject: [PATCH 04/14] replay: extract logic to pick commits We're about to add a new git-history(1) command that will reuse some of the same infrastructure as git-replay(1). To prepare for this, extract the logic to pick a commit into a new "replay.c" file so that it can be shared between both commands. Rename the function to have a "replay_" prefix to clearly indicate its subsystem. Signed-off-by: Patrick Steinhardt --- Makefile | 1 + builtin/replay.c | 110 ++------------------------------------------- meson.build | 1 + replay.c | 115 +++++++++++++++++++++++++++++++++++++++++++++++ replay.h | 23 ++++++++++ 5 files changed, 143 insertions(+), 107 deletions(-) create mode 100644 replay.c create mode 100644 replay.h diff --git a/Makefile b/Makefile index 237b56fc9d..809b094595 100644 --- a/Makefile +++ b/Makefile @@ -1274,6 +1274,7 @@ LIB_OBJS += repack-geometry.o LIB_OBJS += repack-midx.o LIB_OBJS += repack-promisor.o LIB_OBJS += replace-object.o +LIB_OBJS += replay.o LIB_OBJS += repo-settings.o LIB_OBJS += repository.o LIB_OBJS += rerere.o diff --git a/builtin/replay.c b/builtin/replay.c index 1f9ffd2b3e..f974a8c963 100644 --- a/builtin/replay.c +++ b/builtin/replay.c @@ -2,7 +2,6 @@ * "git replay" builtin command */ -#define USE_THE_REPOSITORY_VARIABLE #define DISABLE_SIGN_COMPARE_WARNINGS #include "git-compat-util.h" @@ -16,6 +15,7 @@ #include "object-name.h" #include "parse-options.h" #include "refs.h" +#include "replay.h" #include "revision.h" #include "strmap.h" #include @@ -26,13 +26,6 @@ enum ref_action_mode { REF_ACTION_PRINT, }; -static const char *short_commit_name(struct repository *repo, - struct commit *commit) -{ - return repo_find_unique_abbrev(repo, &commit->object.oid, - DEFAULT_ABBREV); -} - static struct commit *peel_committish(struct repository *repo, const char *name) { struct object *obj; @@ -45,59 +38,6 @@ static struct commit *peel_committish(struct repository *repo, const char *name) OBJ_COMMIT); } -static char *get_author(const char *message) -{ - size_t len; - const char *a; - - a = find_commit_header(message, "author", &len); - if (a) - return xmemdupz(a, len); - - return NULL; -} - -static struct commit *create_commit(struct repository *repo, - struct tree *tree, - struct commit *based_on, - struct commit *parent) -{ - struct object_id ret; - struct object *obj = NULL; - struct commit_list *parents = NULL; - char *author; - char *sign_commit = NULL; /* FIXME: cli users might want to sign again */ - struct commit_extra_header *extra = NULL; - struct strbuf msg = STRBUF_INIT; - const char *out_enc = get_commit_output_encoding(); - const char *message = repo_logmsg_reencode(repo, based_on, - NULL, out_enc); - const char *orig_message = NULL; - const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL }; - - commit_list_insert(parent, &parents); - extra = read_commit_extra_headers(based_on, exclude_gpgsig); - find_commit_subject(message, &orig_message); - strbuf_addstr(&msg, orig_message); - author = get_author(message); - reset_ident_date(); - if (commit_tree_extended(msg.buf, msg.len, &tree->object.oid, parents, - &ret, author, NULL, sign_commit, extra)) { - error(_("failed to write commit object")); - goto out; - } - - obj = parse_object(repo, &ret); - -out: - repo_unuse_commit_buffer(the_repository, based_on, message); - free_commit_extra_headers(extra); - free_commit_list(parents); - strbuf_release(&msg); - free(author); - return (struct commit *)obj; -} - struct ref_info { struct commit *onto; struct strset positive_refs; @@ -246,50 +186,6 @@ static void determine_replay_mode(struct repository *repo, strset_clear(&rinfo.positive_refs); } -static struct commit *mapped_commit(kh_oid_map_t *replayed_commits, - struct commit *commit, - struct commit *fallback) -{ - khint_t pos = kh_get_oid_map(replayed_commits, commit->object.oid); - if (pos == kh_end(replayed_commits)) - return fallback; - return kh_value(replayed_commits, pos); -} - -static struct commit *pick_regular_commit(struct repository *repo, - struct commit *pickme, - kh_oid_map_t *replayed_commits, - struct commit *onto, - struct merge_options *merge_opt, - struct merge_result *result) -{ - struct commit *base, *replayed_base; - struct tree *pickme_tree, *base_tree; - - base = pickme->parents->item; - replayed_base = mapped_commit(replayed_commits, base, onto); - - result->tree = repo_get_commit_tree(repo, replayed_base); - pickme_tree = repo_get_commit_tree(repo, pickme); - base_tree = repo_get_commit_tree(repo, base); - - merge_opt->branch1 = short_commit_name(repo, replayed_base); - merge_opt->branch2 = short_commit_name(repo, pickme); - merge_opt->ancestor = xstrfmt("parent of %s", merge_opt->branch2); - - merge_incore_nonrecursive(merge_opt, - base_tree, - result->tree, - pickme_tree, - result); - - free((char*)merge_opt->ancestor); - merge_opt->ancestor = NULL; - if (!result->clean) - return NULL; - return create_commit(repo, result->tree, pickme, replayed_base); -} - static enum ref_action_mode parse_ref_action_mode(const char *ref_action, const char *source) { if (!ref_action || !strcmp(ref_action, "update")) @@ -495,8 +391,8 @@ int cmd_replay(int argc, if (commit->parents->next) die(_("replaying merge commits is not supported yet!")); - last_commit = pick_regular_commit(repo, commit, replayed_commits, - onto, &merge_opt, &result); + last_commit = replay_pick_regular_commit(repo, commit, replayed_commits, + onto, &merge_opt, &result); if (!last_commit) break; diff --git a/meson.build b/meson.build index f1b3615659..1878dff2b1 100644 --- a/meson.build +++ b/meson.build @@ -470,6 +470,7 @@ libgit_sources = [ 'repack-midx.c', 'repack-promisor.c', 'replace-object.c', + 'replay.c', 'repo-settings.c', 'repository.c', 'rerere.c', diff --git a/replay.c b/replay.c new file mode 100644 index 0000000000..fb906e9b51 --- /dev/null +++ b/replay.c @@ -0,0 +1,115 @@ +#define USE_THE_REPOSITORY_VARIABLE + +#include "git-compat-util.h" +#include "commit.h" +#include "environment.h" +#include "gettext.h" +#include "ident.h" +#include "object.h" +#include "object-name.h" +#include "replay.h" +#include "tree.h" + +static const char *short_commit_name(struct repository *repo, + struct commit *commit) +{ + return repo_find_unique_abbrev(repo, &commit->object.oid, + DEFAULT_ABBREV); +} + +static char *get_author(const char *message) +{ + size_t len; + const char *a; + + a = find_commit_header(message, "author", &len); + if (a) + return xmemdupz(a, len); + + return NULL; +} + +struct commit *replay_create_commit(struct repository *repo, + struct tree *tree, + struct commit *based_on, + struct commit *parent) +{ + struct object_id ret; + struct object *obj = NULL; + struct commit_list *parents = NULL; + char *author; + char *sign_commit = NULL; /* FIXME: cli users might want to sign again */ + struct commit_extra_header *extra = NULL; + struct strbuf msg = STRBUF_INIT; + const char *out_enc = get_commit_output_encoding(); + const char *message = repo_logmsg_reencode(repo, based_on, + NULL, out_enc); + const char *orig_message = NULL; + const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL }; + + commit_list_insert(parent, &parents); + extra = read_commit_extra_headers(based_on, exclude_gpgsig); + find_commit_subject(message, &orig_message); + strbuf_addstr(&msg, orig_message); + author = get_author(message); + reset_ident_date(); + if (commit_tree_extended(msg.buf, msg.len, &tree->object.oid, parents, + &ret, author, NULL, sign_commit, extra)) { + error(_("failed to write commit object")); + goto out; + } + + obj = parse_object(repo, &ret); + +out: + repo_unuse_commit_buffer(the_repository, based_on, message); + free_commit_extra_headers(extra); + free_commit_list(parents); + strbuf_release(&msg); + free(author); + return (struct commit *)obj; +} + +static struct commit *mapped_commit(kh_oid_map_t *replayed_commits, + struct commit *commit, + struct commit *fallback) +{ + khint_t pos = kh_get_oid_map(replayed_commits, commit->object.oid); + if (pos == kh_end(replayed_commits)) + return fallback; + return kh_value(replayed_commits, pos); +} + +struct commit *replay_pick_regular_commit(struct repository *repo, + struct commit *pickme, + kh_oid_map_t *replayed_commits, + struct commit *onto, + struct merge_options *merge_opt, + struct merge_result *result) +{ + struct commit *base, *replayed_base; + struct tree *pickme_tree, *base_tree; + + base = pickme->parents->item; + replayed_base = mapped_commit(replayed_commits, base, onto); + + result->tree = repo_get_commit_tree(repo, replayed_base); + pickme_tree = repo_get_commit_tree(repo, pickme); + base_tree = repo_get_commit_tree(repo, base); + + merge_opt->branch1 = short_commit_name(repo, replayed_base); + merge_opt->branch2 = short_commit_name(repo, pickme); + merge_opt->ancestor = xstrfmt("parent of %s", merge_opt->branch2); + + merge_incore_nonrecursive(merge_opt, + base_tree, + result->tree, + pickme_tree, + result); + + free((char*)merge_opt->ancestor); + merge_opt->ancestor = NULL; + if (!result->clean) + return NULL; + return replay_create_commit(repo, result->tree, pickme, replayed_base); +} diff --git a/replay.h b/replay.h new file mode 100644 index 0000000000..d6535ee56c --- /dev/null +++ b/replay.h @@ -0,0 +1,23 @@ +#ifndef REPLAY_H +#define REPLAY_H + +#include "khash.h" +#include "merge-ort.h" +#include "repository.h" + +struct commit; +struct tree; + +struct commit *replay_create_commit(struct repository *repo, + struct tree *tree, + struct commit *based_on, + struct commit *parent); + +struct commit *replay_pick_regular_commit(struct repository *repo, + struct commit *pickme, + kh_oid_map_t *replayed_commits, + struct commit *onto, + struct merge_options *merge_opt, + struct merge_result *result); + +#endif -- GitLab From fe73ba40596c84961be1eb95d20f1e3f22b4acf4 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 1 Oct 2025 10:08:48 +0200 Subject: [PATCH 05/14] replay: stop using `the_repository` In `create_commit()` we're using `the_repository` even though we already have a repository passed to use as an argument. Fix this. Note that we still cannot get rid of `USE_THE_REPOSITORY_VARIABLE`. This is because we use `DEFAULT_ABBREV and `get_commit_output_encoding()`, both of which are stored as global variables that can be modified via the Git configuration. Signed-off-by: Patrick Steinhardt --- replay.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/replay.c b/replay.c index fb906e9b51..13983dbc56 100644 --- a/replay.c +++ b/replay.c @@ -62,7 +62,7 @@ struct commit *replay_create_commit(struct repository *repo, obj = parse_object(repo, &ret); out: - repo_unuse_commit_buffer(the_repository, based_on, message); + repo_unuse_commit_buffer(repo, based_on, message); free_commit_extra_headers(extra); free_commit_list(parents); strbuf_release(&msg); -- GitLab From c162f407dbd82f639bdeb2ebcecd16b6064efd04 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Mon, 11 Aug 2025 20:20:29 +0200 Subject: [PATCH 06/14] builtin: add new "history" command When rewriting history via git-rebase(1) there are a few very common use cases: - The ordering of two commits should be reversed. - A commit should be split up into two commits. - A commit should be dropped from the history completely. - Multiple commits should be squashed into one. - Editing an existing commit that is not the tip of the current branch. While these operations are all doable, it often feels needlessly kludgey to do so by doing an interactive rebase, using the editor to say what one wants, and then perform the actions. Furthermore, some operations like splitting up a commit into two are way more involved than that and require a whole series of commands. Add a new "history" command to plug this gap. This command will have several different subcommands to imperatively rewrite history for common use cases like the above. Some of these subcommands will be implemented in subsequent commits. Signed-off-by: Patrick Steinhardt --- .gitignore | 1 + Documentation/git-history.adoc | 42 ++++++++++++++++++++++++++++++++++ Documentation/meson.build | 1 + Makefile | 1 + builtin.h | 1 + builtin/history.c | 22 ++++++++++++++++++ command-list.txt | 1 + git.c | 1 + meson.build | 1 + t/meson.build | 1 + t/t3450-history.sh | 17 ++++++++++++++ 11 files changed, 89 insertions(+) create mode 100644 Documentation/git-history.adoc create mode 100644 builtin/history.c create mode 100755 t/t3450-history.sh diff --git a/.gitignore b/.gitignore index 78a45cb5be..24635cf2d6 100644 --- a/.gitignore +++ b/.gitignore @@ -79,6 +79,7 @@ /git-grep /git-hash-object /git-help +/git-history /git-hook /git-http-backend /git-http-fetch diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc new file mode 100644 index 0000000000..67b8ce2040 --- /dev/null +++ b/Documentation/git-history.adoc @@ -0,0 +1,42 @@ +git-history(1) +============== + +NAME +---- +git-history - EXPERIMENTAL: Rewrite history of the current branch + +SYNOPSIS +-------- +[synopsis] +git history [] + +DESCRIPTION +----------- + +Rewrite history by rearranging or modifying specific commits in the +history. + +THIS COMMAND IS EXPERIMENTAL. THE BEHAVIOR MAY CHANGE. + +This command is related to linkgit:git-rebase[1] in that both commands can be +used to rewrite history. You should use rebases if you want to reapply a range +of commits onto a different base, or interactive rebases if you want to edit a +range of commits. + +Note that this command does not (yet) work with histories that contain +merges. You should use linkgit:git-rebase[1] with the `--rebase-merges` +flag instead. + +COMMANDS +-------- + +Several commands are available to rewrite history in different ways: + +CONFIGURATION +------------- + +include::includes/cmd-config-section-all.adoc[] + +GIT +--- +Part of the linkgit:git[1] suite diff --git a/Documentation/meson.build b/Documentation/meson.build index f02dbc20cb..fd2e8cc02d 100644 --- a/Documentation/meson.build +++ b/Documentation/meson.build @@ -64,6 +64,7 @@ manpages = { 'git-gui.adoc' : 1, 'git-hash-object.adoc' : 1, 'git-help.adoc' : 1, + 'git-history.adoc' : 1, 'git-hook.adoc' : 1, 'git-http-backend.adoc' : 1, 'git-http-fetch.adoc' : 1, diff --git a/Makefile b/Makefile index 809b094595..458841178b 100644 --- a/Makefile +++ b/Makefile @@ -1408,6 +1408,7 @@ BUILTIN_OBJS += builtin/get-tar-commit-id.o BUILTIN_OBJS += builtin/grep.o BUILTIN_OBJS += builtin/hash-object.o BUILTIN_OBJS += builtin/help.o +BUILTIN_OBJS += builtin/history.o BUILTIN_OBJS += builtin/hook.o BUILTIN_OBJS += builtin/index-pack.o BUILTIN_OBJS += builtin/init-db.o diff --git a/builtin.h b/builtin.h index 1b35565fbd..93c91d07d4 100644 --- a/builtin.h +++ b/builtin.h @@ -172,6 +172,7 @@ int cmd_get_tar_commit_id(int argc, const char **argv, const char *prefix, struc int cmd_grep(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_hash_object(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_help(int argc, const char **argv, const char *prefix, struct repository *repo); +int cmd_history(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_hook(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_index_pack(int argc, const char **argv, const char *prefix, struct repository *repo); int cmd_init_db(int argc, const char **argv, const char *prefix, struct repository *repo); diff --git a/builtin/history.c b/builtin/history.c new file mode 100644 index 0000000000..f6fe32610b --- /dev/null +++ b/builtin/history.c @@ -0,0 +1,22 @@ +#include "builtin.h" +#include "gettext.h" +#include "parse-options.h" + +int cmd_history(int argc, + const char **argv, + const char *prefix, + struct repository *repo UNUSED) +{ + const char * const usage[] = { + N_("git history []"), + NULL, + }; + struct option options[] = { + OPT_END(), + }; + + argc = parse_options(argc, argv, prefix, options, usage, 0); + if (argc) + usagef("unrecognized argument: %s", argv[0]); + return 0; +} diff --git a/command-list.txt b/command-list.txt index accd3d0c4b..f9005cf459 100644 --- a/command-list.txt +++ b/command-list.txt @@ -115,6 +115,7 @@ git-grep mainporcelain info git-gui mainporcelain git-hash-object plumbingmanipulators git-help ancillaryinterrogators complete +git-history mainporcelain history git-hook purehelpers git-http-backend synchingrepositories git-http-fetch synchelpers diff --git a/git.c b/git.c index c5fad56813..744cb6527e 100644 --- a/git.c +++ b/git.c @@ -586,6 +586,7 @@ static struct cmd_struct commands[] = { { "grep", cmd_grep, RUN_SETUP_GENTLY }, { "hash-object", cmd_hash_object }, { "help", cmd_help }, + { "history", cmd_history, RUN_SETUP }, { "hook", cmd_hook, RUN_SETUP }, { "index-pack", cmd_index_pack, RUN_SETUP_GENTLY | NO_PARSEOPT }, { "init", cmd_init_db }, diff --git a/meson.build b/meson.build index 1878dff2b1..f07a308fb8 100644 --- a/meson.build +++ b/meson.build @@ -610,6 +610,7 @@ builtin_sources = [ 'builtin/grep.c', 'builtin/hash-object.c', 'builtin/help.c', + 'builtin/history.c', 'builtin/hook.c', 'builtin/index-pack.c', 'builtin/init-db.c', diff --git a/t/meson.build b/t/meson.build index 7c994d4643..62f5dca098 100644 --- a/t/meson.build +++ b/t/meson.build @@ -386,6 +386,7 @@ integration_tests = [ 't3436-rebase-more-options.sh', 't3437-rebase-fixup-options.sh', 't3438-rebase-broken-files.sh', + 't3450-history.sh', 't3500-cherry.sh', 't3501-revert-cherry-pick.sh', 't3502-cherry-pick-merge.sh', diff --git a/t/t3450-history.sh b/t/t3450-history.sh new file mode 100755 index 0000000000..417c343d43 --- /dev/null +++ b/t/t3450-history.sh @@ -0,0 +1,17 @@ +#!/bin/sh + +test_description='tests for git-history command' + +. ./test-lib.sh + +test_expect_success 'does nothing without any arguments' ' + git history >out 2>&1 && + test_must_be_empty out +' + +test_expect_success 'raises an error with unknown argument' ' + test_must_fail git history garbage 2>err && + test_grep "unrecognized argument: garbage" err +' + +test_done -- GitLab From 3ecbe6ec8317161ddf5c37da7e606339637719e5 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 1 Oct 2025 14:58:19 +0200 Subject: [PATCH 07/14] builtin/history: implement "reword" subcommand Implement a new "reword" subcommand for git-history(1). This subcommand is similar to the user performing an interactive rebase with a single commit changed to use the "reword" instruction. The major difference is that we do not check out the commit that is to be reworded. This has the obvious benefit of being significantly faster compared to git-rebase(1), but even more importantly it allows the user to rewrite history even if there are local changes in the working tree or in the index. Signed-off-by: Patrick Steinhardt --- Documentation/git-history.adoc | 7 +- builtin/history.c | 372 ++++++++++++++++++++++++++++++++- t/meson.build | 1 + t/t3450-history.sh | 6 +- t/t3451-history-reword.sh | 236 +++++++++++++++++++++ 5 files changed, 613 insertions(+), 9 deletions(-) create mode 100755 t/t3451-history-reword.sh diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc index 67b8ce2040..160bf5d4d2 100644 --- a/Documentation/git-history.adoc +++ b/Documentation/git-history.adoc @@ -8,7 +8,7 @@ git-history - EXPERIMENTAL: Rewrite history of the current branch SYNOPSIS -------- [synopsis] -git history [] +git history reword DESCRIPTION ----------- @@ -32,6 +32,11 @@ COMMANDS Several commands are available to rewrite history in different ways: +`reword `:: + Rewrite the commit message of the specified commit. All the other + details of this commit remain unchanged. This command will spawn an + editor with the current message of that commit. + CONFIGURATION ------------- diff --git a/builtin/history.c b/builtin/history.c index f6fe32610b..17bb150b95 100644 --- a/builtin/history.c +++ b/builtin/history.c @@ -1,22 +1,384 @@ +#define USE_THE_REPOSITORY_VARIABLE + #include "builtin.h" +#include "commit-reach.h" +#include "commit.h" +#include "config.h" +#include "editor.h" +#include "environment.h" #include "gettext.h" +#include "hex.h" #include "parse-options.h" +#include "refs.h" +#include "replay.h" +#include "reset.h" +#include "revision.h" +#include "sequencer.h" +#include "strvec.h" +#include "tree.h" +#include "wt-status.h" + +#define GIT_HISTORY_REWORD_USAGE N_("git history reword ") + +static int collect_commits(struct repository *repo, + struct commit *old_commit, + struct commit *new_commit, + struct strvec *out) +{ + struct setup_revision_opt revision_opts = { + .assume_dashdash = 1, + }; + struct strvec revisions = STRVEC_INIT; + struct commit *child; + struct rev_info rev = { 0 }; + int ret; + + repo_init_revisions(repo, &rev, NULL); + rev.reverse = 1; + strvec_push(&revisions, ""); + strvec_push(&revisions, oid_to_hex(&new_commit->object.oid)); + if (old_commit) { + strvec_pushf(&revisions, "^%s", oid_to_hex(&old_commit->object.oid)); + strvec_pushf(&revisions, "--ancestry-path=%s", oid_to_hex(&old_commit->object.oid)); + } + + setup_revisions_from_strvec(&revisions, &rev, &revision_opts); + if (revisions.nr != 1 || prepare_revision_walk(&rev)) { + ret = error(_("revision walk setup failed")); + goto out; + } + + while ((child = get_revision(&rev))) { + if (old_commit && !child->parents) + BUG("revision walk did not find child commit"); + if (child->parents && child->parents->next) { + ret = error(_("cannot rearrange commit history with merges")); + goto out; + } + + strvec_push(out, oid_to_hex(&child->object.oid)); + } + + ret = 0; + +out: + strvec_clear(&revisions); + release_revisions(&rev); + reset_revision_walk(); + return ret; +} + +static int gather_commits_between_head_and_revision(struct repository *repo, + const char *revision, + struct commit **original_commit, + struct commit **parent_commit, + struct commit **head, + struct strvec *commits) +{ + struct commit_list *from_list = NULL; + int ret; + + *original_commit = lookup_commit_reference_by_name(revision); + if (!*original_commit) { + ret = error(_("commit cannot be found: %s"), revision); + goto out; + } + + *parent_commit = (*original_commit)->parents ? (*original_commit)->parents->item : NULL; + if (*parent_commit && repo_parse_commit(repo, *parent_commit)) { + ret = error(_("unable to parse commit %s"), + oid_to_hex(&(*parent_commit)->object.oid)); + goto out; + } + + *head = lookup_commit_reference_by_name("HEAD"); + if (!(*head)) { + ret = error(_("could not resolve HEAD to a commit")); + goto out; + } + + commit_list_append(*original_commit, &from_list); + if (!repo_is_descendant_of(repo, *head, from_list)) { + ret = error(_("commit must be reachable from current HEAD commit")); + goto out; + } + + /* + * Collect the list of commits that we'll have to reapply now already. + * This ensures that we'll abort early on in case the range of commits + * contains merges, which we do not yet handle. + */ + ret = collect_commits(repo, *parent_commit, *head, commits); + if (ret < 0) + goto out; + +out: + free_commit_list(from_list); + return ret; +} + +static void replace_commits(struct strvec *commits, + const struct object_id *commit_to_replace, + const struct object_id *replacements, + size_t replacements_nr) +{ + char commit_to_replace_oid[GIT_MAX_HEXSZ + 1]; + struct strvec replacement_oids = STRVEC_INIT; + bool found = false; + + oid_to_hex_r(commit_to_replace_oid, commit_to_replace); + for (size_t i = 0; i < replacements_nr; i++) + strvec_push(&replacement_oids, oid_to_hex(&replacements[i])); + + for (size_t i = 0; i < commits->nr; i++) { + if (strcmp(commits->v[i], commit_to_replace_oid)) + continue; + strvec_splice(commits, i, 1, replacement_oids.v, replacement_oids.nr); + found = true; + break; + } + if (!found) + BUG("could not find commit to replace"); + + strvec_clear(&replacement_oids); +} + +static int apply_commits(struct repository *repo, + const struct strvec *commits, + struct commit *onto, + struct commit *orig_head, + const char *action) +{ + struct reset_head_opts reset_opts = { 0 }; + struct strbuf buf = STRBUF_INIT; + int ret; + + for (size_t i = 0; i < commits->nr; i++) { + struct object_id commit_id; + struct commit *commit; + const char *end; + + if (parse_oid_hex_algop(commits->v[i], &commit_id, &end, + repo->hash_algo)) { + ret = error(_("invalid object ID: %s"), commits->v[i]); + goto out; + } + + commit = lookup_commit(repo, &commit_id); + if (!commit || repo_parse_commit(repo, commit)) { + ret = error(_("failed to look up commit: %s"), oid_to_hex(&commit_id)); + goto out; + } + + if (!onto) { + onto = commit; + } else { + struct tree *tree = repo_get_commit_tree(repo, commit); + onto = replay_create_commit(repo, tree, commit, onto); + if (!onto) { + ret = -1; + goto out; + } + } + } + + reset_opts.oid = &onto->object.oid; + strbuf_addf(&buf, "%s: switch to rewritten %s", action, oid_to_hex(reset_opts.oid)); + reset_opts.flags = RESET_HEAD_REFS_ONLY | RESET_ORIG_HEAD; + reset_opts.orig_head = &orig_head->object.oid; + reset_opts.default_reflog_action = action; + if (reset_head(repo, &reset_opts) < 0) { + ret = error(_("could not switch to %s"), oid_to_hex(reset_opts.oid)); + goto out; + } + + ret = 0; + +out: + strbuf_release(&buf); + return ret; +} + +static void change_data_free(void *util, const char *str UNUSED) +{ + struct wt_status_change_data *d = util; + free(d->rename_source); + free(d); +} + +static int fill_commit_message(struct repository *repo, + const struct object_id *old_tree, + const struct object_id *new_tree, + const char *default_message, + const char *action, + struct strbuf *out) +{ + const char *path = git_path_commit_editmsg(); + const char *hint = + _("Please enter the commit message for the %s changes." + " Lines starting\nwith '%s' will be ignored, and an" + " empty message aborts the commit.\n"); + struct wt_status s; + + strbuf_addstr(out, default_message); + strbuf_addch(out, '\n'); + strbuf_commented_addf(out, comment_line_str, hint, action, comment_line_str); + write_file_buf(path, out->buf, out->len); + + wt_status_prepare(repo, &s); + FREE_AND_NULL(s.branch); + s.ahead_behind_flags = AHEAD_BEHIND_QUICK; + s.commit_template = 1; + s.colopts = 0; + s.display_comment_prefix = 1; + s.hints = 0; + s.use_color = 0; + s.whence = FROM_COMMIT; + s.committable = 1; + + s.fp = fopen(git_path_commit_editmsg(), "a"); + if (!s.fp) + return error_errno(_("could not open '%s'"), git_path_commit_editmsg()); + + wt_status_collect_changes_trees(&s, old_tree, new_tree); + wt_status_print(&s); + wt_status_collect_free_buffers(&s); + string_list_clear_func(&s.change, change_data_free); + + strbuf_reset(out); + if (launch_editor(path, out, NULL)) { + fprintf(stderr, _("Aborting commit as launching the editor failed.\n")); + return -1; + } + strbuf_stripspace(out, comment_line_str); + + cleanup_message(out, COMMIT_MSG_CLEANUP_ALL, 0); + + if (!out->len) { + fprintf(stderr, _("Aborting commit due to empty commit message.\n")); + return -1; + } + + return 0; +} + +static int commit_tree_with_edited_message(struct repository *repo, + const char *action, + struct commit *original_commit, + const struct object_id *new_tree_oid, + const struct commit_list *parents, + const struct object_id *parent_tree_oid, + struct object_id *out) +{ + const char *exclude_gpgsig[] = { "gpgsig", "gpgsig-sha256", NULL }; + const char *original_message, *original_body, *ptr; + struct commit_extra_header *original_extra_headers = NULL; + struct strbuf commit_message = STRBUF_INIT; + char *original_author = NULL; + size_t len; + int ret; + + /* We retain authorship of the original commit. */ + original_message = repo_logmsg_reencode(repo, original_commit, NULL, NULL); + ptr = find_commit_header(original_message, "author", &len); + if (ptr) + original_author = xmemdupz(ptr, len); + find_commit_subject(original_message, &original_body); + + ret = fill_commit_message(repo, parent_tree_oid, new_tree_oid, + original_body, action, &commit_message); + if (ret < 0) + goto out; + + original_extra_headers = read_commit_extra_headers(original_commit, exclude_gpgsig); + + ret = commit_tree_extended(commit_message.buf, commit_message.len, new_tree_oid, + parents, out, original_author, NULL, NULL, + original_extra_headers); + if (ret < 0) + goto out; + +out: + free_commit_extra_headers(original_extra_headers); + strbuf_release(&commit_message); + free(original_author); + return ret; +} + +static int cmd_history_reword(int argc, + const char **argv, + const char *prefix, + struct repository *repo) +{ + const char * const usage[] = { + GIT_HISTORY_REWORD_USAGE, + NULL, + }; + struct option options[] = { + OPT_END(), + }; + struct commit *original_commit, *parent, *head; + struct strvec commits = STRVEC_INIT; + struct object_id parent_tree_oid, original_commit_tree_oid; + struct object_id rewritten_commit; + int ret; + + argc = parse_options(argc, argv, prefix, options, usage, 0); + if (argc != 1) { + ret = error(_("command expects a single revision")); + goto out; + } + repo_config(repo, git_default_config, NULL); + + ret = gather_commits_between_head_and_revision(repo, argv[0], &original_commit, + &parent, &head, &commits); + if (ret < 0) + goto out; + + original_commit_tree_oid = repo_get_commit_tree(repo, original_commit)->object.oid; + if (parent) + parent_tree_oid = repo_get_commit_tree(repo, parent)->object.oid; + else + oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree); + + /* We retain authorship of the original commit. */ + ret = commit_tree_with_edited_message(repo, "reworded", original_commit, + &original_commit_tree_oid, + original_commit->parents, &parent_tree_oid, + &rewritten_commit); + if (ret < 0) { + ret = error(_("failed writing reworded commit")); + goto out; + } + + replace_commits(&commits, &original_commit->object.oid, &rewritten_commit, 1); + + ret = apply_commits(repo, &commits, parent, head, "reword"); + if (ret < 0) + goto out; + + ret = 0; + +out: + strvec_clear(&commits); + return ret; +} int cmd_history(int argc, const char **argv, const char *prefix, - struct repository *repo UNUSED) + struct repository *repo) { const char * const usage[] = { - N_("git history []"), + GIT_HISTORY_REWORD_USAGE, NULL, }; + parse_opt_subcommand_fn *fn = NULL; struct option options[] = { + OPT_SUBCOMMAND("reword", &fn, cmd_history_reword), OPT_END(), }; argc = parse_options(argc, argv, prefix, options, usage, 0); - if (argc) - usagef("unrecognized argument: %s", argv[0]); - return 0; + return fn(argc, argv, prefix, repo); } diff --git a/t/meson.build b/t/meson.build index 62f5dca098..e187aef31e 100644 --- a/t/meson.build +++ b/t/meson.build @@ -387,6 +387,7 @@ integration_tests = [ 't3437-rebase-fixup-options.sh', 't3438-rebase-broken-files.sh', 't3450-history.sh', + 't3451-history-reword.sh', 't3500-cherry.sh', 't3501-revert-cherry-pick.sh', 't3502-cherry-pick-merge.sh', diff --git a/t/t3450-history.sh b/t/t3450-history.sh index 417c343d43..f513463b92 100755 --- a/t/t3450-history.sh +++ b/t/t3450-history.sh @@ -5,13 +5,13 @@ test_description='tests for git-history command' . ./test-lib.sh test_expect_success 'does nothing without any arguments' ' - git history >out 2>&1 && - test_must_be_empty out + test_must_fail git history 2>err && + test_grep "need a subcommand" err ' test_expect_success 'raises an error with unknown argument' ' test_must_fail git history garbage 2>err && - test_grep "unrecognized argument: garbage" err + test_grep "unknown subcommand: .garbage." err ' test_done diff --git a/t/t3451-history-reword.sh b/t/t3451-history-reword.sh new file mode 100755 index 0000000000..2a638d2378 --- /dev/null +++ b/t/t3451-history-reword.sh @@ -0,0 +1,236 @@ +#!/bin/sh + +test_description='tests for git-history reword subcommand' + +. ./test-lib.sh + +reword_with_message () { + cat >message && + write_script fake-editor.sh <<-\EOF && + cp message "$1" + EOF + test_set_editor "$(pwd)"/fake-editor.sh && + git history reword "$@" && + rm fake-editor.sh message +} + +test_expect_success 'refuses to work with merge commits' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit base && + git branch branch && + test_commit ours && + git switch branch && + test_commit theirs && + git switch - && + git merge theirs && + test_must_fail git history reword HEAD~ 2>err && + test_grep "cannot rearrange commit history with merges" err && + test_must_fail git history reword HEAD 2>err && + test_grep "cannot rearrange commit history with merges" err + ) +' + +test_expect_success 'refuses to work with unrelated commits' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit base && + git branch branch && + test_commit ours && + git switch branch && + test_commit theirs && + test_must_fail git history reword ours 2>err && + test_grep "commit must be reachable from current HEAD commit" err + ) +' + +test_expect_success 'can reword tip of a branch' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit first && + test_commit second && + test_commit third && + + git symbolic-ref HEAD >expect && + reword_with_message HEAD <<-EOF && + third reworded + EOF + git symbolic-ref HEAD >actual && + test_cmp expect actual && + + cat >expect <<-EOF && + third reworded + second + first + EOF + git log --format=%s >actual && + test_cmp expect actual && + + git reflog >reflog && + test_grep "reword: updating HEAD" reflog + ) +' + +test_expect_success 'can reword commit in the middle' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit first && + test_commit second && + test_commit third && + + git symbolic-ref HEAD >expect && + reword_with_message HEAD~ <<-EOF && + second reworded + EOF + git symbolic-ref HEAD >actual && + test_cmp expect actual && + + cat >expect <<-EOF && + third + second reworded + first + EOF + git log --format=%s >actual && + test_cmp expect actual + ) +' + +test_expect_success 'can reword root commit' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit first && + test_commit second && + test_commit third && + reword_with_message HEAD~2 <<-EOF && + first reworded + EOF + + cat >expect <<-EOF && + third + second + first reworded + EOF + git log --format=%s >actual && + test_cmp expect actual + ) +' + +test_expect_success 'editor shows proper status' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit first && + + write_script fake-editor.sh <<-\EOF && + cp "$1" . && + printf "\namend a comment\n" >>"$1" + EOF + test_set_editor "$(pwd)"/fake-editor.sh && + git history reword HEAD && + + cat >expect <<-EOF && + first + + # Please enter the commit message for the reworded changes. Lines starting + # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit. + # Changes to be committed: + # new file: first.t + # + EOF + test_cmp expect COMMIT_EDITMSG && + + test_commit_message HEAD <<-\EOF + first + + amend a comment + EOF + ) +' + +# For now, git-history(1) does not yet execute any hooks. This is subject to +# change in the future, and if it does this test here is expected to start +# failing. In other words, this test is not an endorsement of the current +# status quo. +test_expect_success 'hooks are not executed for rewritten commits' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit first && + test_commit second && + test_commit third && + + ORIG_PATH="$(pwd)" && + export ORIG_PATH && + for hook in prepare-commit-msg pre-commit post-commit post-rewrite commit-msg + do + write_script .git/hooks/$hook <<-\EOF || exit 1 + touch "$ORIG_PATH/hooks.log + EOF + done && + + reword_with_message HEAD~ <<-EOF && + second reworded + EOF + + cat >expect <<-EOF && + third + second reworded + first + EOF + git log --format=%s >actual && + test_cmp expect actual && + + test_path_is_missing hooks.log + ) +' + +test_expect_success 'aborts with empty commit message' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit first && + + ! reword_with_message HEAD 2>err a && + echo bar >b && + git add b && + reword_with_message HEAD <<-EOF && + message + EOF + cat >expect <<-\EOF && + M a + M b + ?? actual + ?? expect + EOF + git status --porcelain >actual && + test_cmp expect actual + ) +' + +test_done -- GitLab From a1c6ad546d7e0e90bd18351407f0b7eadc4a7d95 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 14 Aug 2025 08:30:28 +0200 Subject: [PATCH 08/14] add-patch: split out header from "add-interactive.h" While we have a "add-patch.c" code file, its declarations are part of "add-interactive.h". This makes it somewhat harder than necessary to find relevant code and to identify clear boundaries between the two subsystems. Split up concerns and move declarations that relate to "add-patch.c" into a new "add-patch.h" header. Signed-off-by: Patrick Steinhardt --- add-interactive.h | 23 +++-------------------- add-patch.c | 1 + add-patch.h | 26 ++++++++++++++++++++++++++ 3 files changed, 30 insertions(+), 20 deletions(-) create mode 100644 add-patch.h diff --git a/add-interactive.h b/add-interactive.h index da49502b76..2e3d1d871d 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -1,14 +1,11 @@ #ifndef ADD_INTERACTIVE_H #define ADD_INTERACTIVE_H +#include "add-patch.h" #include "color.h" -struct add_p_opt { - int context; - int interhunkcontext; -}; - -#define ADD_P_OPT_INIT { .context = -1, .interhunkcontext = -1 } +struct pathspec; +struct repository; struct add_i_state { struct repository *r; @@ -35,21 +32,7 @@ void init_add_i_state(struct add_i_state *s, struct repository *r, struct add_p_opt *add_p_opt); void clear_add_i_state(struct add_i_state *s); -struct repository; -struct pathspec; int run_add_i(struct repository *r, const struct pathspec *ps, struct add_p_opt *add_p_opt); -enum add_p_mode { - ADD_P_ADD, - ADD_P_STASH, - ADD_P_RESET, - ADD_P_CHECKOUT, - ADD_P_WORKTREE, -}; - -int run_add_p(struct repository *r, enum add_p_mode mode, - struct add_p_opt *o, const char *revision, - const struct pathspec *ps); - #endif diff --git a/add-patch.c b/add-patch.c index 173a53241e..5e3481083b 100644 --- a/add-patch.c +++ b/add-patch.c @@ -3,6 +3,7 @@ #include "git-compat-util.h" #include "add-interactive.h" +#include "add-patch.h" #include "advice.h" #include "editor.h" #include "environment.h" diff --git a/add-patch.h b/add-patch.h new file mode 100644 index 0000000000..4394c74107 --- /dev/null +++ b/add-patch.h @@ -0,0 +1,26 @@ +#ifndef ADD_PATCH_H +#define ADD_PATCH_H + +struct pathspec; +struct repository; + +struct add_p_opt { + int context; + int interhunkcontext; +}; + +#define ADD_P_OPT_INIT { .context = -1, .interhunkcontext = -1 } + +enum add_p_mode { + ADD_P_ADD, + ADD_P_STASH, + ADD_P_RESET, + ADD_P_CHECKOUT, + ADD_P_WORKTREE, +}; + +int run_add_p(struct repository *r, enum add_p_mode mode, + struct add_p_opt *o, const char *revision, + const struct pathspec *ps); + +#endif -- GitLab From eea2ac7a18486e243c7c0d5f4d5afd0642fc9d94 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 14 Aug 2025 09:05:06 +0200 Subject: [PATCH 09/14] add-patch: split out `struct interactive_options` The `struct add_p_opt` is reused both by our infra for "git add -p" and "git add -i". Users of `run_add_i()` for example are expected to pass `struct add_p_opt`. This is somewhat confusing and raises the question of which options apply to what part of the stack. But things are even more confusing than that: while callers are expected to pass in `struct add_p_opt`, these options ultimately get used to initialize a `struct add_i_state` that is used by both subsystems. So we are basically going full circle here. Refactor the code and split out a new `struct interactive_options` that hosts common options used by both. These options are then applied to a `struct interactive_config` that hosts common configuration. This refactoring doesn't yet fully detangle the two subsystems from one another, as we still end up calling `init_add_i_state()` in the "git add -p" subsystem. This will be fixed in a subsequent commit. Signed-off-by: Patrick Steinhardt --- add-interactive.c | 174 ++++++++++----------------------------------- add-interactive.h | 23 +----- add-patch.c | 170 ++++++++++++++++++++++++++++++++++++------- add-patch.h | 36 +++++++++- builtin/add.c | 22 +++--- builtin/checkout.c | 4 +- builtin/commit.c | 16 ++--- builtin/reset.c | 16 ++--- builtin/stash.c | 46 ++++++------ commit.h | 2 +- 10 files changed, 270 insertions(+), 239 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 68fc09547d..05d2e7eefe 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -3,7 +3,6 @@ #include "git-compat-util.h" #include "add-interactive.h" #include "color.h" -#include "config.h" #include "diffcore.h" #include "gettext.h" #include "hash.h" @@ -20,119 +19,18 @@ #include "prompt.h" #include "tree.h" -static void init_color(struct repository *r, enum git_colorbool use_color, - const char *section_and_slot, char *dst, - const char *default_color) -{ - char *key = xstrfmt("color.%s", section_and_slot); - const char *value; - - if (!want_color(use_color)) - dst[0] = '\0'; - else if (repo_config_get_value(r, key, &value) || - color_parse(value, dst)) - strlcpy(dst, default_color, COLOR_MAXLEN); - - free(key); -} - -static enum git_colorbool check_color_config(struct repository *r, const char *var) -{ - const char *value; - enum git_colorbool ret; - - if (repo_config_get_value(r, var, &value)) - ret = GIT_COLOR_UNKNOWN; - else - ret = git_config_colorbool(var, value); - - /* - * Do not rely on want_color() to fall back to color.ui for us. It uses - * the value parsed by git_color_config(), which may not have been - * called by the main command. - */ - if (ret == GIT_COLOR_UNKNOWN && - !repo_config_get_value(r, "color.ui", &value)) - ret = git_config_colorbool("color.ui", value); - - return ret; -} - void init_add_i_state(struct add_i_state *s, struct repository *r, - struct add_p_opt *add_p_opt) + struct interactive_options *opts) { s->r = r; - s->context = -1; - s->interhunkcontext = -1; - - s->use_color_interactive = check_color_config(r, "color.interactive"); - - init_color(r, s->use_color_interactive, "interactive.header", - s->header_color, GIT_COLOR_BOLD); - init_color(r, s->use_color_interactive, "interactive.help", - s->help_color, GIT_COLOR_BOLD_RED); - init_color(r, s->use_color_interactive, "interactive.prompt", - s->prompt_color, GIT_COLOR_BOLD_BLUE); - init_color(r, s->use_color_interactive, "interactive.error", - s->error_color, GIT_COLOR_BOLD_RED); - strlcpy(s->reset_color_interactive, - want_color(s->use_color_interactive) ? GIT_COLOR_RESET : "", COLOR_MAXLEN); - - s->use_color_diff = check_color_config(r, "color.diff"); - - init_color(r, s->use_color_diff, "diff.frag", s->fraginfo_color, - diff_get_color(s->use_color_diff, DIFF_FRAGINFO)); - init_color(r, s->use_color_diff, "diff.context", s->context_color, - "fall back"); - if (!strcmp(s->context_color, "fall back")) - init_color(r, s->use_color_diff, "diff.plain", - s->context_color, - diff_get_color(s->use_color_diff, DIFF_CONTEXT)); - init_color(r, s->use_color_diff, "diff.old", s->file_old_color, - diff_get_color(s->use_color_diff, DIFF_FILE_OLD)); - init_color(r, s->use_color_diff, "diff.new", s->file_new_color, - diff_get_color(s->use_color_diff, DIFF_FILE_NEW)); - strlcpy(s->reset_color_diff, - want_color(s->use_color_diff) ? GIT_COLOR_RESET : "", COLOR_MAXLEN); - - FREE_AND_NULL(s->interactive_diff_filter); - repo_config_get_string(r, "interactive.difffilter", - &s->interactive_diff_filter); - - FREE_AND_NULL(s->interactive_diff_algorithm); - repo_config_get_string(r, "diff.algorithm", - &s->interactive_diff_algorithm); - - if (!repo_config_get_int(r, "diff.context", &s->context)) - if (s->context < 0) - die(_("%s cannot be negative"), "diff.context"); - if (!repo_config_get_int(r, "diff.interHunkContext", &s->interhunkcontext)) - if (s->interhunkcontext < 0) - die(_("%s cannot be negative"), "diff.interHunkContext"); - - repo_config_get_bool(r, "interactive.singlekey", &s->use_single_key); - if (s->use_single_key) - setbuf(stdin, NULL); - - if (add_p_opt->context != -1) { - if (add_p_opt->context < 0) - die(_("%s cannot be negative"), "--unified"); - s->context = add_p_opt->context; - } - if (add_p_opt->interhunkcontext != -1) { - if (add_p_opt->interhunkcontext < 0) - die(_("%s cannot be negative"), "--inter-hunk-context"); - s->interhunkcontext = add_p_opt->interhunkcontext; - } + interactive_config_init(&s->cfg, r, opts); } void clear_add_i_state(struct add_i_state *s) { - FREE_AND_NULL(s->interactive_diff_filter); - FREE_AND_NULL(s->interactive_diff_algorithm); + interactive_config_clear(&s->cfg); memset(s, 0, sizeof(*s)); - s->use_color_interactive = GIT_COLOR_UNKNOWN; - s->use_color_diff = GIT_COLOR_UNKNOWN; + interactive_config_clear(&s->cfg); } /* @@ -286,7 +184,7 @@ static void list(struct add_i_state *s, struct string_list *list, int *selected, return; if (opts->header) - color_fprintf_ln(stdout, s->header_color, + color_fprintf_ln(stdout, s->cfg.header_color, "%s", opts->header); for (i = 0; i < list->nr; i++) { @@ -354,7 +252,7 @@ static ssize_t list_and_choose(struct add_i_state *s, list(s, &items->items, items->selected, &opts->list_opts); - color_fprintf(stdout, s->prompt_color, "%s", opts->prompt); + color_fprintf(stdout, s->cfg.prompt_color, "%s", opts->prompt); fputs(singleton ? "> " : ">> ", stdout); fflush(stdout); @@ -432,7 +330,7 @@ static ssize_t list_and_choose(struct add_i_state *s, if (from < 0 || from >= items->items.nr || (singleton && from + 1 != to)) { - color_fprintf_ln(stderr, s->error_color, + color_fprintf_ln(stderr, s->cfg.error_color, _("Huh (%s)?"), p); break; } else if (singleton) { @@ -992,7 +890,7 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps, free(files->items.items[i].string); } else if (item->index.unmerged || item->worktree.unmerged) { - color_fprintf_ln(stderr, s->error_color, + color_fprintf_ln(stderr, s->cfg.error_color, _("ignoring unmerged: %s"), files->items.items[i].string); free(item); @@ -1014,9 +912,9 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps, opts->prompt = N_("Patch update"); count = list_and_choose(s, files, opts); if (count > 0) { - struct add_p_opt add_p_opt = { - .context = s->context, - .interhunkcontext = s->interhunkcontext, + struct interactive_options opts = { + .context = s->cfg.context, + .interhunkcontext = s->cfg.interhunkcontext, }; struct strvec args = STRVEC_INIT; struct pathspec ps_selected = { 0 }; @@ -1028,7 +926,7 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps, parse_pathspec(&ps_selected, PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL, PATHSPEC_LITERAL_PATH, "", args.v); - res = run_add_p(s->r, ADD_P_ADD, &add_p_opt, NULL, &ps_selected); + res = run_add_p(s->r, ADD_P_ADD, &opts, NULL, &ps_selected); strvec_clear(&args); clear_pathspec(&ps_selected); } @@ -1064,10 +962,10 @@ static int run_diff(struct add_i_state *s, const struct pathspec *ps, struct child_process cmd = CHILD_PROCESS_INIT; strvec_pushl(&cmd.args, "git", "diff", "-p", "--cached", NULL); - if (s->context != -1) - strvec_pushf(&cmd.args, "--unified=%i", s->context); - if (s->interhunkcontext != -1) - strvec_pushf(&cmd.args, "--inter-hunk-context=%i", s->interhunkcontext); + if (s->cfg.context != -1) + strvec_pushf(&cmd.args, "--unified=%i", s->cfg.context); + if (s->cfg.interhunkcontext != -1) + strvec_pushf(&cmd.args, "--inter-hunk-context=%i", s->cfg.interhunkcontext); strvec_pushl(&cmd.args, oid_to_hex(!is_initial ? &oid : s->r->hash_algo->empty_tree), "--", NULL); for (i = 0; i < files->items.nr; i++) @@ -1085,17 +983,17 @@ static int run_help(struct add_i_state *s, const struct pathspec *ps UNUSED, struct prefix_item_list *files UNUSED, struct list_and_choose_options *opts UNUSED) { - color_fprintf_ln(stdout, s->help_color, "status - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "status - %s", _("show paths with changes")); - color_fprintf_ln(stdout, s->help_color, "update - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "update - %s", _("add working tree state to the staged set of changes")); - color_fprintf_ln(stdout, s->help_color, "revert - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "revert - %s", _("revert staged set of changes back to the HEAD version")); - color_fprintf_ln(stdout, s->help_color, "patch - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "patch - %s", _("pick hunks and update selectively")); - color_fprintf_ln(stdout, s->help_color, "diff - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "diff - %s", _("view diff between HEAD and index")); - color_fprintf_ln(stdout, s->help_color, "add untracked - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "add untracked - %s", _("add contents of untracked files to the staged set of changes")); return 0; @@ -1103,21 +1001,21 @@ static int run_help(struct add_i_state *s, const struct pathspec *ps UNUSED, static void choose_prompt_help(struct add_i_state *s) { - color_fprintf_ln(stdout, s->help_color, "%s", + color_fprintf_ln(stdout, s->cfg.help_color, "%s", _("Prompt help:")); - color_fprintf_ln(stdout, s->help_color, "1 - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "1 - %s", _("select a single item")); - color_fprintf_ln(stdout, s->help_color, "3-5 - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "3-5 - %s", _("select a range of items")); - color_fprintf_ln(stdout, s->help_color, "2-3,6-9 - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "2-3,6-9 - %s", _("select multiple ranges")); - color_fprintf_ln(stdout, s->help_color, "foo - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "foo - %s", _("select item based on unique prefix")); - color_fprintf_ln(stdout, s->help_color, "-... - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "-... - %s", _("unselect specified items")); - color_fprintf_ln(stdout, s->help_color, "* - %s", + color_fprintf_ln(stdout, s->cfg.help_color, "* - %s", _("choose all items")); - color_fprintf_ln(stdout, s->help_color, " - %s", + color_fprintf_ln(stdout, s->cfg.help_color, " - %s", _("(empty) finish selecting")); } @@ -1152,7 +1050,7 @@ static void print_command_item(int i, int selected UNUSED, static void command_prompt_help(struct add_i_state *s) { - const char *help_color = s->help_color; + const char *help_color = s->cfg.help_color; color_fprintf_ln(stdout, help_color, "%s", _("Prompt help:")); color_fprintf_ln(stdout, help_color, "1 - %s", _("select a numbered item")); @@ -1163,7 +1061,7 @@ static void command_prompt_help(struct add_i_state *s) } int run_add_i(struct repository *r, const struct pathspec *ps, - struct add_p_opt *add_p_opt) + struct interactive_options *interactive_opts) { struct add_i_state s = { NULL }; struct print_command_item_data data = { "[", "]" }; @@ -1206,15 +1104,15 @@ int run_add_i(struct repository *r, const struct pathspec *ps, ->util = util; } - init_add_i_state(&s, r, add_p_opt); + init_add_i_state(&s, r, interactive_opts); /* * When color was asked for, use the prompt color for * highlighting, otherwise use square brackets. */ - if (want_color(s.use_color_interactive)) { - data.color = s.prompt_color; - data.reset = s.reset_color_interactive; + if (want_color(s.cfg.use_color_interactive)) { + data.color = s.cfg.prompt_color; + data.reset = s.cfg.reset_color_interactive; } print_file_item_data.color = data.color; print_file_item_data.reset = data.reset; diff --git a/add-interactive.h b/add-interactive.h index 2e3d1d871d..eefa2edc7c 100644 --- a/add-interactive.h +++ b/add-interactive.h @@ -2,37 +2,20 @@ #define ADD_INTERACTIVE_H #include "add-patch.h" -#include "color.h" struct pathspec; struct repository; struct add_i_state { struct repository *r; - enum git_colorbool use_color_interactive; - enum git_colorbool use_color_diff; - char header_color[COLOR_MAXLEN]; - char help_color[COLOR_MAXLEN]; - char prompt_color[COLOR_MAXLEN]; - char error_color[COLOR_MAXLEN]; - char reset_color_interactive[COLOR_MAXLEN]; - - char fraginfo_color[COLOR_MAXLEN]; - char context_color[COLOR_MAXLEN]; - char file_old_color[COLOR_MAXLEN]; - char file_new_color[COLOR_MAXLEN]; - char reset_color_diff[COLOR_MAXLEN]; - - int use_single_key; - char *interactive_diff_filter, *interactive_diff_algorithm; - int context, interhunkcontext; + struct interactive_config cfg; }; void init_add_i_state(struct add_i_state *s, struct repository *r, - struct add_p_opt *add_p_opt); + struct interactive_options *opts); void clear_add_i_state(struct add_i_state *s); int run_add_i(struct repository *r, const struct pathspec *ps, - struct add_p_opt *add_p_opt); + struct interactive_options *opts); #endif diff --git a/add-patch.c b/add-patch.c index 5e3481083b..6797d2f690 100644 --- a/add-patch.c +++ b/add-patch.c @@ -5,6 +5,8 @@ #include "add-interactive.h" #include "add-patch.h" #include "advice.h" +#include "config.h" +#include "diff.h" #include "editor.h" #include "environment.h" #include "gettext.h" @@ -279,6 +281,122 @@ struct add_p_state { const char *revision; }; +static void init_color(struct repository *r, + enum git_colorbool use_color, + const char *section_and_slot, char *dst, + const char *default_color) +{ + char *key = xstrfmt("color.%s", section_and_slot); + const char *value; + + if (!want_color(use_color)) + dst[0] = '\0'; + else if (repo_config_get_value(r, key, &value) || + color_parse(value, dst)) + strlcpy(dst, default_color, COLOR_MAXLEN); + + free(key); +} + +static enum git_colorbool check_color_config(struct repository *r, const char *var) +{ + const char *value; + enum git_colorbool ret; + + if (repo_config_get_value(r, var, &value)) + ret = GIT_COLOR_UNKNOWN; + else + ret = git_config_colorbool(var, value); + + /* + * Do not rely on want_color() to fall back to color.ui for us. It uses + * the value parsed by git_color_config(), which may not have been + * called by the main command. + */ + if (ret == GIT_COLOR_UNKNOWN && + !repo_config_get_value(r, "color.ui", &value)) + ret = git_config_colorbool("color.ui", value); + + return ret; +} + +void interactive_config_init(struct interactive_config *cfg, + struct repository *r, + struct interactive_options *opts) +{ + cfg->context = -1; + cfg->interhunkcontext = -1; + + cfg->use_color_interactive = check_color_config(r, "color.interactive"); + + init_color(r, cfg->use_color_interactive, "interactive.header", + cfg->header_color, GIT_COLOR_BOLD); + init_color(r, cfg->use_color_interactive, "interactive.help", + cfg->help_color, GIT_COLOR_BOLD_RED); + init_color(r, cfg->use_color_interactive, "interactive.prompt", + cfg->prompt_color, GIT_COLOR_BOLD_BLUE); + init_color(r, cfg->use_color_interactive, "interactive.error", + cfg->error_color, GIT_COLOR_BOLD_RED); + strlcpy(cfg->reset_color_interactive, + want_color(cfg->use_color_interactive) ? GIT_COLOR_RESET : "", COLOR_MAXLEN); + + cfg->use_color_diff = check_color_config(r, "color.diff"); + + init_color(r, cfg->use_color_diff, "diff.frag", cfg->fraginfo_color, + diff_get_color(cfg->use_color_diff, DIFF_FRAGINFO)); + init_color(r, cfg->use_color_diff, "diff.context", cfg->context_color, + "fall back"); + if (!strcmp(cfg->context_color, "fall back")) + init_color(r, cfg->use_color_diff, "diff.plain", + cfg->context_color, + diff_get_color(cfg->use_color_diff, DIFF_CONTEXT)); + init_color(r, cfg->use_color_diff, "diff.old", cfg->file_old_color, + diff_get_color(cfg->use_color_diff, DIFF_FILE_OLD)); + init_color(r, cfg->use_color_diff, "diff.new", cfg->file_new_color, + diff_get_color(cfg->use_color_diff, DIFF_FILE_NEW)); + strlcpy(cfg->reset_color_diff, + want_color(cfg->use_color_diff) ? GIT_COLOR_RESET : "", COLOR_MAXLEN); + + FREE_AND_NULL(cfg->interactive_diff_filter); + repo_config_get_string(r, "interactive.difffilter", + &cfg->interactive_diff_filter); + + FREE_AND_NULL(cfg->interactive_diff_algorithm); + repo_config_get_string(r, "diff.algorithm", + &cfg->interactive_diff_algorithm); + + if (!repo_config_get_int(r, "diff.context", &cfg->context)) + if (cfg->context < 0) + die(_("%s cannot be negative"), "diff.context"); + if (!repo_config_get_int(r, "diff.interHunkContext", &cfg->interhunkcontext)) + if (cfg->interhunkcontext < 0) + die(_("%s cannot be negative"), "diff.interHunkContext"); + + repo_config_get_bool(r, "interactive.singlekey", &cfg->use_single_key); + if (cfg->use_single_key) + setbuf(stdin, NULL); + + if (opts->context != -1) { + if (opts->context < 0) + die(_("%s cannot be negative"), "--unified"); + cfg->context = opts->context; + } + if (opts->interhunkcontext != -1) { + if (opts->interhunkcontext < 0) + die(_("%s cannot be negative"), "--inter-hunk-context"); + cfg->interhunkcontext = opts->interhunkcontext; + } +} + +void interactive_config_clear(struct interactive_config *cfg) +{ + FREE_AND_NULL(cfg->interactive_diff_filter); + FREE_AND_NULL(cfg->interactive_diff_algorithm); + memset(cfg, 0, sizeof(*cfg)); + cfg->use_color_interactive = GIT_COLOR_UNKNOWN; + cfg->use_color_diff = GIT_COLOR_UNKNOWN; +} + static void add_p_state_clear(struct add_p_state *s) { size_t i; @@ -299,9 +417,9 @@ static void err(struct add_p_state *s, const char *fmt, ...) va_list args; va_start(args, fmt); - fputs(s->s.error_color, stdout); + fputs(s->s.cfg.error_color, stdout); vprintf(fmt, args); - puts(s->s.reset_color_interactive); + puts(s->s.cfg.reset_color_interactive); va_end(args); } @@ -424,12 +542,12 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) int res; strvec_pushv(&args, s->mode->diff_cmd); - if (s->s.context != -1) - strvec_pushf(&args, "--unified=%i", s->s.context); - if (s->s.interhunkcontext != -1) - strvec_pushf(&args, "--inter-hunk-context=%i", s->s.interhunkcontext); - if (s->s.interactive_diff_algorithm) - strvec_pushf(&args, "--diff-algorithm=%s", s->s.interactive_diff_algorithm); + if (s->s.cfg.context != -1) + strvec_pushf(&args, "--unified=%i", s->s.cfg.context); + if (s->s.cfg.interhunkcontext != -1) + strvec_pushf(&args, "--inter-hunk-context=%i", s->s.cfg.interhunkcontext); + if (s->s.cfg.interactive_diff_algorithm) + strvec_pushf(&args, "--diff-algorithm=%s", s->s.cfg.interactive_diff_algorithm); if (s->revision) { struct object_id oid; strvec_push(&args, @@ -458,9 +576,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) } strbuf_complete_line(plain); - if (want_color_fd(1, s->s.use_color_diff)) { + if (want_color_fd(1, s->s.cfg.use_color_diff)) { struct child_process colored_cp = CHILD_PROCESS_INIT; - const char *diff_filter = s->s.interactive_diff_filter; + const char *diff_filter = s->s.cfg.interactive_diff_filter; setup_child_process(s, &colored_cp, NULL); xsnprintf((char *)args.v[color_arg_index], 8, "--color"); @@ -693,7 +811,7 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk, hunk->colored_end - hunk->colored_start); return; } else { - strbuf_addstr(out, s->s.fraginfo_color); + strbuf_addstr(out, s->s.cfg.fraginfo_color); p = s->colored.buf + header->colored_extra_start; len = header->colored_extra_end - header->colored_extra_start; @@ -715,7 +833,7 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk, if (len) strbuf_add(out, p, len); else if (colored) - strbuf_addf(out, "%s\n", s->s.reset_color_diff); + strbuf_addf(out, "%s\n", s->s.cfg.reset_color_diff); else strbuf_addch(out, '\n'); } @@ -1104,12 +1222,12 @@ static void recolor_hunk(struct add_p_state *s, struct hunk *hunk) strbuf_addstr(&s->colored, plain[current] == '-' ? - s->s.file_old_color : + s->s.cfg.file_old_color : plain[current] == '+' ? - s->s.file_new_color : - s->s.context_color); + s->s.cfg.file_new_color : + s->s.cfg.context_color); strbuf_add(&s->colored, plain + current, eol - current); - strbuf_addstr(&s->colored, s->s.reset_color_diff); + strbuf_addstr(&s->colored, s->s.cfg.reset_color_diff); if (next > eol) strbuf_add(&s->colored, plain + eol, next - eol); current = next; @@ -1238,7 +1356,7 @@ static int run_apply_check(struct add_p_state *s, static int read_single_character(struct add_p_state *s) { - if (s->s.use_single_key) { + if (s->s.cfg.use_single_key) { int res = read_key_without_echo(&s->answer); printf("%s\n", res == EOF ? "" : s->answer.buf); return res; @@ -1252,7 +1370,7 @@ static int read_single_character(struct add_p_state *s) static int prompt_yesno(struct add_p_state *s, const char *prompt) { for (;;) { - color_fprintf(stdout, s->s.prompt_color, "%s", _(prompt)); + color_fprintf(stdout, s->s.cfg.prompt_color, "%s", _(prompt)); fflush(stdout); if (read_single_character(s) == EOF) return -1; @@ -1560,15 +1678,15 @@ static int patch_update_file(struct add_p_state *s, else prompt_mode_type = PROMPT_HUNK; - printf("%s(%"PRIuMAX"/%"PRIuMAX") ", s->s.prompt_color, + printf("%s(%"PRIuMAX"/%"PRIuMAX") ", s->s.cfg.prompt_color, (uintmax_t)hunk_index + 1, (uintmax_t)(file_diff->hunk_nr ? file_diff->hunk_nr : 1)); printf(_(s->mode->prompt_mode[prompt_mode_type]), s->buf.buf); - if (*s->s.reset_color_interactive) - fputs(s->s.reset_color_interactive, stdout); + if (*s->s.cfg.reset_color_interactive) + fputs(s->s.cfg.reset_color_interactive, stdout); fflush(stdout); if (read_single_character(s) == EOF) { quit = 1; @@ -1731,7 +1849,7 @@ static int patch_update_file(struct add_p_state *s, err(s, _("Sorry, cannot split this hunk")); } else if (!split_hunk(s, file_diff, hunk - file_diff->hunk)) { - color_fprintf_ln(stdout, s->s.header_color, + color_fprintf_ln(stdout, s->s.cfg.header_color, _("Split into %d hunks."), (int)splittable_into); rendered_hunk_index = -1; @@ -1749,7 +1867,7 @@ static int patch_update_file(struct add_p_state *s, } else if (s->answer.buf[0] == '?') { const char *p = _(help_patch_remainder), *eol = p; - color_fprintf(stdout, s->s.help_color, "%s", + color_fprintf(stdout, s->s.cfg.help_color, "%s", _(s->mode->help_patch_text)); /* @@ -1767,7 +1885,7 @@ static int patch_update_file(struct add_p_state *s, if (*p != '?' && !strchr(s->buf.buf, *p)) continue; - color_fprintf_ln(stdout, s->s.help_color, + color_fprintf_ln(stdout, s->s.cfg.help_color, "%.*s", (int)(eol - p), p); } } else { @@ -1808,7 +1926,7 @@ static int patch_update_file(struct add_p_state *s, } int run_add_p(struct repository *r, enum add_p_mode mode, - struct add_p_opt *o, const char *revision, + struct interactive_options *opts, const char *revision, const struct pathspec *ps) { struct add_p_state s = { @@ -1816,7 +1934,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode, }; size_t i, binary_count = 0; - init_add_i_state(&s.s, r, o); + init_add_i_state(&s.s, r, opts); if (mode == ADD_P_STASH) s.mode = &patch_mode_stash; diff --git a/add-patch.h b/add-patch.h index 4394c74107..a4a05d9d14 100644 --- a/add-patch.h +++ b/add-patch.h @@ -1,15 +1,45 @@ #ifndef ADD_PATCH_H #define ADD_PATCH_H +#include "color.h" + struct pathspec; struct repository; -struct add_p_opt { +struct interactive_options { int context; int interhunkcontext; }; -#define ADD_P_OPT_INIT { .context = -1, .interhunkcontext = -1 } +#define INTERACTIVE_OPTIONS_INIT { \ + .context = -1, \ + .interhunkcontext = -1, \ +} + +struct interactive_config { + enum git_colorbool use_color_interactive; + enum git_colorbool use_color_diff; + char header_color[COLOR_MAXLEN]; + char help_color[COLOR_MAXLEN]; + char prompt_color[COLOR_MAXLEN]; + char error_color[COLOR_MAXLEN]; + char reset_color_interactive[COLOR_MAXLEN]; + + char fraginfo_color[COLOR_MAXLEN]; + char context_color[COLOR_MAXLEN]; + char file_old_color[COLOR_MAXLEN]; + char file_new_color[COLOR_MAXLEN]; + char reset_color_diff[COLOR_MAXLEN]; + + int use_single_key; + char *interactive_diff_filter, *interactive_diff_algorithm; + int context, interhunkcontext; +}; + +void interactive_config_init(struct interactive_config *cfg, + struct repository *r, + struct interactive_options *opts); +void interactive_config_clear(struct interactive_config *cfg); enum add_p_mode { ADD_P_ADD, @@ -20,7 +50,7 @@ enum add_p_mode { }; int run_add_p(struct repository *r, enum add_p_mode mode, - struct add_p_opt *o, const char *revision, + struct interactive_options *opts, const char *revision, const struct pathspec *ps); #endif diff --git a/builtin/add.c b/builtin/add.c index 32709794b3..6f1e213052 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -31,7 +31,7 @@ static const char * const builtin_add_usage[] = { NULL }; static int patch_interactive, add_interactive, edit_interactive; -static struct add_p_opt add_p_opt = ADD_P_OPT_INIT; +static struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT; static int take_worktree_changes; static int add_renormalize; static int pathspec_file_nul; @@ -160,7 +160,7 @@ static int refresh(struct repository *repo, int verbose, const struct pathspec * int interactive_add(struct repository *repo, const char **argv, const char *prefix, - int patch, struct add_p_opt *add_p_opt) + int patch, struct interactive_options *interactive_opts) { struct pathspec pathspec; int ret; @@ -172,9 +172,9 @@ int interactive_add(struct repository *repo, prefix, argv); if (patch) - ret = !!run_add_p(repo, ADD_P_ADD, add_p_opt, NULL, &pathspec); + ret = !!run_add_p(repo, ADD_P_ADD, interactive_opts, NULL, &pathspec); else - ret = !!run_add_i(repo, &pathspec, add_p_opt); + ret = !!run_add_i(repo, &pathspec, interactive_opts); clear_pathspec(&pathspec); return ret; @@ -256,8 +256,8 @@ static struct option builtin_add_options[] = { OPT_GROUP(""), OPT_BOOL('i', "interactive", &add_interactive, N_("interactive picking")), OPT_BOOL('p', "patch", &patch_interactive, N_("select hunks interactively")), - OPT_DIFF_UNIFIED(&add_p_opt.context), - OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), + OPT_DIFF_UNIFIED(&interactive_opts.context), + OPT_DIFF_INTERHUNK_CONTEXT(&interactive_opts.interhunkcontext), OPT_BOOL('e', "edit", &edit_interactive, N_("edit current diff and apply")), OPT__FORCE(&ignored_too, N_("allow adding otherwise ignored files"), 0), OPT_BOOL('u', "update", &take_worktree_changes, N_("update tracked files")), @@ -400,9 +400,9 @@ int cmd_add(int argc, prepare_repo_settings(repo); repo->settings.command_requires_full_index = 0; - if (add_p_opt.context < -1) + if (interactive_opts.context < -1) die(_("'%s' cannot be negative"), "--unified"); - if (add_p_opt.interhunkcontext < -1) + if (interactive_opts.interhunkcontext < -1) die(_("'%s' cannot be negative"), "--inter-hunk-context"); if (patch_interactive) @@ -412,11 +412,11 @@ int cmd_add(int argc, die(_("options '%s' and '%s' cannot be used together"), "--dry-run", "--interactive/--patch"); if (pathspec_from_file) die(_("options '%s' and '%s' cannot be used together"), "--pathspec-from-file", "--interactive/--patch"); - exit(interactive_add(repo, argv + 1, prefix, patch_interactive, &add_p_opt)); + exit(interactive_add(repo, argv + 1, prefix, patch_interactive, &interactive_opts)); } else { - if (add_p_opt.context != -1) + if (interactive_opts.context != -1) die(_("the option '%s' requires '%s'"), "--unified", "--interactive/--patch"); - if (add_p_opt.interhunkcontext != -1) + if (interactive_opts.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--interactive/--patch"); } diff --git a/builtin/checkout.c b/builtin/checkout.c index 66b69df6e6..530ae956ad 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -546,7 +546,7 @@ static int checkout_paths(const struct checkout_opts *opts, if (opts->patch_mode) { enum add_p_mode patch_mode; - struct add_p_opt add_p_opt = { + struct interactive_options interactive_opts = { .context = opts->patch_context, .interhunkcontext = opts->patch_interhunk_context, }; @@ -575,7 +575,7 @@ static int checkout_paths(const struct checkout_opts *opts, else BUG("either flag must have been set, worktree=%d, index=%d", opts->checkout_worktree, opts->checkout_index); - return !!run_add_p(the_repository, patch_mode, &add_p_opt, + return !!run_add_p(the_repository, patch_mode, &interactive_opts, rev, &opts->pathspec); } diff --git a/builtin/commit.c b/builtin/commit.c index 0243f17d53..640495cc57 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -123,7 +123,7 @@ static const char *edit_message, *use_message; static char *fixup_message, *fixup_commit, *squash_message; static const char *fixup_prefix; static int all, also, interactive, patch_interactive, only, amend, signoff; -static struct add_p_opt add_p_opt = ADD_P_OPT_INIT; +static struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT; static int edit_flag = -1; /* unspecified */ static int quiet, verbose, no_verify, allow_empty, dry_run, renew_authorship; static int config_commit_verbose = -1; /* unspecified */ @@ -356,9 +356,9 @@ static const char *prepare_index(const char **argv, const char *prefix, const char *ret; char *path = NULL; - if (add_p_opt.context < -1) + if (interactive_opts.context < -1) die(_("'%s' cannot be negative"), "--unified"); - if (add_p_opt.interhunkcontext < -1) + if (interactive_opts.interhunkcontext < -1) die(_("'%s' cannot be negative"), "--inter-hunk-context"); if (is_status) @@ -407,7 +407,7 @@ static const char *prepare_index(const char **argv, const char *prefix, old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT)); setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1); - if (interactive_add(the_repository, argv, prefix, patch_interactive, &add_p_opt) != 0) + if (interactive_add(the_repository, argv, prefix, patch_interactive, &interactive_opts) != 0) die(_("interactive add failed")); the_repository->index_file = old_repo_index_file; @@ -432,9 +432,9 @@ static const char *prepare_index(const char **argv, const char *prefix, ret = get_lock_file_path(&index_lock); goto out; } else { - if (add_p_opt.context != -1) + if (interactive_opts.context != -1) die(_("the option '%s' requires '%s'"), "--unified", "--interactive/--patch"); - if (add_p_opt.interhunkcontext != -1) + if (interactive_opts.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--interactive/--patch"); } @@ -1742,8 +1742,8 @@ int cmd_commit(int argc, OPT_BOOL('i', "include", &also, N_("add specified files to index for commit")), OPT_BOOL(0, "interactive", &interactive, N_("interactively add files")), OPT_BOOL('p', "patch", &patch_interactive, N_("interactively add changes")), - OPT_DIFF_UNIFIED(&add_p_opt.context), - OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), + OPT_DIFF_UNIFIED(&interactive_opts.context), + OPT_DIFF_INTERHUNK_CONTEXT(&interactive_opts.interhunkcontext), OPT_BOOL('o', "only", &only, N_("commit only specified files")), OPT_BOOL('n', "no-verify", &no_verify, N_("bypass pre-commit and commit-msg hooks")), OPT_BOOL(0, "dry-run", &dry_run, N_("show what would be committed")), diff --git a/builtin/reset.c b/builtin/reset.c index ed35802af1..088449e120 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -346,7 +346,7 @@ int cmd_reset(int argc, struct object_id oid; struct pathspec pathspec; int intent_to_add = 0; - struct add_p_opt add_p_opt = ADD_P_OPT_INIT; + struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT; const struct option options[] = { OPT__QUIET(&quiet, N_("be quiet, only report errors")), OPT_BOOL(0, "no-refresh", &no_refresh, @@ -371,8 +371,8 @@ int cmd_reset(int argc, PARSE_OPT_OPTARG, option_parse_recurse_submodules_worktree_updater), OPT_BOOL('p', "patch", &patch_mode, N_("select hunks interactively")), - OPT_DIFF_UNIFIED(&add_p_opt.context), - OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), + OPT_DIFF_UNIFIED(&interactive_opts.context), + OPT_DIFF_INTERHUNK_CONTEXT(&interactive_opts.interhunkcontext), OPT_BOOL('N', "intent-to-add", &intent_to_add, N_("record only the fact that removed paths will be added later")), OPT_PATHSPEC_FROM_FILE(&pathspec_from_file), @@ -423,9 +423,9 @@ int cmd_reset(int argc, oidcpy(&oid, &tree->object.oid); } - if (add_p_opt.context < -1) + if (interactive_opts.context < -1) die(_("'%s' cannot be negative"), "--unified"); - if (add_p_opt.interhunkcontext < -1) + if (interactive_opts.interhunkcontext < -1) die(_("'%s' cannot be negative"), "--inter-hunk-context"); prepare_repo_settings(the_repository); @@ -436,12 +436,12 @@ int cmd_reset(int argc, die(_("options '%s' and '%s' cannot be used together"), "--patch", "--{hard,mixed,soft}"); trace2_cmd_mode("patch-interactive"); update_ref_status = !!run_add_p(the_repository, ADD_P_RESET, - &add_p_opt, rev, &pathspec); + &interactive_opts, rev, &pathspec); goto cleanup; } else { - if (add_p_opt.context != -1) + if (interactive_opts.context != -1) die(_("the option '%s' requires '%s'"), "--unified", "--patch"); - if (add_p_opt.interhunkcontext != -1) + if (interactive_opts.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--patch"); } diff --git a/builtin/stash.c b/builtin/stash.c index 948eba06fb..3b50905233 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1306,7 +1306,7 @@ static int stash_staged(struct stash_info *info, struct strbuf *out_patch, static int stash_patch(struct stash_info *info, const struct pathspec *ps, struct strbuf *out_patch, int quiet, - struct add_p_opt *add_p_opt) + struct interactive_options *interactive_opts) { int ret = 0; struct child_process cp_read_tree = CHILD_PROCESS_INIT; @@ -1331,7 +1331,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps, old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT)); setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1); - ret = !!run_add_p(the_repository, ADD_P_STASH, add_p_opt, NULL, ps); + ret = !!run_add_p(the_repository, ADD_P_STASH, interactive_opts, NULL, ps); the_repository->index_file = old_repo_index_file; if (old_index_env && *old_index_env) @@ -1427,7 +1427,8 @@ static int stash_working_tree(struct stash_info *info, const struct pathspec *ps } static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_buf, - int include_untracked, int patch_mode, struct add_p_opt *add_p_opt, + int include_untracked, int patch_mode, + struct interactive_options *interactive_opts, int only_staged, struct stash_info *info, struct strbuf *patch, int quiet) { @@ -1509,7 +1510,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b untracked_commit_option = 1; } if (patch_mode) { - ret = stash_patch(info, ps, patch, quiet, add_p_opt); + ret = stash_patch(info, ps, patch, quiet, interactive_opts); if (ret < 0) { if (!quiet) fprintf_ln(stderr, _("Cannot save the current " @@ -1595,7 +1596,8 @@ static int create_stash(int argc, const char **argv, const char *prefix UNUSED, } static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int quiet, - int keep_index, int patch_mode, struct add_p_opt *add_p_opt, + int keep_index, int patch_mode, + struct interactive_options *interactive_opts, int include_untracked, int only_staged) { int ret = 0; @@ -1667,7 +1669,7 @@ static int do_push_stash(const struct pathspec *ps, const char *stash_msg, int q if (stash_msg) strbuf_addstr(&stash_msg_buf, stash_msg); if (do_create_stash(ps, &stash_msg_buf, include_untracked, patch_mode, - add_p_opt, only_staged, &info, &patch, quiet)) { + interactive_opts, only_staged, &info, &patch, quiet)) { ret = -1; goto done; } @@ -1841,7 +1843,7 @@ static int push_stash(int argc, const char **argv, const char *prefix, const char *stash_msg = NULL; char *pathspec_from_file = NULL; struct pathspec ps; - struct add_p_opt add_p_opt = ADD_P_OPT_INIT; + struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT; struct option options[] = { OPT_BOOL('k', "keep-index", &keep_index, N_("keep index")), @@ -1849,8 +1851,8 @@ static int push_stash(int argc, const char **argv, const char *prefix, N_("stash staged changes only")), OPT_BOOL('p', "patch", &patch_mode, N_("stash in patch mode")), - OPT_DIFF_UNIFIED(&add_p_opt.context), - OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), + OPT_DIFF_UNIFIED(&interactive_opts.context), + OPT_DIFF_INTERHUNK_CONTEXT(&interactive_opts.interhunkcontext), OPT__QUIET(&quiet, N_("quiet mode")), OPT_BOOL('u', "include-untracked", &include_untracked, N_("include untracked files in stash")), @@ -1907,19 +1909,19 @@ static int push_stash(int argc, const char **argv, const char *prefix, } if (!patch_mode) { - if (add_p_opt.context != -1) + if (interactive_opts.context != -1) die(_("the option '%s' requires '%s'"), "--unified", "--patch"); - if (add_p_opt.interhunkcontext != -1) + if (interactive_opts.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--patch"); } - if (add_p_opt.context < -1) + if (interactive_opts.context < -1) die(_("'%s' cannot be negative"), "--unified"); - if (add_p_opt.interhunkcontext < -1) + if (interactive_opts.interhunkcontext < -1) die(_("'%s' cannot be negative"), "--inter-hunk-context"); ret = do_push_stash(&ps, stash_msg, quiet, keep_index, patch_mode, - &add_p_opt, include_untracked, only_staged); + &interactive_opts, include_untracked, only_staged); clear_pathspec(&ps); free(pathspec_from_file); @@ -1944,7 +1946,7 @@ static int save_stash(int argc, const char **argv, const char *prefix, const char *stash_msg = NULL; struct pathspec ps; struct strbuf stash_msg_buf = STRBUF_INIT; - struct add_p_opt add_p_opt = ADD_P_OPT_INIT; + struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT; struct option options[] = { OPT_BOOL('k', "keep-index", &keep_index, N_("keep index")), @@ -1952,8 +1954,8 @@ static int save_stash(int argc, const char **argv, const char *prefix, N_("stash staged changes only")), OPT_BOOL('p', "patch", &patch_mode, N_("stash in patch mode")), - OPT_DIFF_UNIFIED(&add_p_opt.context), - OPT_DIFF_INTERHUNK_CONTEXT(&add_p_opt.interhunkcontext), + OPT_DIFF_UNIFIED(&interactive_opts.context), + OPT_DIFF_INTERHUNK_CONTEXT(&interactive_opts.interhunkcontext), OPT__QUIET(&quiet, N_("quiet mode")), OPT_BOOL('u', "include-untracked", &include_untracked, N_("include untracked files in stash")), @@ -1973,20 +1975,20 @@ static int save_stash(int argc, const char **argv, const char *prefix, memset(&ps, 0, sizeof(ps)); - if (add_p_opt.context < -1) + if (interactive_opts.context < -1) die(_("'%s' cannot be negative"), "--unified"); - if (add_p_opt.interhunkcontext < -1) + if (interactive_opts.interhunkcontext < -1) die(_("'%s' cannot be negative"), "--inter-hunk-context"); if (!patch_mode) { - if (add_p_opt.context != -1) + if (interactive_opts.context != -1) die(_("the option '%s' requires '%s'"), "--unified", "--patch"); - if (add_p_opt.interhunkcontext != -1) + if (interactive_opts.interhunkcontext != -1) die(_("the option '%s' requires '%s'"), "--inter-hunk-context", "--patch"); } ret = do_push_stash(&ps, stash_msg, quiet, keep_index, - patch_mode, &add_p_opt, include_untracked, + patch_mode, &interactive_opts, include_untracked, only_staged); strbuf_release(&stash_msg_buf); diff --git a/commit.h b/commit.h index 1d6e0c7518..7b6e59d6c1 100644 --- a/commit.h +++ b/commit.h @@ -258,7 +258,7 @@ int for_each_commit_graft(each_commit_graft_fn, void *); int interactive_add(struct repository *repo, const char **argv, const char *prefix, - int patch, struct add_p_opt *add_p_opt); + int patch, struct interactive_options *opts); struct commit_extra_header { struct commit_extra_header *next; -- GitLab From 95be396a5ae2c91eb92b9653937a5e63b8e4cf19 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 14 Aug 2025 09:10:56 +0200 Subject: [PATCH 10/14] add-patch: remove dependency on "add-interactive" subsystem With the preceding commit we have split out interactive configuration that is used by both "git add -p" and "git add -i". But we still initialize that configuration in the "add -p" subsystem by calling `init_add_i_state()`, even though we only do so to initialize the interactive configuration as well as a repository pointer. Stop doing so and instead store and initialize the interactive configuration in `struct add_p_state` directly. Signed-off-by: Patrick Steinhardt --- add-patch.c | 70 ++++++++++++++++++++++++++++------------------------- 1 file changed, 37 insertions(+), 33 deletions(-) diff --git a/add-patch.c b/add-patch.c index 6797d2f690..2780738153 100644 --- a/add-patch.c +++ b/add-patch.c @@ -2,7 +2,6 @@ #define DISABLE_SIGN_COMPARE_WARNINGS #include "git-compat-util.h" -#include "add-interactive.h" #include "add-patch.h" #include "advice.h" #include "config.h" @@ -263,7 +262,8 @@ struct hunk { }; struct add_p_state { - struct add_i_state s; + struct repository *r; + struct interactive_config cfg; struct strbuf answer, buf; /* parsed diff */ @@ -408,7 +408,7 @@ static void add_p_state_clear(struct add_p_state *s) for (i = 0; i < s->file_diff_nr; i++) free(s->file_diff[i].hunk); free(s->file_diff); - clear_add_i_state(&s->s); + interactive_config_clear(&s->cfg); } __attribute__((format (printf, 2, 3))) @@ -417,9 +417,9 @@ static void err(struct add_p_state *s, const char *fmt, ...) va_list args; va_start(args, fmt); - fputs(s->s.cfg.error_color, stdout); + fputs(s->cfg.error_color, stdout); vprintf(fmt, args); - puts(s->s.cfg.reset_color_interactive); + puts(s->cfg.reset_color_interactive); va_end(args); } @@ -437,7 +437,7 @@ static void setup_child_process(struct add_p_state *s, cp->git_cmd = 1; strvec_pushf(&cp->env, - INDEX_ENVIRONMENT "=%s", s->s.r->index_file); + INDEX_ENVIRONMENT "=%s", s->r->index_file); } static int parse_range(const char **p, @@ -542,12 +542,12 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) int res; strvec_pushv(&args, s->mode->diff_cmd); - if (s->s.cfg.context != -1) - strvec_pushf(&args, "--unified=%i", s->s.cfg.context); - if (s->s.cfg.interhunkcontext != -1) - strvec_pushf(&args, "--inter-hunk-context=%i", s->s.cfg.interhunkcontext); - if (s->s.cfg.interactive_diff_algorithm) - strvec_pushf(&args, "--diff-algorithm=%s", s->s.cfg.interactive_diff_algorithm); + if (s->cfg.context != -1) + strvec_pushf(&args, "--unified=%i", s->cfg.context); + if (s->cfg.interhunkcontext != -1) + strvec_pushf(&args, "--inter-hunk-context=%i", s->cfg.interhunkcontext); + if (s->cfg.interactive_diff_algorithm) + strvec_pushf(&args, "--diff-algorithm=%s", s->cfg.interactive_diff_algorithm); if (s->revision) { struct object_id oid; strvec_push(&args, @@ -576,9 +576,9 @@ static int parse_diff(struct add_p_state *s, const struct pathspec *ps) } strbuf_complete_line(plain); - if (want_color_fd(1, s->s.cfg.use_color_diff)) { + if (want_color_fd(1, s->cfg.use_color_diff)) { struct child_process colored_cp = CHILD_PROCESS_INIT; - const char *diff_filter = s->s.cfg.interactive_diff_filter; + const char *diff_filter = s->cfg.interactive_diff_filter; setup_child_process(s, &colored_cp, NULL); xsnprintf((char *)args.v[color_arg_index], 8, "--color"); @@ -811,7 +811,7 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk, hunk->colored_end - hunk->colored_start); return; } else { - strbuf_addstr(out, s->s.cfg.fraginfo_color); + strbuf_addstr(out, s->cfg.fraginfo_color); p = s->colored.buf + header->colored_extra_start; len = header->colored_extra_end - header->colored_extra_start; @@ -833,7 +833,7 @@ static void render_hunk(struct add_p_state *s, struct hunk *hunk, if (len) strbuf_add(out, p, len); else if (colored) - strbuf_addf(out, "%s\n", s->s.cfg.reset_color_diff); + strbuf_addf(out, "%s\n", s->cfg.reset_color_diff); else strbuf_addch(out, '\n'); } @@ -1222,12 +1222,12 @@ static void recolor_hunk(struct add_p_state *s, struct hunk *hunk) strbuf_addstr(&s->colored, plain[current] == '-' ? - s->s.cfg.file_old_color : + s->cfg.file_old_color : plain[current] == '+' ? - s->s.cfg.file_new_color : - s->s.cfg.context_color); + s->cfg.file_new_color : + s->cfg.context_color); strbuf_add(&s->colored, plain + current, eol - current); - strbuf_addstr(&s->colored, s->s.cfg.reset_color_diff); + strbuf_addstr(&s->colored, s->cfg.reset_color_diff); if (next > eol) strbuf_add(&s->colored, plain + eol, next - eol); current = next; @@ -1356,7 +1356,7 @@ static int run_apply_check(struct add_p_state *s, static int read_single_character(struct add_p_state *s) { - if (s->s.cfg.use_single_key) { + if (s->cfg.use_single_key) { int res = read_key_without_echo(&s->answer); printf("%s\n", res == EOF ? "" : s->answer.buf); return res; @@ -1370,7 +1370,7 @@ static int read_single_character(struct add_p_state *s) static int prompt_yesno(struct add_p_state *s, const char *prompt) { for (;;) { - color_fprintf(stdout, s->s.cfg.prompt_color, "%s", _(prompt)); + color_fprintf(stdout, s->cfg.prompt_color, "%s", _(prompt)); fflush(stdout); if (read_single_character(s) == EOF) return -1; @@ -1678,15 +1678,15 @@ static int patch_update_file(struct add_p_state *s, else prompt_mode_type = PROMPT_HUNK; - printf("%s(%"PRIuMAX"/%"PRIuMAX") ", s->s.cfg.prompt_color, + printf("%s(%"PRIuMAX"/%"PRIuMAX") ", s->cfg.prompt_color, (uintmax_t)hunk_index + 1, (uintmax_t)(file_diff->hunk_nr ? file_diff->hunk_nr : 1)); printf(_(s->mode->prompt_mode[prompt_mode_type]), s->buf.buf); - if (*s->s.cfg.reset_color_interactive) - fputs(s->s.cfg.reset_color_interactive, stdout); + if (*s->cfg.reset_color_interactive) + fputs(s->cfg.reset_color_interactive, stdout); fflush(stdout); if (read_single_character(s) == EOF) { quit = 1; @@ -1849,7 +1849,7 @@ static int patch_update_file(struct add_p_state *s, err(s, _("Sorry, cannot split this hunk")); } else if (!split_hunk(s, file_diff, hunk - file_diff->hunk)) { - color_fprintf_ln(stdout, s->s.cfg.header_color, + color_fprintf_ln(stdout, s->cfg.header_color, _("Split into %d hunks."), (int)splittable_into); rendered_hunk_index = -1; @@ -1867,7 +1867,7 @@ static int patch_update_file(struct add_p_state *s, } else if (s->answer.buf[0] == '?') { const char *p = _(help_patch_remainder), *eol = p; - color_fprintf(stdout, s->s.cfg.help_color, "%s", + color_fprintf(stdout, s->cfg.help_color, "%s", _(s->mode->help_patch_text)); /* @@ -1885,7 +1885,7 @@ static int patch_update_file(struct add_p_state *s, if (*p != '?' && !strchr(s->buf.buf, *p)) continue; - color_fprintf_ln(stdout, s->s.cfg.help_color, + color_fprintf_ln(stdout, s->cfg.help_color, "%.*s", (int)(eol - p), p); } } else { @@ -1905,7 +1905,7 @@ static int patch_update_file(struct add_p_state *s, strbuf_reset(&s->buf); reassemble_patch(s, file_diff, 0, &s->buf); - discard_index(s->s.r->index); + discard_index(s->r->index); if (s->mode->apply_for_checkout) apply_for_checkout(s, &s->buf, s->mode->is_reverse); @@ -1916,8 +1916,8 @@ static int patch_update_file(struct add_p_state *s, NULL, 0, NULL, 0)) error(_("'git apply' failed")); } - if (repo_read_index(s->s.r) >= 0) - repo_refresh_and_write_index(s->s.r, REFRESH_QUIET, 0, + if (repo_read_index(s->r) >= 0) + repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0, 1, NULL, NULL, NULL); } @@ -1930,11 +1930,15 @@ int run_add_p(struct repository *r, enum add_p_mode mode, const struct pathspec *ps) { struct add_p_state s = { - { r }, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT, STRBUF_INIT + .r = r, + .answer = STRBUF_INIT, + .buf = STRBUF_INIT, + .plain = STRBUF_INIT, + .colored = STRBUF_INIT, }; size_t i, binary_count = 0; - init_add_i_state(&s.s, r, opts); + interactive_config_init(&s.cfg, r, opts); if (mode == ADD_P_STASH) s.mode = &patch_mode_stash; -- GitLab From d114c622a87257be7ad5fc6178609e1608c55a24 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 14 Aug 2025 09:16:06 +0200 Subject: [PATCH 11/14] add-patch: add support for in-memory index patching With `run_add_p()` callers have the ability to apply changes from a specific revision to a repository's index. This infra supports several different modes, like for example applying changes to the index, working tree or both. One feature that is missing though is the ability to apply changes to an in-memory index different from the repository's index. Add a new function `run_add_p_index()` to plug this gap. This new function will be used in a subsequent commit. Signed-off-by: Patrick Steinhardt --- add-patch.c | 139 +++++++++++++++++++++++++++++++++++++++++++++------- add-patch.h | 8 +++ 2 files changed, 128 insertions(+), 19 deletions(-) diff --git a/add-patch.c b/add-patch.c index 2780738153..31d82a3e22 100644 --- a/add-patch.c +++ b/add-patch.c @@ -4,11 +4,13 @@ #include "git-compat-util.h" #include "add-patch.h" #include "advice.h" +#include "commit.h" #include "config.h" #include "diff.h" #include "editor.h" #include "environment.h" #include "gettext.h" +#include "hex.h" #include "object-name.h" #include "pager.h" #include "read-cache-ll.h" @@ -47,7 +49,7 @@ static struct patch_mode patch_mode_add = { N_("Stage mode change [y,n,q,a,d%s,?]? "), N_("Stage deletion [y,n,q,a,d%s,?]? "), N_("Stage addition [y,n,q,a,d%s,?]? "), - N_("Stage this hunk [y,n,q,a,d%s,?]? ") + N_("Stage this hunk [y,n,q,a,d%s,?]? "), }, .edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk " "will immediately be marked for staging."), @@ -263,6 +265,8 @@ struct hunk { struct add_p_state { struct repository *r; + struct index_state *index; + const char *index_file; struct interactive_config cfg; struct strbuf answer, buf; @@ -437,7 +441,7 @@ static void setup_child_process(struct add_p_state *s, cp->git_cmd = 1; strvec_pushf(&cp->env, - INDEX_ENVIRONMENT "=%s", s->r->index_file); + INDEX_ENVIRONMENT "=%s", s->index_file); } static int parse_range(const char **p, @@ -1905,7 +1909,7 @@ static int patch_update_file(struct add_p_state *s, strbuf_reset(&s->buf); reassemble_patch(s, file_diff, 0, &s->buf); - discard_index(s->r->index); + discard_index(s->index); if (s->mode->apply_for_checkout) apply_for_checkout(s, &s->buf, s->mode->is_reverse); @@ -1916,27 +1920,54 @@ static int patch_update_file(struct add_p_state *s, NULL, 0, NULL, 0)) error(_("'git apply' failed")); } - if (repo_read_index(s->r) >= 0) + if (read_index_from(s->index, s->index_file, s->r->gitdir) >= 0 && + s->index == s->r->index) { repo_refresh_and_write_index(s->r, REFRESH_QUIET, 0, 1, NULL, NULL, NULL); + } } putchar('\n'); return quit; } +static int run_add_p_common(struct add_p_state *state, + const struct pathspec *ps) +{ + size_t binary_count = 0; + + if (parse_diff(state, ps) < 0) + return -1; + + for (size_t i = 0; i < state->file_diff_nr; i++) { + if (state->file_diff[i].binary && !state->file_diff[i].hunk_nr) + binary_count++; + else if (patch_update_file(state, state->file_diff + i)) + break; + } + + if (state->file_diff_nr == 0) + err(state, _("No changes.")); + else if (binary_count == state->file_diff_nr) + err(state, _("Only binary files changed.")); + + return 0; +} + int run_add_p(struct repository *r, enum add_p_mode mode, struct interactive_options *opts, const char *revision, const struct pathspec *ps) { struct add_p_state s = { .r = r, + .index = r->index, + .index_file = r->index_file, .answer = STRBUF_INIT, .buf = STRBUF_INIT, .plain = STRBUF_INIT, .colored = STRBUF_INIT, }; - size_t i, binary_count = 0; + int ret; interactive_config_init(&s.cfg, r, opts); @@ -1969,23 +2000,93 @@ int run_add_p(struct repository *r, enum add_p_mode mode, if (repo_read_index(r) < 0 || (!s.mode->index_only && repo_refresh_and_write_index(r, REFRESH_QUIET, 0, 1, - NULL, NULL, NULL) < 0) || - parse_diff(&s, ps) < 0) { - add_p_state_clear(&s); - return -1; + NULL, NULL, NULL) < 0)) { + ret = -1; + goto out; } - for (i = 0; i < s.file_diff_nr; i++) - if (s.file_diff[i].binary && !s.file_diff[i].hunk_nr) - binary_count++; - else if (patch_update_file(&s, s.file_diff + i)) - break; + ret = run_add_p_common(&s, ps); + if (ret < 0) + goto out; - if (s.file_diff_nr == 0) - err(&s, _("No changes.")); - else if (binary_count == s.file_diff_nr) - err(&s, _("Only binary files changed.")); + ret = 0; +out: add_p_state_clear(&s); - return 0; + return ret; +} + +int run_add_p_index(struct repository *r, + struct index_state *index, + const char *index_file, + struct interactive_options *opts, + const char *revision, + const struct pathspec *ps) +{ + struct patch_mode mode = { + .apply_args = { "--cached", NULL }, + .apply_check_args = { "--cached", NULL }, + .prompt_mode = { + N_("Stage mode change [y,n,q,a,d%s,?]? "), + N_("Stage deletion [y,n,q,a,d%s,?]? "), + N_("Stage addition [y,n,q,a,d%s,?]? "), + N_("Stage this hunk [y,n,q,a,d%s,?]? ") + }, + .edit_hunk_hint = N_("If the patch applies cleanly, the edited hunk " + "will immediately be marked for staging."), + .help_patch_text = + N_("y - stage this hunk\n" + "n - do not stage this hunk\n" + "q - quit; do not stage this hunk or any of the remaining " + "ones\n" + "a - stage this hunk and all later hunks in the file\n" + "d - do not stage this hunk or any of the later hunks in " + "the file\n"), + .index_only = 1, + }; + struct add_p_state s = { + .r = r, + .index = index, + .index_file = index_file, + .answer = STRBUF_INIT, + .buf = STRBUF_INIT, + .plain = STRBUF_INIT, + .colored = STRBUF_INIT, + .mode = &mode, + .revision = revision, + }; + struct strbuf parent_revision = STRBUF_INIT; + char parent_tree_oid[GIT_MAX_HEXSZ + 1]; + struct commit *commit; + int ret; + + commit = lookup_commit_reference_by_name(revision); + if (!commit) { + err(&s, _("Revision does not refer to a commit")); + ret = -1; + goto out; + } + + if (commit->parents) + oid_to_hex_r(parent_tree_oid, get_commit_tree_oid(commit->parents->item)); + else + oid_to_hex_r(parent_tree_oid, r->hash_algo->empty_tree); + + strbuf_addf(&parent_revision, "%s~", revision); + mode.diff_cmd[0] = "diff-tree"; + mode.diff_cmd[1] = "-r"; + mode.diff_cmd[2] = parent_tree_oid; + + interactive_config_init(&s.cfg, r, opts); + + ret = run_add_p_common(&s, ps); + if (ret < 0) + goto out; + + ret = 0; + +out: + strbuf_release(&parent_revision); + add_p_state_clear(&s); + return ret; } diff --git a/add-patch.h b/add-patch.h index a4a05d9d14..901c42fd7b 100644 --- a/add-patch.h +++ b/add-patch.h @@ -3,6 +3,7 @@ #include "color.h" +struct index_state; struct pathspec; struct repository; @@ -53,4 +54,11 @@ int run_add_p(struct repository *r, enum add_p_mode mode, struct interactive_options *opts, const char *revision, const struct pathspec *ps); +int run_add_p_index(struct repository *r, + struct index_state *index, + const char *index_file, + struct interactive_options *opts, + const char *revision, + const struct pathspec *ps); + #endif -- GitLab From 6f8c443002cc005f971cb1c7d5ac022677947d6a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 2 Dec 2025 10:24:35 +0100 Subject: [PATCH 12/14] add-patch: allow disabling editing of hunks The "add-patch" mode allows the user to edit hunks to apply custom changes. This is incompatible with a new `git history split` command that we're about to introduce in a subsequent commit, so we need a way to disable this mode. Add a new flag to disable editing hunks. Signed-off-by: Patrick Steinhardt --- add-interactive.c | 2 +- add-patch.c | 21 +++++++++++++-------- add-patch.h | 11 +++++++++-- builtin/add.c | 2 +- builtin/checkout.c | 2 +- builtin/reset.c | 2 +- builtin/stash.c | 2 +- 7 files changed, 27 insertions(+), 15 deletions(-) diff --git a/add-interactive.c b/add-interactive.c index 05d2e7eefe..d41e039bc3 100644 --- a/add-interactive.c +++ b/add-interactive.c @@ -926,7 +926,7 @@ static int run_patch(struct add_i_state *s, const struct pathspec *ps, parse_pathspec(&ps_selected, PATHSPEC_ALL_MAGIC & ~PATHSPEC_LITERAL, PATHSPEC_LITERAL_PATH, "", args.v); - res = run_add_p(s->r, ADD_P_ADD, &opts, NULL, &ps_selected); + res = run_add_p(s->r, ADD_P_ADD, &opts, NULL, &ps_selected, 0); strvec_clear(&args); clear_pathspec(&ps_selected); } diff --git a/add-patch.c b/add-patch.c index 31d82a3e22..51a7b371a2 100644 --- a/add-patch.c +++ b/add-patch.c @@ -1565,7 +1565,8 @@ static bool get_first_undecided(const struct file_diff *file_diff, size_t *idx) } static int patch_update_file(struct add_p_state *s, - struct file_diff *file_diff) + struct file_diff *file_diff, + unsigned flags) { size_t hunk_index = 0; ssize_t i, undecided_previous, undecided_next, rendered_hunk_index = -1; @@ -1666,7 +1667,8 @@ static int patch_update_file(struct add_p_state *s, permitted |= ALLOW_SPLIT; strbuf_addstr(&s->buf, ",s"); } - if (hunk_index + 1 > file_diff->mode_change && + if (!(flags & ADD_P_DISALLOW_EDIT) && + hunk_index + 1 > file_diff->mode_change && !file_diff->deleted) { permitted |= ALLOW_EDIT; strbuf_addstr(&s->buf, ",e"); @@ -1932,7 +1934,8 @@ static int patch_update_file(struct add_p_state *s, } static int run_add_p_common(struct add_p_state *state, - const struct pathspec *ps) + const struct pathspec *ps, + unsigned flags) { size_t binary_count = 0; @@ -1942,7 +1945,7 @@ static int run_add_p_common(struct add_p_state *state, for (size_t i = 0; i < state->file_diff_nr; i++) { if (state->file_diff[i].binary && !state->file_diff[i].hunk_nr) binary_count++; - else if (patch_update_file(state, state->file_diff + i)) + else if (patch_update_file(state, state->file_diff + i, flags)) break; } @@ -1956,7 +1959,8 @@ static int run_add_p_common(struct add_p_state *state, int run_add_p(struct repository *r, enum add_p_mode mode, struct interactive_options *opts, const char *revision, - const struct pathspec *ps) + const struct pathspec *ps, + unsigned flags) { struct add_p_state s = { .r = r, @@ -2005,7 +2009,7 @@ int run_add_p(struct repository *r, enum add_p_mode mode, goto out; } - ret = run_add_p_common(&s, ps); + ret = run_add_p_common(&s, ps, flags); if (ret < 0) goto out; @@ -2021,7 +2025,8 @@ int run_add_p_index(struct repository *r, const char *index_file, struct interactive_options *opts, const char *revision, - const struct pathspec *ps) + const struct pathspec *ps, + unsigned flags) { struct patch_mode mode = { .apply_args = { "--cached", NULL }, @@ -2079,7 +2084,7 @@ int run_add_p_index(struct repository *r, interactive_config_init(&s.cfg, r, opts); - ret = run_add_p_common(&s, ps); + ret = run_add_p_common(&s, ps, flags); if (ret < 0) goto out; diff --git a/add-patch.h b/add-patch.h index 901c42fd7b..1facf19f96 100644 --- a/add-patch.h +++ b/add-patch.h @@ -50,15 +50,22 @@ enum add_p_mode { ADD_P_WORKTREE, }; +enum add_p_flags { + /* Disallow "editing" hunks. */ + ADD_P_DISALLOW_EDIT = (1 << 0), +}; + int run_add_p(struct repository *r, enum add_p_mode mode, struct interactive_options *opts, const char *revision, - const struct pathspec *ps); + const struct pathspec *ps, + unsigned flags); int run_add_p_index(struct repository *r, struct index_state *index, const char *index_file, struct interactive_options *opts, const char *revision, - const struct pathspec *ps); + const struct pathspec *ps, + unsigned flags); #endif diff --git a/builtin/add.c b/builtin/add.c index 6f1e213052..dfe9636079 100644 --- a/builtin/add.c +++ b/builtin/add.c @@ -172,7 +172,7 @@ int interactive_add(struct repository *repo, prefix, argv); if (patch) - ret = !!run_add_p(repo, ADD_P_ADD, interactive_opts, NULL, &pathspec); + ret = !!run_add_p(repo, ADD_P_ADD, interactive_opts, NULL, &pathspec, 0); else ret = !!run_add_i(repo, &pathspec, interactive_opts); diff --git a/builtin/checkout.c b/builtin/checkout.c index 530ae956ad..d325083ff3 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -576,7 +576,7 @@ static int checkout_paths(const struct checkout_opts *opts, BUG("either flag must have been set, worktree=%d, index=%d", opts->checkout_worktree, opts->checkout_index); return !!run_add_p(the_repository, patch_mode, &interactive_opts, - rev, &opts->pathspec); + rev, &opts->pathspec, 0); } repo_hold_locked_index(the_repository, &lock_file, LOCK_DIE_ON_ERROR); diff --git a/builtin/reset.c b/builtin/reset.c index 088449e120..008929bc7c 100644 --- a/builtin/reset.c +++ b/builtin/reset.c @@ -436,7 +436,7 @@ int cmd_reset(int argc, die(_("options '%s' and '%s' cannot be used together"), "--patch", "--{hard,mixed,soft}"); trace2_cmd_mode("patch-interactive"); update_ref_status = !!run_add_p(the_repository, ADD_P_RESET, - &interactive_opts, rev, &pathspec); + &interactive_opts, rev, &pathspec, 0); goto cleanup; } else { if (interactive_opts.context != -1) diff --git a/builtin/stash.c b/builtin/stash.c index 3b50905233..eb8142565e 100644 --- a/builtin/stash.c +++ b/builtin/stash.c @@ -1331,7 +1331,7 @@ static int stash_patch(struct stash_info *info, const struct pathspec *ps, old_index_env = xstrdup_or_null(getenv(INDEX_ENVIRONMENT)); setenv(INDEX_ENVIRONMENT, the_repository->index_file, 1); - ret = !!run_add_p(the_repository, ADD_P_STASH, interactive_opts, NULL, ps); + ret = !!run_add_p(the_repository, ADD_P_STASH, interactive_opts, NULL, ps, 0); the_repository->index_file = old_repo_index_file; if (old_index_env && *old_index_env) -- GitLab From f5f4ab21bbc44318ac2bcaef88fa1a69ed7036ad Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 14 Aug 2025 09:16:06 +0200 Subject: [PATCH 13/14] cache-tree: allow writing in-memory index as tree The function `write_in_core_index_as_tree()` takes a repository and writes its index into a tree object. What this function cannot do though is to take an _arbitrary_ in-memory index. Introduce a new `struct index_state` parameter so that the caller can pass a different index than the one belonging to the repository. This will be used in a subsequent commit. Signed-off-by: Patrick Steinhardt --- builtin/checkout.c | 3 ++- cache-tree.c | 4 ++-- cache-tree.h | 3 ++- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/builtin/checkout.c b/builtin/checkout.c index d325083ff3..aa00f0f483 100644 --- a/builtin/checkout.c +++ b/builtin/checkout.c @@ -902,7 +902,8 @@ static int merge_working_tree(const struct checkout_opts *opts, 0); init_ui_merge_options(&o, the_repository); o.verbosity = 0; - work = write_in_core_index_as_tree(the_repository); + work = write_in_core_index_as_tree(the_repository, + the_repository->index); ret = reset_tree(new_tree, opts, 1, diff --git a/cache-tree.c b/cache-tree.c index 2d8947b518..2976092270 100644 --- a/cache-tree.c +++ b/cache-tree.c @@ -723,11 +723,11 @@ static int write_index_as_tree_internal(struct object_id *oid, return 0; } -struct tree* write_in_core_index_as_tree(struct repository *repo) { +struct tree *write_in_core_index_as_tree(struct repository *repo, + struct index_state *index_state) { struct object_id o; int was_valid, ret; - struct index_state *index_state = repo->index; was_valid = index_state->cache_tree && cache_tree_fully_valid(index_state->cache_tree); diff --git a/cache-tree.h b/cache-tree.h index b82c4963e7..f8bddae523 100644 --- a/cache-tree.h +++ b/cache-tree.h @@ -47,7 +47,8 @@ int cache_tree_verify(struct repository *, struct index_state *); #define WRITE_TREE_UNMERGED_INDEX (-2) #define WRITE_TREE_PREFIX_ERROR (-3) -struct tree* write_in_core_index_as_tree(struct repository *repo); +struct tree *write_in_core_index_as_tree(struct repository *repo, + struct index_state *index_state); int write_index_as_tree(struct object_id *oid, struct index_state *index_state, const char *index_path, int flags, const char *prefix); void prime_cache_tree(struct repository *, struct index_state *, struct tree *); -- GitLab From 0d294bf94356d47589aa888361d9d9acf024c982 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 2 Dec 2025 09:37:31 +0100 Subject: [PATCH 14/14] builtin/history: implement "split" subcommand It is quite a common use case that one wants to split up one commit into multiple commits by moving parts of the changes of the original commit out into a separate commit. This is quite an involved operation though: 1. Identify the commit in question that is to be dropped. 2. Perform an interactive rebase on top of that commit's parent. 3. Modify the instruction sheet to "edit" the commit that is to be split up. 4. Drop the commit via "git reset HEAD~". 5. Stage changes that should go into the first commit and commit it. 6. Stage changes that should go into the second commit and commit it. 7. Finalize the rebase. This is quite complex, and overall I would claim that most people who are not experts in Git would struggle with this flow. Introduce a new "split" subcommand for git-history(1) to make this way easier. All the user needs to do is to say `git history split $COMMIT`. From hereon, Git asks the user which parts of the commit shall be moved out into a separate commit and, once done, asks the user for the commit message. Git then creates that split-out commit and applies the original commit on top of it. Signed-off-by: Patrick Steinhardt --- Documentation/git-history.adoc | 62 +++++ builtin/history.c | 174 +++++++++++++ t/meson.build | 1 + t/t3452-history-split.sh | 452 +++++++++++++++++++++++++++++++++ 4 files changed, 689 insertions(+) create mode 100755 t/t3452-history-split.sh diff --git a/Documentation/git-history.adoc b/Documentation/git-history.adoc index 160bf5d4d2..f1252baa2f 100644 --- a/Documentation/git-history.adoc +++ b/Documentation/git-history.adoc @@ -9,6 +9,7 @@ SYNOPSIS -------- [synopsis] git history reword +git history split [--] [...] DESCRIPTION ----------- @@ -37,11 +38,72 @@ Several commands are available to rewrite history in different ways: details of this commit remain unchanged. This command will spawn an editor with the current message of that commit. +`split [--] [...]`:: + Interactively split up into two commits by choosing + hunks introduced by it that will be moved into the new split-out + commit. These hunks will then be written into a new commit that + becomes the parent of the previous commit. The original commit + stays intact, except that its parent will be the newly split-out + commit. ++ +The commit messages of the split-up commits will be asked for by launching +the configured editor. Authorship of the commit will be the same as for the +original commit. ++ +If passed, __ can be used to limit which changes shall be split out +of the original commit. Files not matching any of the pathspecs will remain +part of the original commit. For more details, see the 'pathspec' entry in +linkgit:gitglossary[7]. ++ +It is invalid to select either all or no hunks, as that would lead to +one of the commits becoming empty. + CONFIGURATION ------------- include::includes/cmd-config-section-all.adoc[] +EXAMPLES +-------- + +Split a commit +~~~~~~~~~~~~~~ + +---------- +$ git log --stat --oneline +3f81232 (HEAD -> main) original + bar | 1 + + foo | 1 + + 2 files changed, 2 insertions(+) + +$ git history split HEAD +diff --git a/bar b/bar +new file mode 100644 +index 0000000..5716ca5 +--- /dev/null ++++ b/bar +@@ -0,0 +1 @@ ++bar +(1/1) Stage addition [y,n,q,a,d,e,p,?]? y + +diff --git a/foo b/foo +new file mode 100644 +index 0000000..257cc56 +--- /dev/null ++++ b/foo +@@ -0,0 +1 @@ ++foo +(1/1) Stage addition [y,n,q,a,d,e,p,?]? n + +$ git log --stat --oneline +7cebe64 (HEAD -> main) original + foo | 1 + + 1 file changed, 1 insertion(+) +d1582f3 split-out commit + bar | 1 + + 1 file changed, 1 insertion(+) +---------- + GIT --- Part of the linkgit:git[1] suite diff --git a/builtin/history.c b/builtin/history.c index 17bb150b95..c5262d964a 100644 --- a/builtin/history.c +++ b/builtin/history.c @@ -1,6 +1,7 @@ #define USE_THE_REPOSITORY_VARIABLE #include "builtin.h" +#include "cache-tree.h" #include "commit-reach.h" #include "commit.h" #include "config.h" @@ -8,17 +9,22 @@ #include "environment.h" #include "gettext.h" #include "hex.h" +#include "oidmap.h" #include "parse-options.h" +#include "path.h" +#include "read-cache.h" #include "refs.h" #include "replay.h" #include "reset.h" #include "revision.h" +#include "run-command.h" #include "sequencer.h" #include "strvec.h" #include "tree.h" #include "wt-status.h" #define GIT_HISTORY_REWORD_USAGE N_("git history reword ") +#define GIT_HISTORY_SPLIT_USAGE N_("git history split [--] [...]") static int collect_commits(struct repository *repo, struct commit *old_commit, @@ -364,6 +370,172 @@ static int cmd_history_reword(int argc, return ret; } +static int split_commit(struct repository *repo, + struct commit *original_commit, + struct pathspec *pathspec, + struct object_id *out) +{ + struct interactive_options interactive_opts = INTERACTIVE_OPTIONS_INIT; + struct strbuf index_file = STRBUF_INIT; + struct child_process read_tree_cmd = CHILD_PROCESS_INIT; + struct index_state index = INDEX_STATE_INIT(repo); + struct object_id original_commit_tree_oid, parent_tree_oid; + char original_commit_oid[GIT_MAX_HEXSZ + 1]; + struct commit_list *parents = NULL; + struct commit *first_commit; + struct tree *split_tree; + int ret; + + if (original_commit->parents) + parent_tree_oid = *get_commit_tree_oid(original_commit->parents->item); + else + oidcpy(&parent_tree_oid, repo->hash_algo->empty_tree); + original_commit_tree_oid = *get_commit_tree_oid(original_commit); + + /* + * Construct the first commit. This is done by taking the original + * commit parent's tree and selectively patching changes from the diff + * between that parent and its child. + */ + repo_git_path_replace(repo, &index_file, "%s", "history-split.index"); + + read_tree_cmd.git_cmd = 1; + strvec_pushf(&read_tree_cmd.env, "GIT_INDEX_FILE=%s", index_file.buf); + strvec_push(&read_tree_cmd.args, "read-tree"); + strvec_push(&read_tree_cmd.args, oid_to_hex(&parent_tree_oid)); + ret = run_command(&read_tree_cmd); + if (ret < 0) + goto out; + + ret = read_index_from(&index, index_file.buf, repo->gitdir); + if (ret < 0) { + ret = error(_("failed reading temporary index")); + goto out; + } + + oid_to_hex_r(original_commit_oid, &original_commit->object.oid); + ret = run_add_p_index(repo, &index, index_file.buf, &interactive_opts, + original_commit_oid, pathspec, ADD_P_DISALLOW_EDIT); + if (ret < 0) + goto out; + + split_tree = write_in_core_index_as_tree(repo, &index); + if (!split_tree) { + ret = error(_("failed split tree")); + goto out; + } + + unlink(index_file.buf); + + /* + * We disallow the cases where either the split-out commit or the + * original commit would become empty. Consequently, if we see that the + * new tree ID matches either of those trees we abort. + */ + if (oideq(&split_tree->object.oid, &parent_tree_oid)) { + ret = error(_("split commit is empty")); + goto out; + } else if (oideq(&split_tree->object.oid, &original_commit_tree_oid)) { + ret = error(_("split commit tree matches original commit")); + goto out; + } + + /* + * The first commit is constructed from the split-out tree. The base + * that shall be diffed against is the parent of the original commit. + */ + ret = commit_tree_with_edited_message(repo, "split-out", original_commit, + &split_tree->object.oid, + original_commit->parents, &parent_tree_oid, &out[0]); + if (ret < 0) { + ret = error(_("failed writing split-out commit")); + goto out; + } + + /* + * The second commit is constructed from the original tree. The base to + * diff against and the parent in this case is the first split-out + * commit. + */ + first_commit = lookup_commit_reference(repo, &out[0]); + commit_list_append(first_commit, &parents); + + ret = commit_tree_with_edited_message(repo, "split-out", original_commit, + &original_commit_tree_oid, + parents, get_commit_tree_oid(first_commit), &out[1]); + if (ret < 0) { + ret = error(_("failed writing split-out commit")); + goto out; + } + + ret = 0; + +out: + if (index_file.len) + unlink(index_file.buf); + strbuf_release(&index_file); + free_commit_list(parents); + release_index(&index); + return ret; +} + +static int cmd_history_split(int argc, + const char **argv, + const char *prefix, + struct repository *repo) +{ + const char * const usage[] = { + GIT_HISTORY_SPLIT_USAGE, + NULL, + }; + struct option options[] = { + OPT_END(), + }; + struct commit *original_commit, *parent, *head; + struct strvec commits = STRVEC_INIT; + struct object_id split_commits[2]; + struct pathspec pathspec = { 0 }; + int ret; + + argc = parse_options(argc, argv, prefix, options, usage, 0); + if (argc < 1) { + ret = error(_("command expects a revision")); + goto out; + } + repo_config(repo, git_default_config, NULL); + + parse_pathspec(&pathspec, 0, + PATHSPEC_PREFER_FULL | PATHSPEC_SYMLINK_LEADING_PATH | PATHSPEC_PREFIX_ORIGIN, + prefix, argv + 1); + + ret = gather_commits_between_head_and_revision(repo, argv[0], &original_commit, + &parent, &head, &commits); + if (ret < 0) + goto out; + + /* + * Then we split up the commit and replace the original commit with the + * new ones. + */ + ret = split_commit(repo, original_commit, &pathspec, split_commits); + if (ret < 0) + goto out; + + replace_commits(&commits, &original_commit->object.oid, + split_commits, ARRAY_SIZE(split_commits)); + + ret = apply_commits(repo, &commits, parent, head, "split"); + if (ret < 0) + goto out; + + ret = 0; + +out: + clear_pathspec(&pathspec); + strvec_clear(&commits); + return ret; +} + int cmd_history(int argc, const char **argv, const char *prefix, @@ -371,11 +543,13 @@ int cmd_history(int argc, { const char * const usage[] = { GIT_HISTORY_REWORD_USAGE, + GIT_HISTORY_SPLIT_USAGE, NULL, }; parse_opt_subcommand_fn *fn = NULL; struct option options[] = { OPT_SUBCOMMAND("reword", &fn, cmd_history_reword), + OPT_SUBCOMMAND("split", &fn, cmd_history_split), OPT_END(), }; diff --git a/t/meson.build b/t/meson.build index e187aef31e..1eef433207 100644 --- a/t/meson.build +++ b/t/meson.build @@ -388,6 +388,7 @@ integration_tests = [ 't3438-rebase-broken-files.sh', 't3450-history.sh', 't3451-history-reword.sh', + 't3452-history-split.sh', 't3500-cherry.sh', 't3501-revert-cherry-pick.sh', 't3502-cherry-pick-merge.sh', diff --git a/t/t3452-history-split.sh b/t/t3452-history-split.sh new file mode 100755 index 0000000000..02ae86a7e6 --- /dev/null +++ b/t/t3452-history-split.sh @@ -0,0 +1,452 @@ +#!/bin/sh + +test_description='tests for git-history split subcommand' + +. ./test-lib.sh + +# The fake editor takes multiple arguments, each of which represents a commit +# message. Subsequent invocations of the editor will then yield those messages +# in order. +# +set_fake_editor () { + printf "%s\n" "$@" >fake-input && + write_script fake-editor.sh <<-\EOF && + head -n1 fake-input >"$1" + sed 1d fake-input >fake-input.trimmed && + mv fake-input.trimmed fake-input + EOF + test_set_editor "$(pwd)"/fake-editor.sh +} + +expect_log () { + git log --format="%s" >actual && + cat >expect && + test_cmp expect actual +} + +expect_tree_entries () { + git ls-tree --name-only "$1" >actual && + cat >expect && + test_cmp expect actual +} + +test_expect_success 'refuses to work with merge commits' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit base && + git branch branch && + test_commit ours && + git switch branch && + test_commit theirs && + git switch - && + git merge theirs && + test_must_fail git history split HEAD 2>err && + test_grep "cannot rearrange commit history with merges" err && + test_must_fail git history split HEAD~ 2>err && + test_grep "cannot rearrange commit history with merges" err + ) +' + +test_expect_success 'refuses to work with unrelated commits' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit base && + git branch branch && + test_commit ours && + git switch branch && + test_commit theirs && + test_must_fail git history split ours 2>err && + test_grep "commit must be reachable from current HEAD commit" err + ) +' + +test_expect_success 'can split up tip commit' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit initial && + touch bar foo && + git add . && + git commit -m split-me && + + git symbolic-ref HEAD >expect && + set_fake_editor "first" "second" && + git history split HEAD <<-EOF && + y + n + EOF + git symbolic-ref HEAD >actual && + test_cmp expect actual && + + expect_log <<-EOF && + second + first + initial + EOF + + expect_tree_entries HEAD~ <<-EOF && + bar + initial.t + EOF + + expect_tree_entries HEAD <<-EOF && + bar + foo + initial.t + EOF + + git reflog >reflog && + test_grep "split: updating HEAD" reflog + ) +' + +test_expect_success 'can split up root commit' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m root && + test_commit tip && + + set_fake_editor "first" "second" && + git history split HEAD~ <<-EOF && + y + n + EOF + + expect_log <<-EOF && + tip + second + first + EOF + + expect_tree_entries HEAD~2 <<-EOF && + bar + EOF + + expect_tree_entries HEAD~ <<-EOF && + bar + foo + EOF + + expect_tree_entries HEAD <<-EOF + bar + foo + tip.t + EOF + ) +' + +test_expect_success 'can split up in-between commit' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit initial && + touch bar foo && + git add . && + git commit -m split-me && + test_commit tip && + + set_fake_editor "first" "second" && + git history split HEAD~ <<-EOF && + y + n + EOF + + expect_log <<-EOF && + tip + second + first + initial + EOF + + expect_tree_entries HEAD~2 <<-EOF && + bar + initial.t + EOF + + expect_tree_entries HEAD~ <<-EOF && + bar + foo + initial.t + EOF + + expect_tree_entries HEAD <<-EOF + bar + foo + initial.t + tip.t + EOF + ) +' + +test_expect_success 'can pick multiple hunks' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar baz foo qux && + git add . && + git commit -m split-me && + + set_fake_editor "first" "second" && + git history split HEAD <<-EOF && + y + n + y + n + EOF + + expect_tree_entries HEAD~ <<-EOF && + bar + foo + EOF + + expect_tree_entries HEAD <<-EOF + bar + baz + foo + qux + EOF + ) +' + + +test_expect_success 'can use only last hunk' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m split-me && + + set_fake_editor "first" "second" && + git history split HEAD <<-EOF && + n + y + EOF + + expect_log <<-EOF && + second + first + EOF + + expect_tree_entries HEAD~ <<-EOF && + foo + EOF + + expect_tree_entries HEAD <<-EOF + bar + foo + EOF + ) +' + +test_expect_success 'aborts with empty commit message' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m split-me && + + set_fake_editor "" && + test_must_fail git history split HEAD <<-EOF 2>err && + y + n + EOF + test_grep "Aborting commit due to empty commit message." err + ) +' + +test_expect_success 'commit message editor sees split-out changes' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m split-me && + + write_script fake-editor.sh <<-\EOF && + cat "$1" >>MESSAGES && + echo "some commit message" >"$1" + EOF + test_set_editor "$(pwd)"/fake-editor.sh && + + git history split HEAD <<-EOF && + y + n + EOF + + # Note that we expect to see the messages twice, once for each + # of the commits. The committed files are different though. + cat >expect <<-EOF && + split-me + + # Please enter the commit message for the split-out changes. Lines starting + # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit. + # Changes to be committed: + # new file: bar + # + split-me + + # Please enter the commit message for the split-out changes. Lines starting + # with ${SQ}#${SQ} will be ignored, and an empty message aborts the commit. + # Changes to be committed: + # new file: foo + # + EOF + test_cmp expect MESSAGES && + + expect_log <<-EOF + some commit message + some commit message + EOF + ) +' + +test_expect_success 'can use pathspec to limit what gets split' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m split-me && + + set_fake_editor "first" "second" && + git history split HEAD -- foo <<-EOF && + y + EOF + + expect_tree_entries HEAD~ <<-EOF && + foo + EOF + + expect_tree_entries HEAD <<-EOF + bar + foo + EOF + ) +' + +test_expect_success 'refuses to create empty split-out commit' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + test_commit base && + touch bar foo && + git add . && + git commit -m split-me && + + test_must_fail git history split HEAD 2>err <<-EOF && + n + n + EOF + test_grep "split commit is empty" err + ) +' + +test_expect_success 'hooks are not executed for rewritten commits' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m split-me && + old_head=$(git rev-parse HEAD) && + + ORIG_PATH="$(pwd)" && + export ORIG_PATH && + for hook in prepare-commit-msg pre-commit post-commit post-rewrite commit-msg + do + write_script .git/hooks/$hook <<-\EOF || exit 1 + touch "$ORIG_PATH/hooks.log + EOF + done && + + set_fake_editor "first" "second" && + git history split HEAD <<-EOF && + y + n + EOF + + expect_log <<-EOF && + second + first + EOF + + test_path_is_missing hooks.log + ) +' + +test_expect_success 'refuses to create empty original commit' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + touch bar foo && + git add . && + git commit -m split-me && + + test_must_fail git history split HEAD 2>err <<-EOF && + y + y + EOF + test_grep "split commit tree matches original commit" err + ) +' + +test_expect_success 'retains changes in the worktree and index' ' + test_when_finished "rm -rf repo" && + git init repo && + ( + cd repo && + echo a >a && + echo b >b && + git add . && + git commit -m "initial commit" && + echo a-modified >a && + echo b-modified >b && + git add b && + set_fake_editor "a-only" "remainder" && + git history split HEAD <<-EOF && + y + n + EOF + + expect_tree_entries HEAD~ <<-EOF && + a + EOF + expect_tree_entries HEAD <<-EOF && + a + b + EOF + + cat >expect <<-\EOF && + M a + M b + ?? actual + ?? expect + ?? fake-editor.sh + ?? fake-input + EOF + git status --porcelain >actual && + test_cmp expect actual + ) +' + +test_done -- GitLab