From: Phillip Wood Date: Fri, 15 Apr 2022 14:03:11 GMT Subject: Re: [PATCH 4/7] rebase --apply: respect GIT_REFLOG_ACTION Message-ID: In-Reply-To: Hi Christian On 07/04/2022 14:59, Christian Couder wrote: > On Tue, Feb 22, 2022 at 6:41 AM Phillip Wood via GitGitGadget > wrote: >> >> From: Phillip Wood >> >> 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" ... GIT_REFLOG_ACTION can be set in the environment by the user (or more likely a script) to change the command name in reflog messages created by rebase. At the moment we respect that for all the messages apart from the ones being changed here. Best Wishes Phillip >> Signed-off-by: Phillip Wood >> --- >> 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;