Re: [PATCH v2 2/2] add-patch: classify '@' as a synonym for 'HEAD'

[Junio C Hamano <gitster@pobox.com>]

Ghanshyam Thakkar <shyamthakkar001@gmail.com> writes:

> Currently, (checkout, reset, restore) commands correctly take '@' as a
> synonym for 'HEAD'. However, in patch mode (-p/--patch), for both '@' and
> 'HEAD', different prompts/messages are given by the commands mentioned
> above. This is due to the literal and only string comparison with the word
> 'HEAD' in run_add_p(). Synonymity between '@' and 'HEAD' is obviously
> desired, especially since '@' already resolves to 'HEAD'.
>
> Therefore, make a new function user_meant_head() which takes the
> revision string and compares it to 'HEAD' as well as '@'. However, in
> builtin/checkout.c, there is some logic to convert all revs besides
> 'HEAD' to hex revs due to 'diff-index', which is used in patch mode
> machinery, not being able to handle '<a>...<b>' revs. Therefore, in
> addition to 'HEAD', do not convert '@' as well, so it can be later
> used in assigning patch_mode_(...)_head.

In this context <a>...<b> names a single rev (not two revs) that is
the merge base of <a> and <b>.  Perhaps

    ... there is a logic to convert all command line input rev to
    the raw object name for underlying machinery (e.g., diff-index)
    that does not recognize the <a>...<b> notation, but we'd need to
    leave "HEAD" intact.  Now we need to teach that "@" is a synonym
    to "HEAD" to that code, too.

which may be a bit shorter.

You decided to use is_rev_head() instead of user_meant_head(), so
you'd need to update the above description to match, I think.

> -		if (rev && new_branch_info->commit && strcmp(rev, "HEAD"))
> +		if (rev && new_branch_info->commit && strcmp(rev, "HEAD") &&
> +		    strcmp(rev, "@"))

Shouldn't this be

		if (rev && new_branch_info->commit && !is_rev_head(rev))

instead of "HEAD" and "@" spelled out?

Other than the above, nicely done.