On Tue, Aug 26, 2025 at 09:14:49AM -0400, D. Ben Knoble wrote:
> On Sun, Aug 24, 2025 at 1:44 PM Patrick Steinhardt <ps@xxxxxx> wrote:
> > 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,
[snip]
> > + 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.
Yeah, it's supposed to show the diff between old and new tree indeed. So
in theory you should see something.
> 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.
Do you maybe have a reproducer for this? It seems to work alright for
me, but I wouldn't be surprised if there was a bug here. The wt-status
interfaces are quite something and I was tearing my hair while trying to
figure them out.
> It doesn't look like it's triggered only on commit.verbose config, either.
Fixed now.
> > +
> > + 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 ;)
True. I didn't want to add too many options right from the start to keep
the series somewhat simple. We should eventually add it though.
> > + 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.
Yeah, I don't dare touching this yet, but certainly see that this might
be a worthwhile addition as we iterate on this command.
Patrick
