Re: [PATCH] builtin-clone: Use is_dir_sep() instead of '/'

From: Daniel Barkalow <barkalow@iabervon.org>
Date: Jul 19, 2008, 17:44 UTC
Message-ID: <alpine.LNX.1.00.0807191333550.19665@iabervon.org>
In-Reply-To: <200807191549.56402.johannes.sixt@telecom.at>

On Sat, 19 Jul 2008, Johannes Sixt wrote:

Show 18 quoted lines

> On Samstag, 19. Juli 2008, Johannes Sixt wrote:
> > On Samstag, 19. Juli 2008, Junio C Hamano wrote:
> > > Ok, but the surrounding code in this function look very suspicious.
> >
> > How about this then?
> >
> > -- snip --
> > builtin-clone: Rewrite guess_dir_name()
> >
> > The function has to do three small and independent tasks, but all of them
> > were crammed into a single loop. This rewrites the function entirely by
> > unrolling these tasks.
> 
> Sigh. I knew it, I knew it. If it had been that trivial, then Daniel had done
> it this way in the first place. :-(
> 
> This needs to be squashed in. It makes sure that we handle 'foo/.git';
> and .git was not stripped if we cloned from 'foo.git/'.

I actually got that from Johannes Schindelin, who added bundle support to my clone patch. I remember looking at his change, thinking it looked overly complicated, but finding that anything I tried to do to simplify it failed tests. If this gets through the test suite (lots of the tests other than the clone test try to do a wider variety of odd things than I expect users do in practice most of the time), then it's probably a better implementation.

	-Daniel
*This .sig left intentionally blank*