On 01/30, Junio C Hamano wrote:
> Thomas Gummerer <t.gumme...@gmail.com> writes:
> 
> > Introduce a new git stash push verb in addition to git stash save.  The
> > push verb is used to transition from the current command line arguments
> > to a more conventional way, in which the message is specified after a -m
> > parameter instead of being a positional argument.
> 
> I think the canonical way to express that is "... the message is
> given as an argument to the -m option" (i.e. some options take an
> argument, some others do not, and the "-m" takes one).
> 
> > This allows introducing a new filename argument to stash single files.
> 
> I do not want them to be "a filename argument", and I do not think
> you meant them as such, either.  
> 
>     This allows us to have pathspecs at the end of the command line
>     arguments like other Git commands do, so that the user can say
>     which subset of paths to stash (and leave others behind).

Yeah, this is much better, thanks.

> > +save_stash () {
> > +   push_options=
> > +   while test $# != 0
> > +   do
> > +           case "$1" in
> > +           -k|--keep-index)
> > +...
> > +           esac
> > +           shift
> > +   done
> 
> It is a bit unfortunate that we need to duplicate the above
> case/esac here.  I do not know if doing it this way:
> 
>       case "$1" in
>       --)
>               shift
>               break 
>               ;;
>       --help)
>               show_help
>               ;;
>       -*)
>               # pass all options through to push_stash
>               push_options="$push_options $1"
>               ;;
>       *)
>               break
>                 ;;
>       esac
> 
> and letting push_stash complain for an unknown option is easier to
> maintain.

I think this will work out nicely.  Will try to implement it this way.

> You are reversing the order of the options in the loop.  Don't.

-- 
Thomas

Reply via email to