Re: [PATCH 1/2] remote: write correct fetch spec when renaming remote 'remote'

From: Martin von Zweigbergk <martin.von.zweigbergk@gmail.com>
Date: Sep 2, 2011, 00:02 UTC
Message-ID: <alpine.DEB.2.00.1109011945560.7066@debian>
In-Reply-To: <20110901024211.GC31838@sigill.intra.peff.net>

On Wed, 31 Aug 2011, Jeff King wrote:

Show 11 quoted lines

> On Wed, Aug 31, 2011 at 09:50:42PM -0400, Martin von Zweigbergk wrote:
> 
> > Reduce the risk of changing incorrect sections of the refspec by
> > requiring the string to be matched to have leading and trailing
> > slashes, i.e. match "/<name>/" instead of just "<name>".
> 
> Doesn't this just mean that:
> 
>   git remote rename remotes foo
> 
> will break in the same way?

Yes, "r", "ead" and such now work and only exactly "refs", "heads" and "remotes" don't work. Sorry, forgot to mention that in the commit message.

Show 5 quoted lines

> > We could have required even a leading ":refs/remotes/", but that would
> > mean that we would limit the types of refspecs we could help the user
> > update.
> 
> Actually, I think it's better to be more conservative [...]

I think you are right. That's what I did (match all the way from "refs/...") at first, but then I thought that maybe the reason the matching was relaxed was to allow non-default refspecs. However, as you point out in the footnote to your reply to PATCH 2/2, that case is not working consistently anyway, so it seems safe (w.r.t. backwards compatibility too) to ignore it. Will switch back to matching ":refs/remotes/$OLD".

Martin