Rohit Ashiwal <rohit.ashiwal...@gmail.com> 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);



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

Reply via email to