From: Junio C Hamano Date: Wed, 07 Feb 2024 01:05:57 GMT Subject: Re: [PATCH v4 2/3] add-patch: classify '@' as a synonym for 'HEAD' Message-ID: In-Reply-To: <20240206225122.1095766-6-shyamthakkar001@gmail.com> Ghanshyam Thakkar 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. > 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 ... > 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 ...)"