[PATCH v2 0/6] New @ shortcut for HEAD

[PATCH v2 0/6] New @ shortcut for HEAD

[Felipe Contreras]

Hi,

Same as before, except that now only '@' changes, 'master@' remains the same as
before.

Also, cleanups for sha1_name, and an update so '@@{u}' works.

Felipe Contreras (6):
  sha1_name: remove no-op
  sha1_name: remove unnecessary braces
  sha1_name: avoid Yoda conditions
  sha1_name: refactor reinterpret()
  Add new @ shortcut for HEAD
  sha1_name: allow @@{u} to work

 Documentation/git-check-ref-format.txt |  2 +
 Documentation/revisions.txt            |  3 ++
 refs.c                                 |  4 ++
 sha1_name.c                            | 78 +++++++++++++++++++++++-----------
 t/t1508-at-combinations.sh             |  3 ++
 5 files changed, 65 insertions(+), 25 deletions(-)

-- 
1.8.3.rc0.395.gfe9a10d

[PATCH v2 1/6] sha1_name: remove no-op

[Felipe Contreras]

'at' is always 0, since we can reach this point only if
!len && reflog_len, and len=at when reflog is assigned.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sha1_name.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sha1_name.c b/sha1_name.c
index 3820f28..01e49a9 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -464,7 +464,7 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		struct strbuf buf = STRBUF_INIT;
 		int ret;
 		/* try the @{-N} syntax for n-th checkout */
-		ret = interpret_branch_name(str+at, &buf);
+		ret = interpret_branch_name(str, &buf);
 		if (ret > 0) {
 			/* substitute this branch name and restart */
 			return get_sha1_1(buf.buf, buf.len, sha1, 0);
-- 
1.8.3.rc0.395.gfe9a10d

[PATCH v2 2/6] sha1_name: remove unnecessary braces

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sha1_name.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 01e49a9..6530ddd 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -465,12 +465,11 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		int ret;
 		/* try the @{-N} syntax for n-th checkout */
 		ret = interpret_branch_name(str, &buf);
-		if (ret > 0) {
+		if (ret > 0)
 			/* substitute this branch name and restart */
 			return get_sha1_1(buf.buf, buf.len, sha1, 0);
-		} else if (ret == 0) {
+		else if (ret == 0)
 			return -1;
-		}
 		/* allow "@{...}" to mean the current branch reflog */
 		refs_found = dwim_ref("HEAD", 4, sha1, &real_ref);
 	} else if (reflog_len)
-- 
1.8.3.rc0.395.gfe9a10d

[PATCH v2 3/6] sha1_name: avoid Yoda conditions

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sha1_name.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 6530ddd..93c4e8c 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -996,9 +996,9 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
 
 	if (!len)
 		return len; /* syntax Ok, not enough switches */
-	if (0 < len && len == namelen)
+	if (len > 0 && len == namelen)
 		return len; /* consumed all */
-	else if (0 < len) {
+	else if (len > 0) {
 		/* we have extra data, which might need further processing */
 		struct strbuf tmp = STRBUF_INIT;
 		int used = buf->len;
-- 
1.8.3.rc0.395.gfe9a10d

[PATCH v2 4/6] sha1_name: refactor reinterpret()

[Felipe Contreras]

This code essentially replaces part of ref with another ref, for example
'@{-1}@{u}' is replaced with 'master@{u}', but this can be reused for
other purposes other than nth prior checkouts.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sha1_name.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/sha1_name.c b/sha1_name.c
index 93c4e8c..76e3219 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -965,6 +965,27 @@ int get_sha1_mb(const char *name, unsigned char *sha1)
 	return st;
 }
 
+static int reinterpret(const char *name, int namelen, int len, struct strbuf *buf)
+{
+	/* we have extra data, which might need further processing */
+	struct strbuf tmp = STRBUF_INIT;
+	int used = buf->len;
+	int ret;
+
+	strbuf_add(buf, name + len, namelen - len);
+	ret = interpret_branch_name(buf->buf, &tmp);
+	/* that data was not interpreted, remove our cruft */
+	if (ret < 0) {
+		strbuf_setlen(buf, used);
+		return len;
+	}
+	strbuf_reset(buf);
+	strbuf_addbuf(buf, &tmp);
+	strbuf_release(&tmp);
+	/* tweak for size of {-N} versus expanded ref name */
+	return ret - used + len;
+}
+
 /*
  * This reads short-hand syntax that not only evaluates to a commit
  * object name, but also can act as if the end user spelled the name
@@ -998,25 +1019,8 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
 		return len; /* syntax Ok, not enough switches */
 	if (len > 0 && len == namelen)
 		return len; /* consumed all */
-	else if (len > 0) {
-		/* we have extra data, which might need further processing */
-		struct strbuf tmp = STRBUF_INIT;
-		int used = buf->len;
-		int ret;
-
-		strbuf_add(buf, name + len, namelen - len);
-		ret = interpret_branch_name(buf->buf, &tmp);
-		/* that data was not interpreted, remove our cruft */
-		if (ret < 0) {
-			strbuf_setlen(buf, used);
-			return len;
-		}
-		strbuf_reset(buf);
-		strbuf_addbuf(buf, &tmp);
-		strbuf_release(&tmp);
-		/* tweak for size of {-N} versus expanded ref name */
-		return ret - used + len;
-	}
+	else if (len > 0)
+		return reinterpret(name, namelen, len, buf);
 
 	cp = strchr(name, '@');
 	if (!cp)
-- 
1.8.3.rc0.395.gfe9a10d

[PATCH v2 5/6] Add new @ shortcut for HEAD

[Felipe Contreras]

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'.

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. Very few people if any probably used this name, if they
did, they can rename it by using the full-path (e.g. refs/heads/@).

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-check-ref-format.txt | 2 ++
 Documentation/revisions.txt            | 3 +++
 refs.c                                 | 4 ++++
 sha1_name.c                            | 6 +++++-
 t/t1508-at-combinations.sh             | 3 +++
 5 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index ec1739a..e8035ec 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -54,6 +54,8 @@ Git imposes the following rules on how references are named:
 
 . They cannot contain a sequence `@{`.
 
+. They cannot be the single character `@`.
+
 . They cannot contain a `\`.
 
 These rules make it easy for shell script based tools to parse
diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index d477b3f..09896a3 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -58,6 +58,9 @@ the '$GIT_DIR/refs' directory or from the '$GIT_DIR/packed-refs' file.
 While the ref name encoding is unspecified, UTF-8 is preferred as
 some output processing may assume ref names in UTF-8.
 
+'@'::
+  '@' alone is a shortcut for 'HEAD'.
+
 '<refname>@\{<date>\}', e.g. 'master@\{yesterday\}', 'HEAD@\{5 minutes ago\}'::
   A ref followed by the suffix '@' with a date specification
   enclosed in a brace
diff --git a/refs.c b/refs.c
index de2d8eb..4e70b3e 100644
--- a/refs.c
+++ b/refs.c
@@ -72,6 +72,10 @@ int check_refname_format(const char *refname, int flags)
 {
 	int component_len, component_count = 0;
 
+	if (!strcmp(refname, "@"))
+		/* Refname is a single character '@'. */
+		return -1;
+
 	while (1) {
 		/* We are at the start of a path component. */
 		component_len = check_refname_component(refname, flags);
diff --git a/sha1_name.c b/sha1_name.c
index 76e3219..887de6c 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -437,11 +437,13 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 	static const char *warn_msg = "refname '%.*s' is ambiguous.";
 	char *real_ref = NULL;
 	int refs_found = 0;
-	int at, reflog_len;
+	int at, reflog_len, only_at;
 
 	if (len == 40 && !get_sha1_hex(str, sha1))
 		return 0;
 
+	only_at = len == 1 && str[0] == '@';
+
 	/* basic@{time or number or -number} format to query ref-log */
 	reflog_len = at = 0;
 	if (len && str[len-1] == '}') {
@@ -474,6 +476,8 @@ static int get_sha1_basic(const char *str, int len, unsigned char *sha1)
 		refs_found = dwim_ref("HEAD", 4, sha1, &real_ref);
 	} else if (reflog_len)
 		refs_found = dwim_log(str, len, sha1, &real_ref);
+	else if (only_at)
+		refs_found = dwim_ref("HEAD", 4, sha1, &real_ref);
 	else
 		refs_found = dwim_ref(str, len, sha1, &real_ref);
 
diff --git a/t/t1508-at-combinations.sh b/t/t1508-at-combinations.sh
index d5d6244..50035cd 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 failure
 nonsense "@{u}@{-1}"
 nonsense "@{1}@{u}"
 
-- 
1.8.3.rc0.395.gfe9a10d

[PATCH v2 6/6] sha1_name: allow @@{u} to work

[Felipe Contreras]

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 sha1_name.c                | 21 +++++++++++++++++++++
 t/t1508-at-combinations.sh |  2 +-
 2 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/sha1_name.c b/sha1_name.c
index 887de6c..8f65bad 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -969,6 +969,21 @@ int get_sha1_mb(const char *name, unsigned char *sha1)
 	return st;
 }
 
+static int interpret_empty_at(const char *name, int namelen, int len, struct strbuf *buf)
+{
+	if (namelen - len <= 1 || name[len + 1] == '{')
+		return -1;
+
+	strbuf_reset(buf);
+	if (len == 0) {
+		strbuf_add(buf, "HEAD", 4);
+		return 1;
+	} else {
+		strbuf_add(buf, name, len);
+		return len + 1;
+	}
+}
+
 static int reinterpret(const char *name, int namelen, int len, struct strbuf *buf)
 {
 	/* we have extra data, which might need further processing */
@@ -1029,9 +1044,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);
+	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 50035cd..65584c0 100755
--- a/t/t1508-at-combinations.sh
+++ b/t/t1508-at-combinations.sh
@@ -47,7 +47,7 @@ check "@{-1}@{u}" master-two
 check "@{-1}@{u}@{1}" master-one
 check "@" new-two
 check "HEAD@{u}" upstream-two
-check "@@{u}" upstream-two failure
+check "@@{u}" upstream-two
 nonsense "@{u}@{-1}"
 nonsense "@{1}@{u}"
 
-- 
1.8.3.rc0.395.gfe9a10d

Re: [PATCH v2 3/6] sha1_name: avoid Yoda conditions

[Junio C Hamano]

What is a Yoda condition?

Re: [PATCH v2 4/6] sha1_name: refactor reinterpret()

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> This code essentially replaces part of ref with another ref, for example
> '@{-1}@{u}' is replaced with 'master@{u}', but this can be reused for
> other purposes other than nth prior checkouts.
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---

Makes sense.


>  sha1_name.c | 42 +++++++++++++++++++++++-------------------
>  1 file changed, 23 insertions(+), 19 deletions(-)
>
> diff --git a/sha1_name.c b/sha1_name.c
> index 93c4e8c..76e3219 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -965,6 +965,27 @@ int get_sha1_mb(const char *name, unsigned char *sha1)
>  	return st;
>  }
>  
> +static int reinterpret(const char *name, int namelen, int len, struct strbuf *buf)
> +{
> +	/* we have extra data, which might need further processing */
> +	struct strbuf tmp = STRBUF_INIT;
> +	int used = buf->len;
> +	int ret;
> +
> +	strbuf_add(buf, name + len, namelen - len);
> +	ret = interpret_branch_name(buf->buf, &tmp);
> +	/* that data was not interpreted, remove our cruft */
> +	if (ret < 0) {
> +		strbuf_setlen(buf, used);
> +		return len;
> +	}
> +	strbuf_reset(buf);
> +	strbuf_addbuf(buf, &tmp);
> +	strbuf_release(&tmp);
> +	/* tweak for size of {-N} versus expanded ref name */
> +	return ret - used + len;
> +}
> +
>  /*
>   * This reads short-hand syntax that not only evaluates to a commit
>   * object name, but also can act as if the end user spelled the name
> @@ -998,25 +1019,8 @@ int interpret_branch_name(const char *name, struct strbuf *buf)
>  		return len; /* syntax Ok, not enough switches */
>  	if (len > 0 && len == namelen)
>  		return len; /* consumed all */
> -	else if (len > 0) {
> -		/* we have extra data, which might need further processing */
> -		struct strbuf tmp = STRBUF_INIT;
> -		int used = buf->len;
> -		int ret;
> -
> -		strbuf_add(buf, name + len, namelen - len);
> -		ret = interpret_branch_name(buf->buf, &tmp);
> -		/* that data was not interpreted, remove our cruft */
> -		if (ret < 0) {
> -			strbuf_setlen(buf, used);
> -			return len;
> -		}
> -		strbuf_reset(buf);
> -		strbuf_addbuf(buf, &tmp);
> -		strbuf_release(&tmp);
> -		/* tweak for size of {-N} versus expanded ref name */
> -		return ret - used + len;
> -	}
> +	else if (len > 0)
> +		return reinterpret(name, namelen, len, buf);
>  
>  	cp = strchr(name, '@');
>  	if (!cp)

Re: [PATCH v2 3/6] sha1_name: avoid Yoda conditions

[Felipe Contreras]

On Tue, Apr 30, 2013 at 5:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
> What is a Yoda condition?

---
Using if (constant == variable) instead of if (variable == constant),
like if (4 == foo).

Because it's like saying "if blue is the sky" or "if tall is the man".
---

-- 
Felipe Contreras

Re: [PATCH v2 5/6] Add new @ shortcut for HEAD

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> 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'.
>
> 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. Very few people if any probably used this name, if they
> did, they can rename it by using the full-path (e.g. refs/heads/@).
>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  Documentation/git-check-ref-format.txt | 2 ++
>  Documentation/revisions.txt            | 3 +++
>  refs.c                                 | 4 ++++
>  sha1_name.c                            | 6 +++++-
>  t/t1508-at-combinations.sh             | 3 +++
>  5 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
> index ec1739a..e8035ec 100644
> --- a/Documentation/git-check-ref-format.txt
> +++ b/Documentation/git-check-ref-format.txt
> @@ -54,6 +54,8 @@ Git imposes the following rules on how references are named:
>  
>  . They cannot contain a sequence `@{`.
>  
> +. They cannot be the single character `@`.
> +
>  . They cannot contain a `\`.
>  
>  These rules make it easy for shell script based tools to parse
> diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
> index d477b3f..09896a3 100644
> --- a/Documentation/revisions.txt
> +++ b/Documentation/revisions.txt
> @@ -58,6 +58,9 @@ the '$GIT_DIR/refs' directory or from the '$GIT_DIR/packed-refs' file.
>  While the ref name encoding is unspecified, UTF-8 is preferred as
>  some output processing may assume ref names in UTF-8.
>  
> +'@'::
> +  '@' alone is a shortcut for 'HEAD'.
> +
>  '<refname>@\{<date>\}', e.g. 'master@\{yesterday\}', 'HEAD@\{5 minutes ago\}'::
>    A ref followed by the suffix '@' with a date specification
>    enclosed in a brace
> diff --git a/refs.c b/refs.c
> index de2d8eb..4e70b3e 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -72,6 +72,10 @@ int check_refname_format(const char *refname, int flags)
>  {
>  	int component_len, component_count = 0;
>  
> +	if (!strcmp(refname, "@"))
> +		/* Refname is a single character '@'. */
> +		return -1;

This checks the single character "@" which is consistent with the
documentation update, but does not seem to mesh well with "they can
rename it by using the full-path refs/heads/@".

I do not personally think it is wrong to end the log message with
"if they did, too bad.", though ;-).

Re: [PATCH v2 3/6] sha1_name: avoid Yoda conditions

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Tue, Apr 30, 2013 at 5:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> What is a Yoda condition?
>
> ---
> Using if (constant == variable) instead of if (variable == constant),
> like if (4 == foo).
>
> Because it's like saying "if blue is the sky" or "if tall is the man".

That is an invalid analogy, as the sentences do not make sense.

A much better explanation I heard on this list is that people do not
say "If 1 is smaller than the number of your wives, you have a big
problem".

I actually was not asking why people find the convention to visually
align comparison with number lines unusual. We discussed this style
long time ago on this list.  I haven't heard the "Yoda condtion"
expression and was asking about the "Yoda" part.

Re: [PATCH v2 3/6] sha1_name: avoid Yoda conditions

[Felipe Contreras]

On Tue, Apr 30, 2013 at 5:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Tue, Apr 30, 2013 at 5:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> What is a Yoda condition?
>>
>> ---
>> Using if (constant == variable) instead of if (variable == constant),
>> like if (4 == foo).
>>
>> Because it's like saying "if blue is the sky" or "if tall is the man".
>
> That is an invalid analogy, as the sentences do not make sense.
>
> A much better explanation I heard on this list is that people do not
> say "If 1 is smaller than the number of your wives, you have a big
> problem".
>
> I actually was not asking why people find the convention to visually
> align comparison with number lines unusual. We discussed this style
> long time ago on this list.  I haven't heard the "Yoda condtion"
> expression and was asking about the "Yoda" part.

It's popular culture.

http://en.wikipedia.org/wiki/Yoda

-- 
Felipe Contreras

Re: [PATCH v2 6/6] sha1_name: allow @@{u} to work

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> ---
>  sha1_name.c                | 21 +++++++++++++++++++++
>  t/t1508-at-combinations.sh |  2 +-
>  2 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/sha1_name.c b/sha1_name.c
> index 887de6c..8f65bad 100644
> --- a/sha1_name.c
> +++ b/sha1_name.c
> @@ -969,6 +969,21 @@ int get_sha1_mb(const char *name, unsigned char *sha1)
>  	return st;
>  }
>  
> +static int interpret_empty_at(const char *name, int namelen, int len, struct strbuf *buf)

Can we have a comment to clarify what "empty"-ness this name refers to?
Is it <empty>@<something>?  <something>@<empty>?  Something else?

> +{
> +	if (namelen - len <= 1 || name[len + 1] == '{')
> +		return -1;
> +
> +	strbuf_reset(buf);
> +	if (len == 0) {
> +		strbuf_add(buf, "HEAD", 4);
> +		return 1;
> +	} else {
> +		strbuf_add(buf, name, len);
> +		return len + 1;
> +	}
> +}
> +
>  static int reinterpret(const char *name, int namelen, int len, struct strbuf *buf)
>  {
>  	/* we have extra data, which might need further processing */
> @@ -1029,9 +1044,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);
> +	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 50035cd..65584c0 100755
> --- a/t/t1508-at-combinations.sh
> +++ b/t/t1508-at-combinations.sh
> @@ -47,7 +47,7 @@ check "@{-1}@{u}" master-two
>  check "@{-1}@{u}@{1}" master-one
>  check "@" new-two
>  check "HEAD@{u}" upstream-two
> -check "@@{u}" upstream-two failure
> +check "@@{u}" upstream-two
>  nonsense "@{u}@{-1}"
>  nonsense "@{1}@{u}"

Re: [PATCH v2 3/6] sha1_name: avoid Yoda conditions

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Tue, Apr 30, 2013 at 5:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>
>>> On Tue, Apr 30, 2013 at 5:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>> What is a Yoda condition?
>>>
>>> ---
>>> Using if (constant == variable) instead of if (variable == constant),
>>> like if (4 == foo).
>>>
>>> Because it's like saying "if blue is the sky" or "if tall is the man".
>>
>> That is an invalid analogy, as the sentences do not make sense.
>>
>> A much better explanation I heard on this list is that people do not
>> say "If 1 is smaller than the number of your wives, you have a big
>> problem".
>>
>> I actually was not asking why people find the convention to visually
>> align comparison with number lines unusual. We discussed this style
>> long time ago on this list.  I haven't heard the "Yoda condtion"
>> expression and was asking about the "Yoda" part.
>
> It's popular culture.
>
> http://en.wikipedia.org/wiki/Yoda

I know who Yoda is.  What I was puzzled with was what it has to do
with "if blue is the sky" (which is a bad analogy for "if (0 < len)"
anyway)?

Re: [PATCH v2 3/6] sha1_name: avoid Yoda conditions

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Tue, Apr 30, 2013 at 5:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> On Tue, Apr 30, 2013 at 5:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>>> What is a Yoda condition?
>>>>
>>>> ---
>>>> Using if (constant == variable) instead of if (variable == constant),
>>>> like if (4 == foo).
>>>>
>>>> Because it's like saying "if blue is the sky" or "if tall is the man".
>>>
>>> That is an invalid analogy, as the sentences do not make sense.
>>>
>>> A much better explanation I heard on this list is that people do not
>>> say "If 1 is smaller than the number of your wives, you have a big
>>> problem".
>>>
>>> I actually was not asking why people find the convention to visually
>>> align comparison with number lines unusual. We discussed this style
>>> long time ago on this list.  I haven't heard the "Yoda condtion"
>>> expression and was asking about the "Yoda" part.
>>
>> It's popular culture.
>>
>> http://en.wikipedia.org/wiki/Yoda
>
> I know who Yoda is.  What I was puzzled with was what it has to do
> with "if blue is the sky" (which is a bad analogy for "if (0 < len)"
> anyway)?

I found http://de.wikipedia.org/wiki/Yoda_Conditions which does not
seem to have many other languages.

No need to reply, thanks.

Re: [PATCH v2 3/6] sha1_name: avoid Yoda conditions

[Felipe Contreras]

On Tue, Apr 30, 2013 at 5:30 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Tue, Apr 30, 2013 at 5:13 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> Felipe Contreras <felipe.contreras@gmail.com> writes:
>>>
>>>> On Tue, Apr 30, 2013 at 5:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>>>> What is a Yoda condition?
>>>>
>>>> ---
>>>> Using if (constant == variable) instead of if (variable == constant),
>>>> like if (4 == foo).
>>>>
>>>> Because it's like saying "if blue is the sky" or "if tall is the man".
>>>
>>> That is an invalid analogy, as the sentences do not make sense.
>>>
>>> A much better explanation I heard on this list is that people do not
>>> say "If 1 is smaller than the number of your wives, you have a big
>>> problem".
>>>
>>> I actually was not asking why people find the convention to visually
>>> align comparison with number lines unusual. We discussed this style
>>> long time ago on this list.  I haven't heard the "Yoda condtion"
>>> expression and was asking about the "Yoda" part.
>>
>> It's popular culture.
>>
>> http://en.wikipedia.org/wiki/Yoda
>
> I know who Yoda is.  What I was puzzled with was what it has to do
> with "if blue is the sky" (which is a bad analogy for "if (0 < len)"
> anyway)?

Yoda speaks in reverse "Stopped they must be; on this all depends".
"if (0 < len)" says "if zero is less than len", which is in reverse,
as reverse as "if 1.50 is taller than you". It's all reversed: "if you
are taller than 1.50", "if len is greater than zero", "They must be
stopped; all depends on this".

I don't understand what is not clear.

-- 
Felipe Contreras

Re: [PATCH v2 3/6] sha1_name: avoid Yoda conditions

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> Yoda speaks in reverse "Stopped they must be; on this all depends".
> "if (0 < len)" says "if zero is less than len", which is in reverse,
> as reverse as "if 1.50 is taller than you". It's all reversed: "if you
> are taller than 1.50", "if len is greater than zero", "They must be
> stopped; all depends on this".
>
> I don't understand what is not clear.

I never said anything is "not clear", did I [*1*]?

I've retitled it to "compare variable with constant, not constant
with variable" to make it easier to understand to those who did not
know the jargon (like me ;-).

By the way, after web-searching "Yoda condition", all mentions of
"Yoda condition" I found were of the

	if (constant == variable)

form, and people's justification why this form is bad, which I
happen to agree with [*2*].


[Footnote]

*1* "zero is less than len" and "len is more than zero" are saying
exactly the same thing, even though people may find it harder to
read.  While "blue is the sky" and "the sky is blue" are not saying
the same thing in the first place, and that is why I said it is not
a good analogy.


*2* I do not write equality comparison in the "constant == variable"
order myself.  For people reading from sidelines, the previous
thread on this topic is this:

    http://thread.gmane.org/gmane.comp.version-control.git/3903/focus=3907

which explains why these were written as "if (smaller < larger)".

Re: [PATCH v2 3/6] sha1_name: avoid Yoda conditions

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> Yoda speaks in reverse "Stopped they must be; on this all depends".
>> "if (0 < len)" says "if zero is less than len", which is in reverse,
>> as reverse as "if 1.50 is taller than you". It's all reversed: "if you
>> are taller than 1.50", "if len is greater than zero", "They must be
>> stopped; all depends on this".
>>
>> I don't understand what is not clear.
>
> I never said anything is "not clear", did I [*1*]?

Ahh, OK.

Earlier I did not see the link between "Yoda" and "const op var"
that is "speak in reverse".  So I did say "not clear" or an
equivalent of it, and you clarified it with "reverse" here.

Thanks.

Re: [PATCH v2 5/6] Add new @ shortcut for HEAD

[Duy Nguyen]

On Wed, May 1, 2013 at 4:49 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> 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'.
>
> 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. Very few people if any probably used this name, if they
> did, they can rename it by using the full-path (e.g. refs/heads/@).

People can write master short for refs/heads/master, but can't with
refs/heads/@. Would it be better to detect if dwim_ref("@") exists,
then disable special '@' and warn user to rename '@' to something
else? After they have renamed it, make '@' special and forbid it in
any component in the ref (i.e. refs/heads/@ is forbidden too).
--
Duy

Re: [PATCH v2 5/6] Add new @ shortcut for HEAD

[Felipe Contreras]

On Tue, Apr 30, 2013 at 9:03 PM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Wed, May 1, 2013 at 4:49 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> 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'.
>>
>> 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. Very few people if any probably used this name, if they
>> did, they can rename it by using the full-path (e.g. refs/heads/@).
>
> People can write master short for refs/heads/master, but can't with
> refs/heads/@. Would it be better to detect if dwim_ref("@") exists,
> then disable special '@' and warn user to rename '@' to something
> else? After they have renamed it, make '@' special and forbid it in
> any component in the ref (i.e. refs/heads/@ is forbidden too).

I think that would be extremely tricky. If they have refs/heads/@, we
could try to warn them about the conflict, but I think it would be
pretty clear when 'git foo @' doesn't do what they want, and still
they, can use 'git foo refs/heads/@'.

I think it's overkill to worry prematurely about users that most
likely don't exist. If say, we introduce this for 1.8.4, and some
people suffer from the decision, then we can do something about it for
1.8.4.1, but it's more likely that they would just do 'git branch -m
refs/heads/@ whatever' (we should probably fix that because it doesn't
actually work with any refs/heads/X). Of course, the much much more
likely scenario is that nothing will happen.

If there was an easy fix, we should go for that, but I just don't see any.

-- 
Felipe Contreras

Re: [PATCH v2 5/6] Add new @ shortcut for HEAD

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> On Wed, May 1, 2013 at 4:49 AM, Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
>> 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'.
>>
>> 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. Very few people if any probably used this name, if they
>> did, they can rename it by using the full-path (e.g. refs/heads/@).
>
> People can write master short for refs/heads/master, but can't with
> refs/heads/@.

Is it a new problem?

You can not say HEAD to refer to refs/heads/HEAD, either.  To avoid
confusion, we teach "git branch HEAD $commit" to error out (but it
seems that "git checkout -b HEAD $commit" does not).

We probably would want to do the same for '@' when we refactor to
have a central place that knows what the "make sense as refnames at
the syntactic level, but are forbidden for operational purposes"
branch names are. And use it to update "git checkout -b".