Re: [PATCH v5 1/2] revision: ensure MERGE_HEAD is a ref in prepare_show_merge
- From
Philippe Blain <levraiphilippeblain@gmail.com>
- Date
- Feb 26, 2024, 17:54 UTC
- Message-ID
- <13c882d4-8f61-1ea5-8949-fdde16bcfc5b@gmail.com>
- In-Reply-To
- <c46a0a4f-63dc-404a-8ee2-64a34f474c5f@gmail.com>
Hi Jean-Noël,
Le 2024-02-26 à 12:22, Jean-Noël Avila a écrit :
Show 36 quoted lines
> Hello,
>
> Le 25/02/2024 à 22:56, Philippe Blain a écrit :
>> From: Michael Lohmann <mi.al.lohmann@gmail.com>
>>
>> This is done to
>> (1) ensure MERGE_HEAD is a ref,
>> (2) obtain the oid without any prefixing by refs.c:repo_dwim_ref()
>> (3) error out when MERGE_HEAD is a symref.
>>
>> Helped-by: Junio C Hamano <gitster@pobox.com>
>> Signed-off-by: Michael Lohmann <mi.al.lohmann@gmail.com>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> Signed-off-by: Philippe Blain <levraiphilippeblain@gmail.com>
>> ---
>> revision.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/revision.c b/revision.c
>> index 2424c9bd67..ee26988cc6 100644
>> --- a/revision.c
>> +++ b/revision.c
>> @@ -1973,8 +1973,12 @@ static void prepare_show_merge(struct rev_info *revs)
>> if (repo_get_oid(the_repository, "HEAD", &oid))
>> die("--merge without HEAD?");
>> head = lookup_commit_or_die(&oid, "HEAD");
>> - if (repo_get_oid(the_repository, "MERGE_HEAD", &oid))
>> + if (read_ref_full("MERGE_HEAD",
>> + RESOLVE_REF_READING | RESOLVE_REF_NO_RECURSE,
>> + &oid, NULL))
>> die("--merge without MERGE_HEAD?");
>> + if (is_null_oid(&oid))
>> + die(_("MERGE_HEAD is a symbolic ref?"));
>
> Following the thread about being less passive-aggressive, maybe this
> could be rephrased in an assertive mood.Yes, Junio suggested the same in <xmqqa5nnj10v.fsf@gitster.g>.
> > By the way, this string is translatable, but not the one 2 lines above. > What is the policy around translation?
My understanding is that new error messages should be translated, but here the patch is not touching the message "--merge without HEAD?" so I would think it is OK to avoid changing these lines to mark it for translation. But, I could make that change in a preparatory patch (and rephrase it at the same time).
Thanks, Philippe.