Re: [PATCH 4/7] rebase --apply: respect GIT_REFLOG_ACTION

From: Christian Couder <christian.couder@gmail.com>
Date: Apr 7, 2022, 13:59 UTC
Message-ID: <CAP8UFD3dx_Mv=X1y11LDy6NOcV0eDq5s92PqdC3LnbBFCi1kxg@mail.gmail.com>
In-Reply-To: <58c560d0e19a3fa6dd5de547821c9642089de3fb.1645441854.git.gitgitgadget@gmail.com>

On Tue, Feb 22, 2022 at 6:41 AM Phillip Wood via GitGitGadget <gitgitgadget@gmail.com> wrote:

Show 5 quoted lines

>
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> The reflog messages when finishing a rebase hard code "rebase" rather
> than using GIT_REFLOG_ACTION.

Yeah, but GIT_REFLOG_ACTION is something like "pick" or "continue" ...

Show 18 quoted lines

> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  builtin/rebase.c          | 7 ++++---
>  t/t3406-rebase-message.sh | 2 +-
>  2 files changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/rebase.c b/builtin/rebase.c
> index 36863117fba..e50361fc2a9 100644
> --- a/builtin/rebase.c
> +++ b/builtin/rebase.c
> @@ -580,10 +580,11 @@ static int move_to_original_branch(struct rebase_options *opts)
>         if (!opts->onto)
>                 BUG("move_to_original_branch without onto");
>
> -       strbuf_addf(&branch_reflog, "rebase finished: %s onto %s",
> +       strbuf_addf(&branch_reflog, "%s finished: %s onto %s",
> +                   getenv(GIT_REFLOG_ACTION_ENVIRONMENT),
>                     opts->head_name, oid_to_hex(&opts->onto->object.oid));

... so this will say "pick finished..." instead of "rebase finished..." while it may be better to have something like "rebase (pick) finished...", or am I missing something?

> -       strbuf_addf(&head_reflog, "rebase finished: returning to %s",
> -                   opts->head_name);
> +       strbuf_addf(&head_reflog, "%s finished: returning to %s",
> +                   getenv(GIT_REFLOG_ACTION_ENVIRONMENT), opts->head_name);

In patch 2/7 GIT_REFLOG_ACTION, instead of GIT_REFLOG_ACTION_ENVIRONMENT, is used.

It looks like GIT_REFLOG_ACTION_ENVIRONMENT is defined in reset.h while GIT_REFLOG_ACTION is defined in sequencer.c. Maybe we could get rid of one of these two, and use the same everywhere?

>         ropts.branch = opts->head_name;
>         ropts.flags = RESET_HEAD_REFS_ONLY;
>         ropts.branch_msg = branch_reflog.buf;