Re: [PATCH v4 5/5] stash: implement builtin stash

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 26, 2017 at 8:53 AM Matthieu Moy
<Matthieu.Moy@xxxxxxxxxxxxxxx> wrote:
>
> Thomas Gummerer <t.gummerer@xxxxxxxxx> writes:
>
> > After the line
> >
> > test -n "$seen_non_option" || set "push" "$@"
> >
> > it's not possible that $# is 0 anymore, so this will never be
> > printed.  From a quick look at the history it seems like it wasn't
> > possible to trigger that codepath for a while.  If I'm reading things
> > correctly 3c2eb80fe3 ("stash: simplify defaulting to "save" and reject
> > unknown options", 2009-08-18) seems to have introduced the small
> > change in behaviour.
>
> Indeed. That wasn't on purpose, but I seem to have turned this
>
>         case $# in
>         0)
>                 push_stash &&
>                 say "$(gettext "(To restore them type \"git stash apply\")")"
>                 ;;
>
> into dead code.
>
> > As I don't think anyone has complained since then, I'd just leave it
> > as is, which makes git stash with no options a little less verbose.
>
> I agree it's OK to keep is as-is, but the original logic (give a bit
> more advice when "stash push" was DWIM-ed) made sense too, so it can
> make sense to re-activate it while porting to C.

I'd be happy either way.  If we decide to restore the original behaviour, I
think it should be done in a separate patch, as the test case will need
some adjustments.  That way we can keep this patch purely as the
conversion step, which makes it a bit easier to review.

> --
> Matthieu Moy
> http://www-verimag.imag.fr/~moy/



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux