| Message ID | 20190717143918.7406-6-alban.gruin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rebase -i: extend rebase.missingCommitsCheck to `--edit-todo' and co. | expand |
Alban Gruin <alban.gruin@gmail.com> writes: > The total amount of commands can be used to show the progression of the > rebasing in a shell. This number is written to the disk by > read_populate_todo() when the todo list is loaded from > sequencer_continue() or pick_commits(), but not by complete_action(), > which releases its todo list before calling sequencer_continue(), which > reloads it immediatly afterwards. > > To avoid to reload the todo list from the disk, sequencer_continue() > will be modified to accept a todo list, and if it is not null, > read_populate_todo() will not be called. That may be good as a plan, but readers who are reading this step are left puzzled as the changes so far do not seem to have much to do with that change. Perhaps this paragraph belongs to the step that actually makes that modification? > This moves the part writing total_nr to a new function so it can be > called by complete_action(). So without 3/9 and 4/9, we have been simply writing out wrong number? Good. > > Signed-off-by: Alban Gruin <alban.gruin@gmail.com> > --- > sequencer.c | 16 +++++++++++----- > 1 file changed, 11 insertions(+), 5 deletions(-) > > diff --git a/sequencer.c b/sequencer.c > index ec9c3d4dc5..d66b80d49f 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -2343,6 +2343,16 @@ void sequencer_post_commit_cleanup(struct repository *r, int verbose) > sequencer_remove_state(&opts); > } > > +static void todo_list_write_total_nr(struct todo_list *todo_list) > +{ > + FILE *f = fopen_or_warn(rebase_path_msgtotal(), "w"); > + > + if (f) { > + fprintf(f, "%d\n", todo_list->total_nr); > + fclose(f); > + } > +} > + > static int read_populate_todo(struct repository *r, > struct todo_list *todo_list, > struct replay_opts *opts) > @@ -2388,7 +2398,6 @@ static int read_populate_todo(struct repository *r, > > if (is_rebase_i(opts)) { > struct todo_list done = TODO_LIST_INIT; > - FILE *f = fopen_or_warn(rebase_path_msgtotal(), "w"); > > if (strbuf_read_file(&done.buf, rebase_path_done(), 0) > 0 && > !todo_list_parse_insn_buffer(r, done.buf.buf, &done)) > @@ -2400,10 +2409,7 @@ static int read_populate_todo(struct repository *r, > + count_commands(todo_list); > todo_list_release(&done); > > - if (f) { > - fprintf(f, "%d\n", todo_list->total_nr); > - fclose(f); > - } > + todo_list_write_total_nr(todo_list); > } > > return 0;
Hi, Le 18/07/2019 à 22:04, Junio C Hamano a écrit : > Alban Gruin <alban.gruin@gmail.com> writes: > > The total amount of commands can be used to show the progression of the > > rebasing in a shell. This number is written to the disk by > > read_populate_todo() when the todo list is loaded from > > sequencer_continue() or pick_commits(), but not by complete_action(), > > which releases its todo list before calling sequencer_continue(), which > > reloads it immediatly afterwards. > > > > To avoid to reload the todo list from the disk, sequencer_continue() > > will be modified to accept a todo list, and if it is not null, > > read_populate_todo() will not be called. > > That may be good as a plan, but readers who are reading this step > are left puzzled as the changes so far do not seem to have much to > do with that change. Perhaps this paragraph belongs to the step > that actually makes that modification? > > > This moves the part writing total_nr to a new function so it can be > > called by complete_action(). > > So without 3/9 and 4/9, we have been simply writing out wrong > number? Good. > Not here, the numbers computed in read_populate_todo() are correct. But they may be incorrect in complete_action(), before calling sequencer_continue(). This was not a big deal as the todo list was released before and sequencer_continue() re-read it, but it would have became a problem with 6/9. > > Signed-off-by: Alban Gruin <alban.gruin@gmail.com> > > --- > > > > sequencer.c | 16 +++++++++++----- > > 1 file changed, 11 insertions(+), 5 deletions(-) > > > > diff --git a/sequencer.c b/sequencer.c > > index ec9c3d4dc5..d66b80d49f 100644 > > --- a/sequencer.c > > +++ b/sequencer.c > > @@ -2343,6 +2343,16 @@ void sequencer_post_commit_cleanup(struct > > repository *r, int verbose)> > > sequencer_remove_state(&opts); > > > > } > > > > +static void todo_list_write_total_nr(struct todo_list *todo_list) > > +{ > > + FILE *f = fopen_or_warn(rebase_path_msgtotal(), "w"); > > + > > + if (f) { > > + fprintf(f, "%d\n", todo_list->total_nr); > > + fclose(f); > > + } > > +} > > + > > > > static int read_populate_todo(struct repository *r, > > > > struct todo_list *todo_list, > > struct replay_opts *opts) > > > > @@ -2388,7 +2398,6 @@ static int read_populate_todo(struct repository *r, > > > > if (is_rebase_i(opts)) { > > > > struct todo_list done = TODO_LIST_INIT; > > > > - FILE *f = fopen_or_warn(rebase_path_msgtotal(), "w"); > > > > if (strbuf_read_file(&done.buf, rebase_path_done(), 0) > 0 && > > > > !todo_list_parse_insn_buffer(r, done.buf.buf, &done)) > > > > @@ -2400,10 +2409,7 @@ static int read_populate_todo(struct repository *r, > > > > + count_commands(todo_list); > > > > todo_list_release(&done); > > > > - if (f) { > > - fprintf(f, "%d\n", todo_list->total_nr); > > - fclose(f); > > - } > > + todo_list_write_total_nr(todo_list); > > > > } > > > > return 0;
diff --git a/sequencer.c b/sequencer.c index ec9c3d4dc5..d66b80d49f 100644 --- a/sequencer.c +++ b/sequencer.c @@ -2343,6 +2343,16 @@ void sequencer_post_commit_cleanup(struct repository *r, int verbose) sequencer_remove_state(&opts); } +static void todo_list_write_total_nr(struct todo_list *todo_list) +{ + FILE *f = fopen_or_warn(rebase_path_msgtotal(), "w"); + + if (f) { + fprintf(f, "%d\n", todo_list->total_nr); + fclose(f); + } +} + static int read_populate_todo(struct repository *r, struct todo_list *todo_list, struct replay_opts *opts) @@ -2388,7 +2398,6 @@ static int read_populate_todo(struct repository *r, if (is_rebase_i(opts)) { struct todo_list done = TODO_LIST_INIT; - FILE *f = fopen_or_warn(rebase_path_msgtotal(), "w"); if (strbuf_read_file(&done.buf, rebase_path_done(), 0) > 0 && !todo_list_parse_insn_buffer(r, done.buf.buf, &done)) @@ -2400,10 +2409,7 @@ static int read_populate_todo(struct repository *r, + count_commands(todo_list); todo_list_release(&done); - if (f) { - fprintf(f, "%d\n", todo_list->total_nr); - fclose(f); - } + todo_list_write_total_nr(todo_list); } return 0;
The total amount of commands can be used to show the progression of the rebasing in a shell. This number is written to the disk by read_populate_todo() when the todo list is loaded from sequencer_continue() or pick_commits(), but not by complete_action(), which releases its todo list before calling sequencer_continue(), which reloads it immediatly afterwards. To avoid to reload the todo list from the disk, sequencer_continue() will be modified to accept a todo list, and if it is not null, read_populate_todo() will not be called. This moves the part writing total_nr to a new function so it can be called by complete_action(). Signed-off-by: Alban Gruin <alban.gruin@gmail.com> --- sequencer.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-)