Re: [PATCH v2 4/8] builtin/reflog: implement subcommand to write new entries

[Karthik Nayak <karthik.188@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1772 bytes --]

Junio C Hamano <gitster@pobox.com> writes:

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> Junio C Hamano <gitster@pobox.com> writes:
>>
>>> "Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
>>>
>>>> I tried `git reflog drop`[1] and it can deal with a branch like
>>>> `branch`.  It doesn’t need to be told `refs/heads/branch`.
>>>
>>> That sounds like a bug to me.
>>
>> So `git reflog drop` `git reflog delete` and `git reflog expire` use
>> `repo_dwim_log()` to resolve the provided reference.
>>
>> And `repo_dwim_log()` uses the following `ref_rev_parse_rules` to
>> resolve the reference.
>>
>>   static const char *ref_rev_parse_rules[] = {
>>   	"%.*s",
>>   	"refs/%.*s",
>>   	"refs/tags/%.*s",
>>   	"refs/heads/%.*s",
>>   	"refs/remotes/%.*s",
>>   	"refs/remotes/%.*s/HEAD",
>>   	NULL
>>   };
>>
>> Which means we do a best case resolution of a given reference, but the
>> function also checks for ambiguity and warns for it.
>
> True.  But as I considered "git reflog" to be a lot closer to the
> plumbing than to Porcelain, using the dwim thing smelled like a bug.
>

I agree that 'git reflog' is more of a plumbing command. I'm trying to
see what subcommands of reflog act this way, so we can take a decision
on how to move forward.

> It also is OK to update the commands that do not use dwim-log to
> also use it.  That way, the result would be consistent across
> subcommands of "git reflog".  As long as the users are aware of the
> fact that the command uses dwim-log, they can always spell their ref
> in full like "refs/heads/branch" to avoid ambiguity check getting in
> the way.
>
> Thanks.

Yeah consistency and documentation around it would be great here.

- Karthik

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 690 bytes --]