On Fri Feb 2, 2024 at 10:38 PM IST, 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. 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 I meant revs which are spelled out in the form of <a>...<b> and not the <a> and <b> themselves. But I can see how it can be confusing. I will change it. > ... 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. Yeah, that is much better and clearer also. > 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. Will update. > > - 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? is_rev_head() is in add-patch.c and the above is in builtin/checkout.c and is_rev_head() is not exported. I can also define it in builtin/checkout.c, but this would be the only instance in that file which will use it. So, I think it is better to just add strcmp(rev, "@") to the if condition.
