From: Matt Hunter Date: Sat, 18 Jul 2026 08:52:24 GMT Subject: Re: [PATCH v9 3/5] history: add squash subcommand to fold a range Message-ID: In-Reply-To: Hi Harald, The new functionality for amend! messages seems to be working well, so I dug a little deeper and found the following... On Wed Jul 15, 2026 at 11:16 AM EDT, Harald Nordgren via GitGitGadget wrote: > ++ > +The range is given in the usual `..` form, where __ is > +the commit just below the oldest commit to squash. For example, `git > +history squash HEAD~3..HEAD` folds the three most recent commits into > +one, and `git history squash HEAD~5..HEAD~2` squashes an interior range > +while leaving the two newest commits in place. Several revisions may be > +given, for example `HEAD~3..HEAD ^topic` to additionally exclude what is > +already on `topic`. Rev-list options may also be given, but any that would > +change how the range is walked are overridden with a warning. > ++ > +The oldest commit's message is preserved by default, except that an `amend!` > +commit targeting it replaces its message. The new behavior from v9 is documented here, but... > Specify `--reedit-message` to edit > +the resulting message. A merge commit inside the range is folded like any > +other, but the range must have a single base, so a range that reaches more > +than one entry point (for example a side branch that forked before the range > +and was later merged into it) is rejected. > ++ > +A `fixup!`, `squash!`, or `amend!` commit is refused unless the commit it > +targets is also in the range, so the fold does not silently absorb a > +marker meant for a commit outside it. The body after an `amend!` subject > +replaces the oldest commit's message when the marker targets that commit. ...a redundant explanation appears here too. Personally, I think this paragraph flows better if the 'The body after an `amend!`...targets that commit.' sentence were removed. > +As an exception, a range made up entirely of markers for one target is combined > +into a single commit, keeping the last `amend!` message if there is one. > ++ > +A branch or tag that points at a commit inside the range would be left > +dangling once those commits are folded away, so with the default > +`--update-refs=branches` the command refuses. Rerun with > +`--update-refs=head` to rewrite only the current branch and leave such > +refs pointing at the old commits. > + > OPTIONS > ------- > > @@ -107,7 +147,8 @@ OPTIONS > ref updates is generally safe. > > `--reedit-message`:: > - Open an editor to modify the target commit's message. > + Open an editor to modify the rewritten commit's message. For `squash` > + the editor is pre-filled with the messages of all the folded commits. At the moment of this patch, this is a false statement, though it is made true by patch 5/5 pre-filling all messages. > diff --git a/builtin/history.c b/builtin/history.c > index cbba25096f..edf98a21d3 100644 > --- a/builtin/history.c > +++ b/builtin/history.c > + > + repo_init_revisions(repo, &revs, NULL); > + revs.reverse = 1; > + revs.topo_order = 1; > + revs.sort_order = REV_SORT_IN_GRAPH_ORDER; > + revs.simplify_history = 0; > + revs.boundary = 1; > + > + strvec_push(&args, "ignored"); > + strvec_push(&args, "--ancestry-path"); > + strvec_pushv(&args, argv); > + setup_revisions_from_strvec(&args, &revs, NULL); > + if (args.nr != 1) { > + ret = error(_("unrecognized argument: %s"), args.v[1]); > + goto out; > + } > + > + if (revs.reverse != 1 || revs.topo_order != 1 || > + revs.sort_order != REV_SORT_IN_GRAPH_ORDER || > + revs.simplify_history != 0) { > + warning(_("ignoring rev-list options that would change how the " > + "range is walked")); > + revs.reverse = 1; > + revs.topo_order = 1; > + revs.sort_order = REV_SORT_IN_GRAPH_ORDER; > + revs.simplify_history = 0; > + } Should revs.boundary still == 1 be asserted here too? > + > + base_tree_oid = &repo_get_commit_tree(repo, base)->object.oid; > + tip_tree_oid = &repo_get_commit_tree(repo, tip)->object.oid; > + commit_list_append(base, &parents); > + > + ret = commit_tree_ext(repo, "squash", msg_source, message_template, > + parents, > + base_tree_oid, tip_tree_oid, &rewritten, flags); > + if (ret < 0) { > + ret = error(_("failed writing squashed commit")); > + goto out; > + } > + > + strbuf_addf(&reflog_msg, "squash: updating %s", argv[0]); With this format string, the reflog will miss cases like: git history squash HEAD~5..HEAD ^origin/master Only "squash: updating HEAD~5..HEAD" will be recorded in the log.