Re: [PATCH] git clean: Don't automatically remove directories when run within subdirectory
- From
Shawn Bohrer <shawn.bohrer@gmail.com>
- Date
- Apr 14, 2008, 17:06 UTC
- Message-ID
- <20080414170643.GA10548@mediacenter>
- In-Reply-To
- <7v8wzgaoqy.fsf@gitster.siamese.dyndns.org>
On Mon, Apr 14, 2008 at 12:18:13AM -0700, Junio C Hamano wrote:
Show 10 quoted lines
> Shawn Bohrer <shawn.bohrer@gmail.com> writes: > > - int len, pos, matches; > > + int len, pos; > > + int matches = 0; > > struct cache_entry *ce; > > struct stat st; > > Initialization of "matches" seems to be an independent clean-up. Although > it forces the initialization in the codepath that do not need the value of > matches, that is not a big deal --- right?
Yes this is an independent clean-up. I can't see any harm in forcing the initializtion.
Show 10 quoted lines
> > - matches = match_pathspec(pathspec, ent->name, ent->len,
> > + matches = match_pathspec(pathspec, ent->name, len,
> > baselen, seen);
> > - } else {
> > - matches = 0;
> > }
>
> And the essential change (fix) is to send len which could be shorter than
> ent->len because we have stripped '/' here, plus the one in match_one()
> that now allows name[] that is not NUL terminated.Yep, I'll add that to the changelog.
Show 7 quoted lines
> > - if (show_only && (remove_directories || matches)) {
> > + if (show_only && (remove_directories || (matches >= 2))) {
> > printf("Would remove %s\n", qname);
> > - } else if (remove_directories || matches) {
> > + } else if (remove_directories || (matches >= 2)) {
>
> These magic numbers are bad. Please update it to use symbolic constants.Agreed I'll send an updated patch later tonight. One additional thought though. 2 is MATCHED_FNMATCH which worries me a little because I think this would mean 'git clean -f *' will also remove directories (I haven't tried though). Perhaps this should really be 3 MATCHED_EXACTLY just to be safe. Does anyone have opinions either way?
-- Shawn