Re: [PATCH v3] Add new @ shortcut for HEAD

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Felipe Contreras <felipe.contreras@xxxxxxxxx> writes:

> So HEAD@{0}~0^0 is too much to type, but we can remove '^0', and we can
> remove '~0', and we can remove 'HEAD', which leaves us with @{0}, but we
> can't remove '{0}'?
>
> This patch allows '@' to be the same as 'HEAD'.

While the above reasoning is cute, it is misleading.

If you start from HEAD@{1}~0^0, we can remove '^0', we can remove
'~0', but you cannot remove HEAD from the remaining "HEAD@{1}"
without changing what it means.  @{1} is where the current branch
was, while HEAD@{1} is where you were---they are different when you
have just did "git checkout anotherbranch".  HEAD@{1} is the tip of
your previous branch, @{1} is where anotherbranch was before its tip
became the commit you have checked out.

You have to be specially talking about "HEAD@{0}" as a whole for
that reasoning to hold; it does not work for HEAD@{$n} for an
arbitrary value of $n.

So I'd suggest toning it down, perhaps something like this:

	Even though we often can do without having to type "HEAD",
	e.g. "git log origin.." substitutes missing RHS with "HEAD",
	sometimes we still do need to type "HEAD" (thats six f*cking
	keystrokes "Caps Lock", "H", "E", "A", "D" and finally "Caps
	Lock").

        That is four keystrokes too many to name an often needed
	reference.  Make "@" usable as its synonym.

>
> So now we can use 'git show @~1', and all that goody goodness.
>
> Until now '@' was a valid name, but it conflicts with this idea, so lets
> make it invalid. Probably very few people, if any, used this name.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@xxxxxxxxx>
> ---
> diff --git a/sha1_name.c b/sha1_name.c
> index 76e3219..3b06e5e 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -965,6 +965,17 @@ int get_sha1_mb(const char *name, unsigned char *sha1)
>  	return st;
>  }
>  
> +/* parse @something syntax, when 'something' is not {.*} */
> +static int interpret_empty_at(const char *name, int namelen, int len, struct strbuf *buf)
> +{
> +	if (len || name[1] == '{')
> +		return -1;

This function is to handle a string that begins with '@', so by
definition len is zero when anything useful is done by it.  So...

> +	strbuf_reset(buf);
> +	strbuf_add(buf, "HEAD", 4);
> +	return 1;
> +}
> +
>  static int reinterpret(const char *name, int namelen, int len, struct strbuf *buf)
>  {
>  	/* we have extra data, which might need further processing */
> @@ -1025,9 +1036,15 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
>  	cp = strchr(name, '@');
>  	if (!cp)
>  		return -1;
> +
> +	len = interpret_empty_at(name, namelen, cp - name, buf);

... it is suboptimal (from readability point of view) to have the
caller unconditionally call interpret_empty_at() when the function
clearly is marked to handle something that _begins_ with '@'.

I would suggest something like

	if (cp == name)
        	len = interpret_empty_at(name, namelen, buf);

which people may find much easier to follow.

For that matter, it may make even more sense to just remove the
"empty-at" function and inline its body here:

	if (cp == name && name[1] != '{') {
		strbuf_reset(buf);
                strbuf_add(buf, "HEAD", 4);
                len = 1;
        } else {
        	len = -1;
	}

> +	if (len > 0)
> +		return reinterpret(name, namelen, len, buf);
> +
>  	tmp_len = upstream_mark(cp, namelen - (cp - name));
>  	if (!tmp_len)
>  		return -1;
> +
>  	len = cp + tmp_len - name;
>  	cp = xstrndup(name, cp - name);
>  	upstream = branch_get(*cp ? cp : NULL);
> diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh
> index d5d6244..65584c0 100755
> --- a/t/t1508-at-combinations.sh
> +++ b/t/t1508-at-combinations.sh
> @@ -45,6 +45,9 @@ check "@{u}" upstream-two
>  check "@{u}@{1}" upstream-one
>  check "@{-1}@{u}" master-two
>  check "@{-1}@{u}@{1}" master-one
> +check "@" new-two
> +check "HEAD@{u}" upstream-two
> +check "@@{u}" upstream-two
>  nonsense "@{u}@{-1}"
>  nonsense "@{1}@{u}"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]