[prev in list] [next in list] [prev in thread] [next in thread] 

List:       git
Subject:    Re: [PATCH 17/22] sequencer: allow editing the commit message on a case-by-case basis
From:       Jakub_Narębski <jnareb () gmail ! com>
Date:       2016-08-31 20:56:15
Message-ID: fa2a0783-afaa-0aa5-877c-24b3513d4bae () gmail ! com
[Download RAW message or body]

W dniu 29.08.2016 o 10:06, Johannes Schindelin pisze:

> In the upcoming commits, we will implement more and more of rebase
> -i's functionality. One particular feature of the commands to come is
> that some of them allow editing the commit message while others don't,
> i.e. we cannot define in the replay_opts whether the commit message
> should be edited or not.

It's a nice, pretty and self contained refactoring step.  Small
enough that it is easy to review.

I would like to have in the commit message that it is sequencer_commit()
function that needs to rely on new parameter, instead of on a property
of command (of its replay_opts).  And that currently it simply passes
the buck to caller, which uses opts->edit, but in the future the
caller that is rebase -i would use todo_item and replay_opts based
expression.

> 
> Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
> ---
>  sequencer.c | 6 +++---
>  sequencer.h | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index e094ac2..7e17d14 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 allow_empty, int edit)
>  {
>  	char **env = NULL;
>  	struct argv_array array;
> @@ -513,7 +513,7 @@ int sequencer_commit(const char *defmsg, struct replay_opts *opts,
>  		argv_array_push(&array, "-s");
>  	if (defmsg)
>  		argv_array_pushl(&array, "-F", defmsg, NULL);
> -	if (opts->edit)
> +	if (edit)
>  		argv_array_push(&array, "-e");
>  	else if (!opts->signoff && !opts->record_origin &&
>  		 git_config_get_value("commit.cleanup", &value))
> @@ -779,7 +779,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, allow, opts->edit);
>  
>  leave:
>  	free_message(commit, &msg);
> diff --git a/sequencer.h b/sequencer.h
> index 9f63c31..fd02baf 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 allow_empty, int edit);
>  
>  extern const char sign_off_header[];
>  
> 

[prev in list] [next in list] [prev in thread] [next in thread]