Re: [PATCH RFC v2 15/16] builtin/history: implement "split" subcommand

From: D. Ben Knoble <ben.knoble@gmail.com>
Date: Aug 26, 2025, 13:14 UTC
Message-ID: <CALnO6CBRv-Jvm1FsUAkDSyaNNRXtMukReKzefvCw5-vj=-pnCA@mail.gmail.com>
In-Reply-To: <20250824-b4-pks-history-builtin-v2-15-964ac12f65bd@pks.im>

On Sun, Aug 24, 2025 at 1:44 PM Patrick Steinhardt <ps@pks.im> wrote:

Show 54 quoted lines

> diff --git a/builtin/history.c b/builtin/history.c
> index 16b516856e..6d3f44152c 100644
> --- a/builtin/history.c
> +++ b/builtin/history.c
> @@ -517,6 +527,285 @@ static int cmd_history_reorder(int argc,
>         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 *provided_message,
> +                              const char *action,
> +                              struct strbuf *out)
> +{
> +       if (!provided_message) {
> +               struct wt_status s;
> +               const char *path = git_path_commit_editmsg();
> +               const char *hint =
> +                       _("Please enter the commit message for the %s changes. Lines starting\n"
> +                         "with '%s' will be kept; you may remove them yourself if you want to.\n");
> +
> +               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);

I think I'm supposed to see the changes between the old and new trees, right? Does this only happen if I use the interactive machinery to edit a hunk? When I try accepting some changes and leaving others for the next commit I get no diff in the template.

I did try to add new diff lines to a hunk, and nothing showed up… maybe I'm holding it wrong? I'm pretty sure I compiled this version.

It doesn't look like it's triggered only on commit.verbose config, either.

> +
> +               strbuf_reset(out);
> +               if (launch_editor(path, out, NULL)) {
> +                       fprintf(stderr, _("Please supply the message using either -m or -F option.\n"));

According to the usage, git history split only supports -m, not -F ;)

Show 12 quoted lines

> +                       return -1;
> +               }
> +               strbuf_stripspace(out, comment_line_str);
> +
> +       } else {
> +               strbuf_addstr(out, provided_message);
> +       }
> +
> +       cleanup_message(out, COMMIT_MSG_CLEANUP_ALL, 0);
> +
> +       if (!out->len) {
> +               fprintf(stderr, _("Aborting commit due to empty commit message.\n"));

It _would_ be nice if this and similar errors left me able to "try again" without losing staged changes—I think I mentioned this before, though. And with the in-memory indices vs. actual working state, presenting a UI here could be very difficult. So it's an understandable choice.

-- 
D. Ben Knoble