Re: [PATCH 18/22] sequencer: support amending commits

2016-09-01 Thread Johannes Schindelin
Hi Kuba,

On Wed, 31 Aug 2016, Jakub Narębski wrote:

> W dniu 29.08.2016 o 10:06, Johannes Schindelin pisze:
> 
> > diff --git a/sequencer.c b/sequencer.c
> > index 7e17d14..20f7590 100644
> > --- a/sequencer.c
> > +++ b/sequencer.c
> > @@ -478,7 +478,7 @@ static char **read_author_script(void)
> >   * (except, of course, while running an interactive rebase).
> >   */
> >  int sequencer_commit(const char *defmsg, struct replay_opts *opts,
> > - int allow_empty, int edit)
> > + int allow_empty, int edit, int amend)
> 
> I guess we won't get much more parameters; it would get unwieldy
> (and hard to remember).  Five is all right.

It will eventually get a sixth, cleanup_commit_message.

> >  {
> > char **env = NULL;
> > struct argv_array array;
> > @@ -507,6 +507,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)
> > @@ -779,7 +781,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);
> 
> ... even of this makes one need to check the calling convention,
> what does this 0 mean.

Yeah, but it will not remain "0", but be replaced by a variable named
"amend"...

Thanks for the review,
Dscho

Re: [PATCH 18/22] sequencer: support amending commits

2016-08-31 Thread Jakub Narębski
W dniu 29.08.2016 o 10:06, Johannes Schindelin pisze:

> 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(-)

Nice and small addition of a new feature, a scaffolding for implementing
rebase -i using the sequencer.

> 
> diff --git a/sequencer.c b/sequencer.c
> index 7e17d14..20f7590 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -478,7 +478,7 @@ static char **read_author_script(void)
>   * (except, of course, while running an interactive rebase).
>   */
>  int sequencer_commit(const char *defmsg, struct replay_opts *opts,
> -   int allow_empty, int edit)
> +   int allow_empty, int edit, int amend)

I guess we won't get much more parameters; it would get unwieldy
(and hard to remember).  Five is all right.

>  {
>   char **env = NULL;
>   struct argv_array array;
> @@ -507,6 +507,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)
> @@ -779,7 +781,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);

... even of this makes one need to check the calling convention,
what does this 0 mean.

>  
>  leave:
>   free_message(commit, &msg);
> diff --git a/sequencer.h b/sequencer.h
> index fd02baf..2106c0d 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -50,7 +50,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[];
>  
>