Re: [PATCH] git clean: Don't automatically remove directories when run within subdirectory

From: Shawn Bohrer <shawn.bohrer@gmail.com>
Date: Apr 15, 2008, 14:26 UTC
Message-ID: <20080415142601.GB10548@mediacenter>
In-Reply-To: <48044C33.20006@broadpark.no>

On Tue, Apr 15, 2008 at 08:33:23AM +0200, Joachim Berdal Haga wrote:

Show 30 quoted lines

> Shawn Bohrer wrote:
> > On Mon, Apr 14, 2008 at 08:18:13PM +0200, Joachim Berdal Haga wrote:
> >> I think that the best option would be to never remove a directory, even if
> >> given explicitly, unless -d is given. Because my gut feeling is that when a
> >> directory name is specified, it is most often meant as "clean inside the
> >> given directory", ie. as a path delimiter.
> > 
> > If there are no tracked files the only difference between the dir/ and
> > dir case is that the former will leave behind an empty directory.  So
> > the difference between too much and too little is of little importance.
> 
> No, check this out; note that only in the very last case dir/subdir/subfile
> would be removed.
> 
> $ git init; mkdir -p dir/subdir; touch dir/file dir/subdir/subfile
> Initialized empty Git repository in .git/
> $ touch dir/tracked-file; git add dir/tracked-file
> $ ~/src/git/git-clean -n dir/
> Would remove dir/file
> Would not remove dir/subdir/
> $ ~/src/git/git-clean -n dir
> Would remove dir/file
> Would not remove dir/subdir/
> $ git rm -f dir/tracked-file
> rm 'dir/tracked-file'
> $ ~/src/git/git-clean -n dir/
> Would remove dir/file
> Would not remove dir/subdir/
> $ ~/src/git/git-clean -n dir
> Would remove dir/

Ah of course, this is the behavior with my patch. Before it would have removed everything which is the same bug you initially reported :)

Show 11 quoted lines

> > However,
> > 
> > git clean dir
> > Would not remove dir/
> > 
> > is a little strange.
> 
> Yes, although it could be made less strange by adding a short explanation,
> like "Would not remove dir/ (-d not given)". But I also think that the
> difference between "dir" and "dir/" is very (too?) subtle in this case and
> therefore should require explicit approval/action from the user.

Yeah, I don't know how I feel about this. I do think that the behavior with my current patch is technically correct, but you may be right that a trailing slash is subtle. In most cases I use my shell's tab completion witch adds the trailing slash, and only remove it when needed. Additionally, I could argue that by default we require explicit action to clean files by requiring -n or -f so hopefully users try -n first (I do).

-- Shawn