Re: [PATCH] only warn about ambiguous refs if stderr is a tty

From: Jeff King <peff@peff.net>
Date: May 9, 2011, 12:49 UTC
Message-ID: <20110509124931.GA18197@sigill.intra.peff.net>
In-Reply-To: <BANLkTimn7542tji-Uu5iH72HS9fcnaywvg@mail.gmail.com>

On Mon, May 09, 2011 at 02:37:48PM +0200, Erik Faye-Lund wrote:

> Yeah, I understood that part. My point was that once the output is
> wanted for diagnostics, you probably also want verbose output. And
> warnings should probably always be output if we're verbose.

Ah, I see. I think my main concern is that the behavior you proposed would simply be surprising to people used to normal unix conventions. But it sounds like we both agree that isn't the right direction anyway.

Show 9 quoted lines

> > Of course, because there is no refs/HEAD at all. I meant "if you have
> > ambiguity between $GIT_DIR/HEAD and $GIT_DIR/refs/HEAD", then saying
> > "refs/HEAD" should disambiguate already. In your example, there is no
> > ambiguity.
> 
> I meant that "refs/HEAD" could be an non-ambiguous alias for HEAD, but
> it's probably easier to just say that 'HEAD' isn't ambiguous. Your
> suggestion of only checking for ambiguousness on the same level is IMO
> an elegant way of doing this.

OK, I see what you meant. But "refs/HEAD" cannot be a shortcut for "HEAD", as it means something totally different. You can have "HEAD", "refs/HEAD", "refs/heads/HEAD" all co-existing.

> I agree. There could be a remote chance that you can get a branch
> called 'HEAD' from some foreign vcs or something, though. But I don't
> think it's very likely, and the problem will also go away if we go
> with your approach mentioned above.

Thinking on it more, I think warning is probably the only sane thing to do there. Having a branch with that name is just going to be confusing in the long run, and the sooner we start making the user aware of the situation, the better.

Show 7 quoted lines

> > I admit I haven't been following this
> > thread too closely. What is the reason not to tell the user "sorry, that
> > is an insane branch name. Accept the ambiguity warning, or choose a
> > different name"?
> 
> I think having the ambiguity warning in itself isn't the problem, it's
> gitk not swallowing it that is.

Agreed.

> The reporter also had some problems pushing with a branch named 'HEAD'
> in his repo, but I didn't look into that part at all.

I expect that would be a separate issue entirely (if it were fetching, I wouldn't be surprised if it was the "fake" refs/remotes/*/HEAD symref we create getting in the way).

-Peff