All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan Nieder <jrnieder@gmail.com>
To: git@vger.kernel.org
Cc: Ronnie Sahlberg <sahlberg@google.com>,
	Michael Haggerty <mhagger@alum.mit.edu>
Subject: check-ref-format: include refs/ in the argument or to strip it?
Date: Fri, 22 Aug 2014 08:41:51 -0700	[thread overview]
Message-ID: <20140822154151.GK20185@google.com> (raw)
In-Reply-To: <047d7b624d36142d46050131f336@google.com>

Hi,

Michael Haggerty wrote[1]:
> Jonathan Nieder wrote:

>> The check-ref-format documentation is pretty unclear, but the
>> intent is that it would be used like
>>
>>	git check-ref-format heads/master
>>
>> (see the surviving examples in contrib/examples/). That way, it can
>> enforce the rule (from git-check-ref-format(1))
>>
>>	2. They must contain at least one /. This enforces the presence
>>	of a category like heads/, tags/ etc. but the actual names
>>	are not restricted.
[...]
> Thanks for the explanation and the pointer.
>
> I suppose that was a usage that was more popular in the past than
> now. I can hardly remember anyone talking about references like
> "heads/master" or "tags/v1". It seems to me that when somebody wants
> to be unambiguous they usually use the whole reference name
> "refs/heads/master" or "refs/tags/v1". When they want to be succinct
> they usually use just "master" or "v1".
>
> To me it seems incongruous to have the "heads/master" syntax
> supported at this deep level of plumbing. In most cases, the caller
> could get the same effect by prepending "refs/" to the string and
> then calling check_refname_format with a new
> REFNAME_REQUIRE_CATEGORY option that requires both a "refs/" prefix
> and a total of at least *three* levels.

I agree that this piece of UI is pretty weird.  Worse, it's
underdocumented.

I wonder if it would make sense to have the check-ref-format commandline
utility require two slashes when its argument begins with "refs/" (still
requiring only one slash when it doesn't, for backward compatibility)
and to start encouraging people to pass refnames with refs/ to it.

The alternative would be something like the following, which doesn't
seem too appealing.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
[1] https://code-review.googlesource.com/1017

 Documentation/git-check-ref-format.txt | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/Documentation/git-check-ref-format.txt b/Documentation/git-check-ref-format.txt
index fc02959..447e9fb 100644
--- a/Documentation/git-check-ref-format.txt
+++ b/Documentation/git-check-ref-format.txt
@@ -15,8 +15,8 @@ SYNOPSIS
 
 DESCRIPTION
 -----------
-Checks if a given 'refname' is acceptable, and exits with a non-zero
-status if it is not.
+Checks if refs/<refname> is an acceptable ref name, and exits
+with a non-zero status if it is not.
 
 A reference is used in Git to specify branches and tags.  A
 branch head is stored in the `refs/heads` hierarchy, while
@@ -91,14 +91,14 @@ OPTIONS
 	components).  The default is `--no-allow-onelevel`.
 
 --refspec-pattern::
-	Interpret <refname> as a reference name pattern for a refspec
-	(as used with remote repositories).  If this option is
-	enabled, <refname> is allowed to contain a single `*`
-	in place of a one full pathname component (e.g.,
-	`foo/*/bar` but not `foo/bar*`).
+	Interpret refs/<refname> as a reference name pattern
+	for a refspec (as used with remote repositories).
+	If this option is enabled, <refname> is allowed
+	to contain a single `*` in place of a one full pathname
+	component (e.g., `foo/*/bar` but not `foo/bar*`).
 
 --normalize::
-	Normalize 'refname' by removing any leading slash (`/`)
+	Normalize <refname> by removing any leading slash (`/`)
 	characters and collapsing runs of adjacent slashes between
 	name components into a single slash.  Iff the normalized
 	refname is valid then print it to standard output and exit
@@ -118,7 +118,7 @@ $ git check-ref-format --branch @{-1}
 * Determine the reference name to use for a new branch:
 +
 ------------
-$ ref=$(git check-ref-format --normalize "refs/heads/$newbranch") ||
+$ ref=$(git check-ref-format --normalize "heads/$newbranch") ||
 die "we do not like '$newbranch' as a branch name."
 ------------
 
-- 

       reply	other threads:[~2014-08-22 15:42 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <gerrit.1408574889668.Iac983fc86f7edd2a0543779d85973c57bf068ca4@code-review.googlesource.com>
     [not found] ` <047d7b624d36142d46050131f336@google.com>
2014-08-22 15:41   ` Jonathan Nieder [this message]
2014-08-22 18:37     ` check-ref-format: include refs/ in the argument or to strip it? Junio C Hamano
2014-08-22 18:45       ` Jonathan Nieder
2014-08-23  5:46         ` Jeff King
2014-08-23  5:50           ` Junio C Hamano
2014-08-25 17:43           ` Ronnie Sahlberg
2014-08-25 18:26             ` Jonathan Nieder
2014-08-25 19:09               ` Jeff King
2014-08-27 20:53                 ` Michael Haggerty
2014-08-25 19:01             ` Junio C Hamano
2014-08-23  2:59       ` Jonathan Nieder

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20140822154151.GK20185@google.com \
    --to=jrnieder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    --cc=sahlberg@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.