Re: [GSoC][PATCH v3 1/3] sequencer: add advice for revert

[Phillip Wood <phillip.wood123@gmail.com>]

On 13/06/2019 05:05, Rohit Ashiwal wrote:
> In the case of merge conflicts, while performing a revert, we are
> currently advised to use `git cherry-pick --<sequencer-options>`
> of which --continue is incompatible for continuing the revert.
> Introduce a separate advice message for `git revert`. Also change
> the signature of `create_seq_dir` to handle which advice to display
> selectively.
> 
> Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
> ---
> changes:
>     - Following Junio's advice[1], I've changed how we're handling error and
>       advice fundamentally.
> 
> [1]: https://public-inbox.org/git/20190608191958.4593-1-rohit.ashiwal265@gmail.com/T/#mf8eefb068b7246c7c2a02cc8f7120a1a903a1eba
> 
>  sequencer.c | 34 ++++++++++++++++++++++++++++------
>  1 file changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index f88a97fb10..918cb5d761 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -2650,15 +2650,37 @@ static int walk_revs_populate_todo(struct todo_list *todo_list,
>  	return 0;
>  }
>  
> -static int create_seq_dir(void)
> +static int create_seq_dir(struct repository *r)
>  {
> -	if (file_exists(git_path_seq_dir())) {
> -		error(_("a cherry-pick or revert is already in progress"));
> -		advise(_("try \"git cherry-pick (--continue | --quit | --abort)\""));
> +	enum replay_action action;
> +	const char *in_progress_advice;
> +	const char *in_progress_error = NULL;
> +
> +	if (!sequencer_get_last_command(r, &action)) {
> +		switch (action) {
> +		case REPLAY_REVERT:
> +			in_progress_error = _("revert is already in progress");
> +			in_progress_advice =
> +			_("try \"git revert (--continue | --abort | --quit)\"");
> +			break;
> +		case REPLAY_PICK:
> +			in_progress_error = _("cherry-pick is already in progress");
> +			in_progress_advice =
> +			_("try \"git cherry-pick (--continue | --abort | --quit)\"");
> +			break;
> +		default:
> +			BUG(_("the control must not reach here"));

This does not need to be translated as BUG() messages are not really for
users. Everything else looks fine to be now

Best Wishes

Phillip

> +		}
> +	}
> +	if (in_progress_error) {
> +		error("%s", in_progress_error);
> +		advise("%s", in_progress_advice);
>  		return -1;
> -	} else if (mkdir(git_path_seq_dir(), 0777) < 0)
> +	}
> +	if (mkdir(git_path_seq_dir(), 0777) < 0)
>  		return error_errno(_("could not create sequencer directory '%s'"),
>  				   git_path_seq_dir());
> +
>  	return 0;
>  }
>  
> @@ -4237,7 +4259,7 @@ int sequencer_pick_revisions(struct repository *r,
>  	 */
>  
>  	if (walk_revs_populate_todo(&todo_list, opts) ||
> -			create_seq_dir() < 0)
> +			create_seq_dir(r) < 0)
>  		return -1;
>  	if (get_oid("HEAD", &oid) && (opts->action == REPLAY_REVERT))
>  		return error(_("can't revert as initial commit"));
>