From: Daniel Barkalow Date: Sat, 19 Jul 2008 17:44:11 GMT Subject: Re: [PATCH] builtin-clone: Use is_dir_sep() instead of '/' Message-ID: In-Reply-To: <200807191549.56402.johannes.sixt@telecom.at> On Sat, 19 Jul 2008, Johannes Sixt wrote: > 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*