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

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

Phillip Wood <phillip.wood123@gmail.com> writes:

> As well as the places you have converted we also have an explicit test
> for "HEAD" in parse_diff() which looks like
>
> 	if (s->revision) {
> 		struct object_id oid;
> 		strvec_push(&args,
> 			    /* could be on an unborn branch */
> 			    !strcmp("HEAD", s->revision) &&
> 			    repo_get_oid(the_repository, "HEAD", &oid) ?
> 			    empty_tree_oid_hex() : s->revision);
> 	}
>
> I suspect we need to update that code as well to accept "@" as a
> synonym for "HEAD" on an unborn branch.

I actually think "@" in the input should be normalized to "HEAD" as
soon as possible.  After the if/elseif/ cascade in run_add_p() the
patch in this thread touches, there is an assignment

	s.revision = revision;

and even though we rewrite !strcmp(revision, "HEAD") to "user means
HEAD" to additionally accept "@" in that if/elseif/ cascade, here we
will stuff different values to s.revision here.  We could normalize
the end-user supplied "@" to "HEAD" before making this assignment,
then you do not have to worry about the code in parse_diff() above,
and more importantly, we do not have to rely on what the current set
of callers happen to do and do not happen to do (e.g., "git reset
-p" happens to use hardcoded "HEAD" for unborn case without using
anything obtained from the user, so the above code in parse_diff()
might be safe in that case, but we do not want to rely on such
subtlety to make sure our code is correct)

Come to think of it, we could even do the "normalizing" even before,
and that might greatly simplify things.  For example, if we did so
at the very beginning of run_add_p(), before we come to that if/else
if/ cascade, we may not even have to worry about the "user meant HEAD"
helper.  After the normalization, we can just keep using !strcmp()
with "HEAD" alone.  Simple and clean, no?

Thanks.