Re: [PATCH 1/2] rebase -i: fix counting of fixups after rebase --skip
From: Junio C Hamano <hidden>
Date: 2026-07-24 21:01:20
Phillip Wood [off-list ref] writes:
quoted hunk ↗ jump to hunk
@@ -3281,7 +3281,13 @@ static int read_populate_opts(struct replay_opts *opts) const char *p = ctx->current_fixups.buf; ctx->current_fixup_count = 1; while ((p = strchr(p, '\n'))) { - ctx->current_fixup_count++; + /* + * Older versions of git accidentally + * inserted blank lines when a fixup + * was skipped. + */ + if (p[1] != '\n') + ctx->current_fixup_count++; p++; } }
If we hit the LF at the very end (e.g. "fixup A\n" at the end of the file), strchr() would have moved p to the newline, and p[1] will be '\0', no? And because p[1] != '\n' and wouldn't current_fixup_count be incremented again? It might be safer to check p[1] != '\n' && p[1] != '\0' to avoid counting a trailing newline as an extra command when reading legacy files.
quoted hunk ↗ jump to hunk
@@ -5353,6 +5359,9 @@ static int commit_staged_changes(struct repository *r, if (!len) BUG("Incorrect current_fixups:\n%s", p); while (len && p[len - 1] != '\n') + len--; + /* Remove trailing newline */ + if (len) len--;
So we removed all the non newline from the end, and the loop would break if !len or p[len - 1] == '\n'. And in the latter case, we also drop that '\n'. Which sounds right. Thanks.