Re: [GSoC][PATCH v2 1/3] sequencer: add advice for revert
From: Junio C Hamano <hidden>
Date: 2019-06-11 21:25:46
Rohit Ashiwal [off-list ref] writes:
-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)\""));
- return -1;
- } else if (mkdir(git_path_seq_dir(), 0777) < 0)
+ enum replay_action action;
+
+ if (!sequencer_get_last_command(r, &action)) {
+ switch (action) {
+ case REPLAY_REVERT:
+ case REPLAY_PICK:
+ error(_("a %s is already in progress"),
+ action == REPLAY_REVERT ?
+ "revert" : "cherry-pick");I wonder if this poses a challenge to translators (imagine an alternate world, in which the name of one of these subcommands began with a vowel---your "a %s is already ..." would not be correct even without localization). The same comment applies to the other one, too.
+ advise(_("try \"git %s (--continue | "
+ "--quit | --abort)\""),
+ action == REPLAY_REVERT ?
+ "revert" : "cherry-pick");
This is horrible but it is not a fault of yours---you merely
inherited it. A call to advise() that is not behind any "advise.*"
configuration variable like this one should be cleaned up before
we do anything else.
The obvious and straight-forward way to deal with it is to do
const char *in_progress_error;
const char *in_progress_advice;
if (action == REPLAY_REVERT) {
in_progress_error = _("a revert is already in progress");
in_progress_advise = _("try ...");
} else if (action == REPLAY_PICK) {
... likewise ...
} else {
BUG("should not come here");
}
and then do
error(in_progress_error);
if (advise_verbosely_how_to_continue_sequencing)
advise(in_progress_advise);
quoted hunk
+ return -1; + 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 +4252,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"));