From: "Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com>
To: git@vger.kernel.org
Cc: Derrick Stolee <derrickstolee@github.com>,
Johannes Schindelin <johannes.schindelin@gmx.de>,
Johannes Schindelin <johannes.schindelin@gmx.de>
Subject: [PATCH 2/2] rebase -r: fix the total number shown in the progress
Date: Wed, 10 May 2023 22:55:38 +0000 [thread overview]
Message-ID: <d12d5469f8cbc21ce1efbffc8e7569c534b5a305.1683759339.git.gitgitgadget@gmail.com> (raw)
In-Reply-To: <pull.1531.git.1683759338.gitgitgadget@gmail.com>
From: Johannes Schindelin <johannes.schindelin@gmx.de>
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>
---
sequencer.c | 12 ++++++++----
t/t3430-rebase-merges.sh | 7 +++++++
2 files changed, 15 insertions(+), 4 deletions(-)
diff --git a/sequencer.c b/sequencer.c
index f5d89abdc5e..46dd07df0f2 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -2609,7 +2609,7 @@ int todo_list_parse_insn_buffer(struct repository *r, char *buf,
char *p = buf, *next_p;
int i, res = 0, fixup_okay = file_exists(rebase_path_done());
- todo_list->current = todo_list->nr = 0;
+ todo_list->current = todo_list->nr = todo_list->total_nr = 0;
for (i = 1; *p; i++, p = next_p) {
char *eol = strchrnul(p, '\n');
@@ -2628,7 +2628,8 @@ int todo_list_parse_insn_buffer(struct repository *r, char *buf,
item->arg_offset = p - buf;
item->arg_len = (int)(eol - p);
item->commit = NULL;
- }
+ } else if (item->command == TODO_COMMENT)
+ todo_list->total_nr--;
if (fixup_okay)
; /* do nothing */
@@ -6039,7 +6040,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
struct todo_list new_todo = TODO_LIST_INIT;
struct strbuf *buf = &todo_list->buf, buf2 = STRBUF_INIT;
struct object_id oid = onto->object.oid;
- int res;
+ int new_total_nr, res;
find_unique_abbrev_r(shortonto, &oid, DEFAULT_ABBREV);
@@ -6066,6 +6067,7 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
return error(_("nothing to do"));
}
+ new_total_nr = todo_list->total_nr - count_commands(todo_list);
res = edit_todo_list(r, todo_list, &new_todo, shortrevisions,
shortonto, flags);
if (res == -1)
@@ -6088,11 +6090,13 @@ int complete_action(struct repository *r, struct replay_opts *opts, unsigned fla
return -1;
}
+ new_total_nr += count_commands(&new_todo);
+ new_todo.total_nr = new_total_nr;
+
/* Expand the commit IDs */
todo_list_to_strbuf(r, &new_todo, &buf2, -1, 0);
strbuf_swap(&new_todo.buf, &buf2);
strbuf_release(&buf2);
- new_todo.total_nr -= new_todo.nr;
if (todo_list_parse_insn_buffer(r, new_todo.buf.buf, &new_todo) < 0)
BUG("invalid todo list after expanding IDs:\n%s",
new_todo.buf.buf);
diff --git a/t/t3430-rebase-merges.sh b/t/t3430-rebase-merges.sh
index f351701fec2..8da99a075dc 100755
--- a/t/t3430-rebase-merges.sh
+++ b/t/t3430-rebase-merges.sh
@@ -517,4 +517,11 @@ test_expect_success '--rebase-merges with message matched with onto label' '
EOF
'
+test_expect_success 'progress shows the correct total' '
+ git checkout -b progress H &&
+ git rebase --rebase-merges --force-rebase --verbose A 2> err &&
+ grep "^Rebasing.*14.$" err >progress &&
+ test_line_count = 14 progress
+'
+
test_done
--
gitgitgadget
next prev parent reply other threads:[~2023-05-10 22:56 UTC|newest]
Thread overview: 11+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-05-10 22:55 [PATCH 0/2] Fix two rebase bugs related to total_nr Johannes Schindelin via GitGitGadget
2023-05-10 22:55 ` [PATCH 1/2] rebase --update-refs: fix loops Johannes Schindelin via GitGitGadget
2023-05-10 23:25 ` Junio C Hamano
2023-05-10 22:55 ` Johannes Schindelin via GitGitGadget [this message]
2023-05-10 23:39 ` [PATCH 2/2] rebase -r: fix the total number shown in the progress Junio C Hamano
2023-05-11 14:13 ` Phillip Wood
2023-05-11 18:08 ` Junio C Hamano
2023-05-13 8:11 ` [PATCH v2 0/2] Fix two rebase bugs related to total_nr Johannes Schindelin via GitGitGadget
2023-05-13 8:11 ` [PATCH v2 1/2] rebase --update-refs: fix loops Johannes Schindelin via GitGitGadget
2023-05-13 8:11 ` [PATCH v2 2/2] rebase -r: fix the total number shown in the progress Johannes Schindelin via GitGitGadget
2023-05-14 17:59 ` [PATCH v2 0/2] Fix two rebase bugs related to total_nr Phillip Wood
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=d12d5469f8cbc21ce1efbffc8e7569c534b5a305.1683759339.git.gitgitgadget@gmail.com \
--to=gitgitgadget@gmail.com \
--cc=derrickstolee@github.com \
--cc=git@vger.kernel.org \
--cc=johannes.schindelin@gmx.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).