Ghanshyam Thakkar <shyamthakkar001@xxxxxxxxx> 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.
