Thomas Gummerer <t.gumme...@gmail.com> writes:

> -             git reset --hard ${GIT_QUIET:+-q}

This hunk is probably the most important one to review in the whole
series, in the sense that these are entirely new code that didn't
exist in the original.

> +             if test $# != 0
> +             then
> +                     saved_untracked=
> +                     if test -n "$(git ls-files --others -- "$@")"

I notice that "ls-files -o" used in the code before this series are
almost always used with --exclude-standard but we do not set up the
standard exclude pattern here.  Is there a good reason to use (or
not to use) it here as well?

> +                     then
> +                             saved_untracked=$(
> +                                     git ls-files -z --others -- "$@" |
> +                                         xargs -0 git stash create -u all --)
> +                     fi

Running the same ls-files twice look somewhat wasteful.

I suspect that we avoid "xargs -0" in our code from portability
concern (isn't it a GNU invention?)

> +                     git ls-files -z -- "$@" | xargs -0 git reset 
> ${GIT_QUIET:+-q} --

Hmm, am I being naive to suspect that the above is a roundabout way
to say:

        git reset ${GIT_QUIET:+-q} -- "$@"

or is an effect quite different from that intended here?

> +                     git ls-files -z --modified -- "$@" | xargs -0 git 
> checkout ${GIT_QUIET:+-q} HEAD --

Likewise.  Wouldn't the above be equivalent to:

        git checkout ${GIT_QUIET:+-q} HEAD -- "$@"

Or is this trying to preserve paths modified in the working tree and
fully added to the index?


> +                     if test -n "$(git ls-files -z --others -- "$@")"
> +                     then
> +                             git ls-files -z --others -- "$@" | xargs -0 git 
> clean --force -d ${GIT_QUIET:+-q} --

Likewise.  "ls-files --others" being the major part of what "clean"
is about, I suspect the "ls-files piped to clean" is redundant.  Do
you even need a test?  IOW, doesn't "git clean" with a pathspec that
does not match anything silently succeed without doing anything
harmful?

> +                     fi
> +                     if test -n "$saved_untracked"
> +                     then
> +                             git stash pop -q $saved_untracked

I see this thing was "created", and the whole point of "create" is
to be different from "save/push" that automatically adds the result
to the stash reflog.  Should we be "pop"ing it, or did you mean to
just call apply_stash on it?

> +                     fi
> +             else
> +                     git reset --hard ${GIT_QUIET:+-q}
> +             fi

Reply via email to