> Le 19 août 2025 à 06:57, Patrick Steinhardt <ps@xxxxxx> a écrit :
>
> It is a fairly common operation to perform an interactive rebase so that
> one of the commits can be dropped from history.
> diff --git a/builtin/history.c b/builtin/history.c
> index d1a40368e0..183ab9d5f7 100644
> --- a/builtin/history.c
> +++ b/builtin/history.c
> @@ -1,20 +1,311 @@
> #include "builtin.h"
> +#include "commit.h"
> +#include "commit-reach.h"
> +#include "config.h"
> +#include "environment.h"
> #include "gettext.h"
> +#include "hex.h"
> +#include "object-name.h"
> #include "parse-options.h"
> +#include "refs.h"
> +#include "reset.h"
> +#include "revision.h"
> +#include "sequencer.h"
> +
> +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_list *from_list = NULL;
> + struct commit *child;
> + struct rev_info rev = { 0 };
> + int ret;
> +
> + /*
> + * Check that the old actually is an ancestor of HEAD. If not
The “old commit” perhaps?
> + * the whole request becomes nonsensical.
> + */
> + if (old_commit) {
> + commit_list_insert(old_commit, &from_list);
> + if (!repo_is_descendant_of(repo, new_commit, from_list)) {
> + ret = error(_("commit must be reachable from current HEAD commit"));
> + goto out;
> + }
> + }
> +
> + repo_init_revisions(repo, &rev, NULL);
> + 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));
> + if (setup_revisions(revisions.nr, revisions.v, &rev, &revision_opts) != 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));
> +
> + if (child->parents && old_commit &&
> + commit_list_contains(old_commit, child->parents))
> + break;
> + }
> +
> + /*
> + * Revisions are in newest-order-first. We have to reverse the
> + * array though so that we pick the oldest commits first. Note
> + * that we keep the first string untouched, as it is the
> + * equivalent of `argv[0]` to `setup_revisions()`.
> + */
> + for (size_t i = 0, j = out->nr - 1; i < j; i++, j--)
> + SWAP(out->v[i], out->v[j]);
> +
But doesn’t this swap out->v[0] on first iteration? I only skimmed the code that built it up, but it doesn’t look the comment is right 🤔
Rest looked reasonable, but I don’t know the sequencer APIs very well.
