On 07/02/2024 01:05, Junio C Hamano wrote:
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 (because of applying reverse mode(-R) in case of '@').
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, replace '@' to 'HEAD' at the beginning of
add-patch.c:run_add_p().
Of course there is only one possible downside for this approach, in
that if we are using "revision" in an error message, users who asked
for "@" may complain when an error message says "HEAD" in it. I think
the simplicity of the implementation far outweighs this downside.
I agree, if we were replacing the revision the user gave us with a hex
object id that would be confusing but as "@" is just a shortcut for
"HEAD" I think replacing it in the error message is fine. It was a good
idea just to replace "@" with "HEAD", this version is much simpler.
Best Wishes
Phillip
There is also logic in builtin/checkout.c 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 and leave '@' intact, too.
Makes me wonder why we cannot use the same "normalize @ to HEAD
upfront" approach here, though?
It would involve translating "@" given to new_branch_info->name to
"HEAD" early, possibly in setup_new_branch_info_and_source_tree(),
and that probably will fix the other strcmp() with "HEAD" that
appears in builtin/checkout.c:update_refs_for_switch() as well, no?
+ /* helpful in deciding the patch mode ahead */
+ if(revision && !strcmp(revision, "@"))
+ revision = "HEAD";
Style. "if (revision ...)"
