Re: [PATCH v2 17/25] sequencer: support amending commits
Hi Junio, On Mon, 12 Sep 2016, Junio C Hamano wrote: > Johannes Schindelin writes: > > > This teaches the sequencer_commit() function to take an argument that > > will allow us to implement "todo" commands that need to amend the commit > > messages ("fixup", "squash" and "reword"). > > > > Signed-off-by: Johannes Schindelin > > --- > > sequencer.c | 6 -- > > sequencer.h | 2 +- > > 2 files changed, 5 insertions(+), 3 deletions(-) > > > > diff --git a/sequencer.c b/sequencer.c > > index 6e9732c..60b522e 100644 > > --- a/sequencer.c > > +++ b/sequencer.c > > @@ -485,7 +485,7 @@ static char **read_author_script(void) > > * author metadata. > > */ > > int sequencer_commit(const char *defmsg, struct replay_opts *opts, > > - int allow_empty, int edit) > > + int allow_empty, int edit, int amend) > > { > > char **env = NULL; > > struct argv_array array; > > @@ -514,6 +514,8 @@ int sequencer_commit(const char *defmsg, struct > > replay_opts *opts, > > argv_array_push(&array, "commit"); > > argv_array_push(&array, "-n"); > > > > + if (amend) > > + argv_array_push(&array, "--amend"); > > if (opts->gpg_sign) > > argv_array_pushf(&array, "-S%s", opts->gpg_sign); > > if (opts->signoff) > > @@ -786,7 +788,7 @@ static int do_pick_commit(enum todo_command command, > > struct commit *commit, > > } > > if (!opts->no_commit) > > res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(), > > - opts, allow, opts->edit); > > + opts, allow, opts->edit, 0); > > > > leave: > > free_message(commit, &msg); > > Hmm, this is more about a comment on 18/25, but I suspect that > "amend" or any opportunity given to the user to futz with the > contents in the editor invites a wish for the result to be treated > with stripspace. The point of separating the cleanup_commit_message from the edit flag is to allow final fixup commands to stripspace, even without letting the user edit the message. So while you are correct that "edit != 0" should imply "cleanup_commit_message != 0", I would rather keep that explicit. > No existing callers use "amend" to call this function, so there is > no change in behaviour, but at the same time, we do not have enough > information to see if 'amend' should by default toggle cleanup. It should not. Non-final fixup/squash commands *need* to keep the comment lines. Keeping things explicit makes it easier to understand the flow, methinks. Ciao, Dscho
Re: [PATCH v2 17/25] sequencer: support amending commits
Johannes Schindelin writes: > This teaches the sequencer_commit() function to take an argument that > will allow us to implement "todo" commands that need to amend the commit > messages ("fixup", "squash" and "reword"). > > Signed-off-by: Johannes Schindelin > --- > sequencer.c | 6 -- > sequencer.h | 2 +- > 2 files changed, 5 insertions(+), 3 deletions(-) > > diff --git a/sequencer.c b/sequencer.c > index 6e9732c..60b522e 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -485,7 +485,7 @@ static char **read_author_script(void) > * author metadata. > */ > int sequencer_commit(const char *defmsg, struct replay_opts *opts, > - int allow_empty, int edit) > + int allow_empty, int edit, int amend) > { > char **env = NULL; > struct argv_array array; > @@ -514,6 +514,8 @@ int sequencer_commit(const char *defmsg, struct > replay_opts *opts, > argv_array_push(&array, "commit"); > argv_array_push(&array, "-n"); > > + if (amend) > + argv_array_push(&array, "--amend"); > if (opts->gpg_sign) > argv_array_pushf(&array, "-S%s", opts->gpg_sign); > if (opts->signoff) > @@ -786,7 +788,7 @@ static int do_pick_commit(enum todo_command command, > struct commit *commit, > } > if (!opts->no_commit) > res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(), > - opts, allow, opts->edit); > + opts, allow, opts->edit, 0); > > leave: > free_message(commit, &msg); Hmm, this is more about a comment on 18/25, but I suspect that "amend" or any opportunity given to the user to futz with the contents in the editor invites a wish for the result to be treated with stripspace. No existing callers use "amend" to call this function, so there is no change in behaviour, but at the same time, we do not have enough information to see if 'amend' should by default toggle cleanup. > diff --git a/sequencer.h b/sequencer.h > index 7f5222f..c45f5c4 100644 > --- a/sequencer.h > +++ b/sequencer.h > @@ -54,7 +54,7 @@ int sequencer_rollback(struct replay_opts *opts); > int sequencer_remove_state(struct replay_opts *opts); > > int sequencer_commit(const char *defmsg, struct replay_opts *opts, > - int allow_empty, int edit); > + int allow_empty, int edit, int amend); > > extern const char sign_off_header[];
[PATCH v2 17/25] sequencer: support amending commits
This teaches the sequencer_commit() function to take an argument that will allow us to implement "todo" commands that need to amend the commit messages ("fixup", "squash" and "reword"). Signed-off-by: Johannes Schindelin --- sequencer.c | 6 -- sequencer.h | 2 +- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/sequencer.c b/sequencer.c index 6e9732c..60b522e 100644 --- a/sequencer.c +++ b/sequencer.c @@ -485,7 +485,7 @@ static char **read_author_script(void) * author metadata. */ int sequencer_commit(const char *defmsg, struct replay_opts *opts, - int allow_empty, int edit) + int allow_empty, int edit, int amend) { char **env = NULL; struct argv_array array; @@ -514,6 +514,8 @@ int sequencer_commit(const char *defmsg, struct replay_opts *opts, argv_array_push(&array, "commit"); argv_array_push(&array, "-n"); + if (amend) + argv_array_push(&array, "--amend"); if (opts->gpg_sign) argv_array_pushf(&array, "-S%s", opts->gpg_sign); if (opts->signoff) @@ -786,7 +788,7 @@ static int do_pick_commit(enum todo_command command, struct commit *commit, } if (!opts->no_commit) res = sequencer_commit(opts->edit ? NULL : git_path_merge_msg(), - opts, allow, opts->edit); + opts, allow, opts->edit, 0); leave: free_message(commit, &msg); diff --git a/sequencer.h b/sequencer.h index 7f5222f..c45f5c4 100644 --- a/sequencer.h +++ b/sequencer.h @@ -54,7 +54,7 @@ int sequencer_rollback(struct replay_opts *opts); int sequencer_remove_state(struct replay_opts *opts); int sequencer_commit(const char *defmsg, struct replay_opts *opts, - int allow_empty, int edit); + int allow_empty, int edit, int amend); extern const char sign_off_header[]; -- 2.10.0.windows.1.10.g803177d