rebase -r: fix the total number shown in the progress

For regular, non-`--rebase-merges` runs, there is very little work to do for the parser when determining the total number of commands in a rebase script: it is simply the number of lines after stripping the commented lines and then trimming the trailing empty line, if any. The `--rebase-merges` mode complicates things by introducing empty lines and comments in the middle of the script. These should _not_ be counted as commands, and indeed, when an interactive rebase is interrupted and subsequently resumed, the total number of commands can magically shrink, sometimes dramatically. The reason for this strange behavior is that empty lines _are_ counted in `edit_todo_list()` (but not the comments, as they are stripped via `strbuf_stripspace(..., 1)`, which is a bug. Let's fix this so that the correct total number is shown from the get-go, by carefully adjusting it according to what's in the rebase script. Extra care needs to be taken in case the user edits the script: the number of commands might be different after the user edited than beforehand. Note: Even though commented lines are skipped in `edit_todo_list()`, we still need to handle `TODO_COMMENT` items by decrementing the already-incremented `total_nr` again: empty lines are also marked as `TODO_COMMENT`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Acked-by: Phillip Wood <phillip.wood@dunelm.org.uk> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Johannes Schindelin committed May 13, 2023 at 08:11 UTC 170eea9750e1bca7a35b5d27def9ee77df92fdf1
2 files changed +14 -3
sequencer.c
+6 -3
@@ -2453,7 +2453,6 @@ void todo_list_release(struct todo_list *todo_list)
2453 static struct todo_item *append_new_todo(struct todo_list *todo_list)
2454 {
2455 ALLOC_GROW(todo_list->items, todo_list->nr + 1, todo_list->alloc);
2456 - todo_list->total_nr++;
2456 return todo_list->items + todo_list->nr++;
2457 }
2458
@@ -2609,7 +2608,7 @@ int todo_list_parse_insn_buffer(struct repository *r, char *buf,
2608 char *p = buf, *next_p;
2609 int i, res = 0, fixup_okay = file_exists(rebase_path_done());
2610
2612 - todo_list->current = todo_list->nr = 0;
2611 + todo_list->current = todo_list->nr = todo_list->total_nr = 0;
2612
2613 for (i = 1; *p; i++, p = next_p) {
2614 char *eol = strchrnul(p, '\n');
@@ -2630,6 +2629,9 @@ int todo_list_parse_insn_buffer(struct repository *r, char *buf,
2629 item->commit = NULL;
2630 }
2631
2632 + if (item->command != TODO_COMMENT)
2633 + todo_list->total_nr++;
2634 +
2635 if (fixup_okay)
2636 ; /* do nothing */
2637 else if (is_fixup(item->command))
@@ -6096,7 +6098,8 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
6098 todo_list_to_strbuf(r, &new_todo, &buf2, -1, 0);
6099 strbuf_swap(&new_todo.buf, &buf2);
6100 strbuf_release(&buf2);
6099 - new_todo.total_nr -= new_todo.nr;
6101 + /* Nothing is done yet, and we're reparsing, so let's reset the count */
6102 + new_todo.total_nr = 0;
6103 if (todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo) < 0)
6104 BUG("invalid todo list after expanding IDs:\n%s",
6105 new_todo.buf.buf);
t/t3430-rebase-merges.sh
+8
@@ -517,4 +517,12 @@ test_expect_success '--rebase-merges with message matched with onto label' '
517 EOF
518 '
519
520 +test_expect_success 'progress shows the correct total' '
521 + git checkout -b progress H &&
522 + git rebase --rebase-merges --force-rebase --verbose A 2> err &&
523 + # Expecting "Rebasing (N/14)" here, no bogus total number
524 + grep "^Rebasing.*/14.$" err >progress &&
525 + test_line_count = 14 progress
526 +'
527 +
528 test_done