[GSoC,v4,1/4] sequencer: add advice for revert
diff mbox series

Message ID 20190616082040.9440-2-rohit.ashiwal265@gmail.com
State New
Headers show
Series
  • Teach cherry-pick/revert to skip commits
Related show

Commit Message

Rohit Ashiwal June 16, 2019, 8:20 a.m. UTC
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:
    - change BUG()'s message under create_seq_dir

 sequencer.c | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

Comments

Thomas Gummerer June 17, 2019, 5:51 a.m. UTC | #1
On 06/16, 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:
>     - change BUG()'s message under create_seq_dir
> 
>  sequencer.c | 34 ++++++++++++++++++++++++++++------
>  1 file changed, 28 insertions(+), 6 deletions(-)
> 
> diff --git a/sequencer.c b/sequencer.c
> index f88a97fb10..d80e1c3fbb 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_error = NULL;
> +	const char *in_progress_advice = 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(_("unexpected action in create_seq_dir"));

As Phillip mentioned in the previous round, this doesn't need to be
translated.  I'd go one step further and say this should not be
translated.  Translating it just adds extra work for translators for a
message that the user will (hopefully) never see.

In fact if we look through the rest of the codebase, BUG() messages
are never translated anywhere, and neither is the one in 3/4.

> +		}
> +	}
> +	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"));
> -- 
> 2.21.0
>

Patch
diff mbox series

diff --git a/sequencer.c b/sequencer.c
index f88a97fb10..d80e1c3fbb 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_error = NULL;
+	const char *in_progress_advice = 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(_("unexpected action in create_seq_dir"));
+		}
+	}
+	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"));