Re: [PATCH] Add git-clean command
- From
- Pavel Roskin <proski@gnu.org>
- Date
- Apr 5, 2006, 06:00 UTC
- Message-ID
- <1144216844.3793.10.camel@dv>
- In-Reply-To
- <7vzmj2b3w3.fsf@assigned-by-dhcp.cox.net>
On Mon, 2006-04-03 at 17:06 -0700, Junio C Hamano wrote:
Show 7 quoted lines
> > diff --git a/git-clean.sh b/git-clean.sh > >... > > +for arg in "$@"; do > > for arg > do > ...
I checked other git shell scripts and copied the "while" construct from one of them - if it's good for other commands, it's good for git-clean.
Show 20 quoted lines
> > + if [ "$arg" = "-d" ]; then > > case "$arg" in -d)... > > > +excl1= > > +excl2= > > +if [ -z "$noexclude" ]; then > > + excl1="--exclude-per-directory=.gitignore" > > + if [ -f "$GIT_DIR/info/exclude" ]; then > > + excl2="--exclude-from=$GIT_DIR/info/exclude" > > + fi > > +fi > > + > > +git-ls-files --others --directory "$excl1" "$excl2" | > > +while read -r file; do > > ... > > The $noexclude case passes two empty strings to git-ls-files, > which may happen to be harmless with the current implementation, > but does not feel quite right.
Good catch. This is needed since $GIT_DIR can contain spaces. I believe ${excl2:+"$excl2"} would do the trick.
> Maybe better to read ls-files -z to be really pathname safe, I > dunno.
I think "xargs -0" has its own problems (argument length limitations), and the other solution is to use perl. While at that, I'd rather rewrite the whole script in Perl, or maybe in even C.
I think this should eventually happen to all git scripts, but I have no intention to do it right now unless you really want me to.
Show 7 quoted lines
> > + $echo1 "Removing $file" > > + [ "$cleandirhard" ] && chmod -R 700 "$file" > > I am not quite sure this chmod -R is a good idea. If we are > trying really hard would we need to also make sure we can rmdir > the "$file" by chmod'ing its parent directory? But once we > start doing that where would we stop?
OK, I was undecided on that. I'm dropping this option.
-- Regards, Pavel Roskin