patchbuiltin-branch - allow deleting a fully specified branch-name

This change allows, for instance
	git branch -d refs/heads/foo
to succeed. Without this patch, the code just assumes that the
given branch name should be appended to "refs/heads" or
"refs/remotes", thus attempting (and failing) in the above case
to delete "refs/heads/refs/heads/foo"

Signed-off-by: Mark Levedahl <mlevedahl@gmail.com>
---
 builtin-branch.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

Mark Levedahl <mlevedahl@gmail.com> writes:

Your logic is broken.

Why doesn't the user simply say "git branch -d foo"? The command takes "the branch name", not "arbitrary ref name".

On Thursday 09 April 2009 20:39:46 Junio C Hamano wrote:

1) git branch -d refs/<whatever> used to work,  I haven't bisected to find 
when this stopped working, but the change broke one of my scripts, so this is 
not new behavior, it is restoration of previous behavior.
2) If I create branch  refs/frotz/bar , how do I ever delete it?

Also, the following all work
3) git branch refs/heads/foo
4) git branch -m refs/heads/foo refs/heads/bar 
5) git  [checkout|pull|push|fetch|show] refs/heads/foo

So, why is "git branch -d" so special?

Mark

Mark Levedahl <mlevedahl@gmail.com> writes:

I need to look at the history, if that is the case then perhaps Ok.

By this you must mean .git/refs/heads/refs/frotz/bar, right? Then shouldn't "git branch -d refs/frotz/bar" just work as is? If you are talking about .git/refs/frotz/bar, "git branch" should not touch it, it is not even a branch.

If you mean it creates .git/refs/heads/refs/heads/foo, then sure it should. If it creates .git/refs/heads/foo, I think it is broken.

Among these, checkout is the only special case that can take "branch name" to switch branches. Everything else takes extended SHA-1 expression. Checkout can interpret the first argument as "branch to switch to", but it does not necessarily so---think "detached HEAD", and also think "checking out paths out of tree-ish".

"git branch -d", "git branch -m" and friends all take branch name, and as such it can use "@{-1}" to _name_ 'the previous branch". In that context, you are _not_ naming the commit at the tip of the branch. You are naming the branch itself.

All other commands happen to take a branch name because that is just one case of extended SHA-1 expression to name an object. In that context, a refname (which a branch name is a special case of) refers to the commit pointed by it. E.g.

        "git checkout HEAD~20 -- Makefile"
        "git show refs/heads/foo"
        "git show heads/foo"
        "git show foo"

On Thursday 09 April 2009 23:18:01 you wrote:

I think my underlying problem here is the porcelain's ability to use either a 
branch name or a ref name, in different contexts, leading to a sometimes 
inconsistent interface. Consider the following
   $ git checkout master
             checks out branch master
   $ git checkout refs/heads/master
             checks out commit pointed to by refs/heads/master on a detached 
HEAD

   $ git checkout -b refs/heads/master refs/heads/master
             creates a new branch, refname = refs/heads/refs/heads/master

The last command is the one that I find most curious. The exact same string has two entirely different meanings to the same command. I can explain why this happens, but I cannot explain why this is a good thing.

A model I could explain without mental gymnastics would be "branch names are simply refnames without the leading refs/heads or refs/remotes, and a refname may be used wherever a branch name is requested. While branch names are potentially ambiguous, refnames never are." Of course, this would mean that the refs/heads/refs/... namespace is illegal. I don't know of any other downside (except of course to someone using that namespace), and frankly I don't think the existence of the refs/heads/refs namespace is a good thing, given the potential for confusion.

Just a thought.

Mark

On Sat, Apr 11, 2009 at 01:01:30PM -0400, Mark Levedahl wrote:

A command like "grep foo foo" has the same property (one string with different meanings based on argument position). The problem is that you are thinking of it as:

  git checkout -b <branch> <branch>

And I can see why you might think of it that way, because that is what the synopsis in git-checkout(1) says. :) But it is really:

  git checkout -b <branch> <commit>

I'm not sure if changing that synopsis would really help, or if it is a bit too subtle.

So the current model is: "branch names are simply refnames without the leading refs/heads or refs/remotes. A <commit> can be referenced by the usual names (see git rev-parse, "specifying revisions" for details)".

The thing that I think is more confusing about that is not the final example you pointed out, but the difference between

  git checkout master

and

  git checkout refs/heads/master

Which is explained by the fact that the usage for checkout is not

  git checkout <branch>

but actually

  git checkout <branch|commit>

If a branch, then we checkout the branch. If a commit, then we detach on that commit.

I'm not sure if that explanation helps you at all, but that is how I think of it (and it makes sense to me).

-Peff

Jeff King <peff@peff.net> writes:

I think in many places we used to be stricter in terminology (e.g. when we only need tree-ish we used to write tree-ish) but during the "usability and approachiablility drive" people updated doc with "most of the time the command takes commit, so we say commit".

I think "the apporachable part" aka "synopsis" should be kept the way it is, but we should clarify in the description when the most general form is different from the white lie we feed to newbies.

On Sun, Apr 12, 2009 at 01:22:23AM -0700, Junio C Hamano wrote:

Yeah, I think this is one of the fundamental usability debates of git. On one hand, if you assume that the user learns about the object types, the DAG, and refs, then all of this is very straightforward to explain. The price you pay is a steep learning curve. On the other, you can get 95% of what you need done without ever being exposed to the technical details, so I can see why people push for learning materials that gloss over it.

Hmm. I tried something in this direction, but I actually think it ended up more confusing.

I think it would be better in the synopsis to split this into two use cases:

  git checkout [<branch>]
  git checkout -b <new_branch> [<start_point>]

And then explain them as separate definitions.

-Peff

Jeff King <peff@peff.net> writes:

I'd agree it would make it much nicer.

Thanks for a bit of sanity.  I sometimes misplace it when I got grumpy.

On Mon, Apr 13, 2009 at 02:54:27AM -0700, Junio C Hamano wrote:

A 5-patch series follows which does this and a few other fixups. I think the result is better.

  1/5 doc: clarify --no-track option
  2/5 doc: refer to tracking configuration as "upstream"
  3/5 doc/checkout: refer to git-branch(1) as appropriate
  4/5 doc/checkout: split checkout and branch creation in synopsis
  5/5 docs/checkout: clarify what "non-branch" means

-Peff

It is not really about ignoring the config option; it is about turning off tracking, _even if_ the config option is set.

Signed-off-by: Jeff King <peff@peff.net>
---
This I just noticed while working in the area, and it bothered me.

 Documentation/git-branch.txt   |    3 ++-
 Documentation/git-checkout.txt |    3 ++-
 2 files changed, 4 insertions(+), 2 deletions(-)

The term "tracking" often creates confusion between remote tracking branches and local branches which track a remote branch. The term "upstream" captures more clearly the idea of "branch A is based on branch B in some way", so it makes sense to mention it.

At the same time, upstream branches are used for more than just git-pull these days; let's mention that here.

Signed-off-by: Jeff King <peff@peff.net>
---
I think this matches nicely with Santi's recent addition of upstream to
the glossary.

 Documentation/git-branch.txt   |   24 +++++++++++++-----------
 Documentation/git-checkout.txt |    2 +-
 2 files changed, 14 insertions(+), 12 deletions(-)

Most of description for the branch creation options is simply cut and paste from git-branch. There are two reasons to fix this:

  1. It can grow stale with respect to what's in "git
     branch" (which it is now is).

  2. It is not just an implementation detail, but rather the
     desired mental model for the command that we are using
     "git branch" here. Being explicit about that can help
     the user understand what is going on.

It also makes sense to strip the branch creation options from the synopsis, as they are making it a long, hard-to-read line. They are still easily discovered by reading the options list, and --track is explicitly referenced when branch creation is described.

Signed-off-by: Jeff King <peff@peff.net>
---
There are two things here that people might disagree with:

 1. Referring the user instead of using asciidoc magic to just include
    the repeated text. While it is nice to save the user the effort
    of referencing the other documentation, I think sometimes we go too
    far with this in git. Sometimes it is beneficial for the user not
    just to get the information, but to have it pointed out that the
    link exists. Then they can better understand exactly what it is git
    is doing (it is not "oh, this is a totally unrelated way to create
    branches" but "this is a convenience wrapper that calls git branch"
    -- I think the latter is the model we want to use).

 2. Shortening the synopsis field. The long lines look awful in the
    manpage on an 80-column terminal. I don't read git documentation
    very often, so I have not really noticed, but some of our synopses
    are quite complex and IMHO unreadable. We are much better off just
    saying "there are some options, then these 1 or 2 arguments" which
    give an overview of how you would invoke the command. You can look
    at the option list if you want to actually see the options.

    In this patch I just shortened the ones related to branch creation.
    But I was tempted to take this a step further and get rid of the
    "-f", "-q", and "-m" options in the synopsis, as well, which I think
    would look much better.

 Documentation/git-checkout.txt |   28 +++++++++-------------------
 1 files changed, 9 insertions(+), 19 deletions(-)

These can really be thought of as two different modes, since the "<branch>" parameter is treated differently in the two (in one it is the branch to be checked out, but in the other it is really a start-point for branch creation).

Signed-off-by: Jeff King <peff@peff.net>
---
This was the actual goal of the series. :)

 Documentation/git-checkout.txt |   40 ++++++++++++++++++++++------------------
 1 files changed, 22 insertions(+), 18 deletions(-)

In the code we literally stick "refs/heads/" on the front and see if it resolves, so that is probably the best explanation.

Signed-off-by: Jeff King <peff@peff.net>
---
I hope this helps a little bit with Mark's confusion. But while writing
it, I really think it would be a simpler rule to say "if it's in
refs/heads/, then it's a branch" (which is similar to what Mark
suggested earlier).

So "git checkout refs/heads/master" would be identical to "git checkout master". That would require a code change, though.

 Documentation/git-checkout.txt |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

Jeff King <peff@peff.net> writes:

Sorry, but I do not get the logic behind such a change.

Because you won't be breaking "git checkout frotz" that checks out the branch whose name is frotz (i.e. refs/heads/frotz) even when a tag frotz exists (i.e. refs/tags/frotz), the updated code cannot be "try to resolve the token given from the command line as-is, and if it is in refs/heads/ it is a branch switch, otherwise it is a detach at the commit". The updated code has to be "try to resolve the token appended to refs/heads and if it resolves, that is a branch switch. Otherwise if the token already begins with refs/heads/ then it also is a switch to the branch whose name is the token minus the leading refs/heads/, otherwise try to resolve it as-is and detach at that commit".

The result changes behaviour for two classes of people.

 (1) People who have a branch whose name is refs/heads/frotz.  Before they
     could switch to the branch by mechanically giving its name.  Now they
     have to remember that such a branch with an unusual name needs to be
     fully spelled "git checkout refs/heads/refs/heads/frotz".

 (2) People who have scripts that gets a refname (or a list of refnames),
     and drive "git checkout" to either switch to the branch if that ref
     is a branch or detach at the commit if it isn't.  Their script had to
     check if the ref begins with refs/heads/ and if so strip that before
     giving it to "git checkout", but with your change they do not have
     to.

The former technically is a usability regression which I presume we do not care. The user with such a branch name is sick enough and deserve to be punished ;-)

The latter might be improvement, but does it really matter?

Such a script is already checking with refs/heads/ (and it is easy to codify in a script anyway), and with or without the change you suggest, it will check out the master branch when the ref is refs/heads/master and the script strips the leading refs/heads/. With the change, it also needs to delete the logic of stripping refs/heads/ to deal with (1) sanely.

In addition, most likely such a script to check out a series of refs in an automated fashion is about autobuilding random set of commits, and it would probably be better off working on the HEAD detached at given commit, whether the incoming ref happens to be a branch ref or not.

So I am still scratching my head, wondering what improvement from the end user's point of view you will be getting from such a change...

On Monday 13 April 2009 12:31:31 Junio C Hamano wrote:

I think the question being posed is: Would unifying branch names across all git commands (i.e., always accepting refs/heads/master as naming branch master, and accepting master when that is unambiguous) sufficiently benefit new users trying to learn git that it would be worth the change? The fact that refs/heads/master will be interpreted as branch or non-branch, and possibly as refs/heads/refs/heads/master, being a different branch, across different git commands is certainly not "intuitively obvious" to new users.

In this vein, I suggest that
	$ git checkout --detach master
as a way to get a detached HEAD on branch master is more understandable than
	$ git checkout refs/heads/master

Mark

Mark Levedahl <mlevedahl@gmail.com> writes:

My knee-jerk reaction is that what you are saying is backwards.

When do new people even need to know about refs/heads vs refs/tags with modern Porcelain command set?

Mark Levedahl wrote:

To detach, use
        $ git checkout master^0

On Tue, Apr 14, 2009 at 01:36:58PM +0200, Jakub Narebski wrote:

I have often though a "--detach" would be useful, just because the ref^0 syntax is so mysterious (and yes, _I_ know what it does, but explaining why it works to a novice git user is just painful).

-Peff

On Mon, Apr 13, 2009 at 09:31:31AM -0700, Junio C Hamano wrote:

It is entirely about simplifying the mental model of "what is a branch" for the user, and making it easier to explain in the documentation.

I think otherwise your analysis was right: it doesn't really buy much in practical usage, and it slightly hurts people with crazy branch names like "refs/heads/refs/heads/foo" (but I consider that unlikely enough not to worry about).

-Peff

On Mon, Apr 13, 2009 at 04:56:22AM -0400, Jeff King wrote:

I meant to attach a patch here so you could judge for yourself, but I forgot. I am preparing a patch series which I think is better, but for reference, here is what I came up with before:

---

On Mon, Apr 13, 2009 at 03:13:06PM +0200, Sverre Rabbelier wrote:

Oops, actually it is supposed to be:

 'git checkout' ... -b <new_branch> [<start_point>]

The cause where "-b <new_branch>" isn't used is covered in the line above.

-Peff

Heya,

On Mon, Apr 13, 2009 at 15:19, Jeff King <peff@peff.net> wrote:

Ah, it is indeed, thatm akes it even more readable! :)