Re: [PATCH] stash: added safety flag for stash clear subcommand

[Junio C Hamano <gitster@pobox.com>]

Nadav Goldstein <nadav.goldstein@blazepod.co> writes:

> If we make "git stash clear" to by default not clear the stash and
> just present the stashes to be dropped (and require -f to force
> clearing the stash), wouldn't we essentially make a duplicate of "git
> stash list"?

Correct, but that is not what I would imagine to be a parallel to
"git clean [-f|-n]" with clean.requireForce.  If you haven't played
with "git clean" with various settings of that variable, you should.
Its behaviour illustrates pretty well what I meant in the message
that prompted your response, to which I am responding to.  I would
imagine that "git stash clear [-f|-n]" with stashClear.requireForce
to be more like:

 * stashClear.requireForce that is not set is treated as if it is
   set to true;

 * "git stash clear" without -f or -n will die with a single line of
   message, telling the user to give either -f or -n, unless the
   configuration is set explicitly to 'false'.

It will be more like "confirmation" to those who run "git stash
clear", and then if they are certain, run "git stash clear -f".

Of course, "interactive" can be added like "git clean -i".

Just like "-i" defeats the refusal by "clear.requireForce" when
running "git clean", "git stash clear -i" would go interactive and
should be usable regardless of what value stashClear.requireForce
has.

> Regarding making it opt out instead of opt in, I am afraid there are
> tools that use git stash clean automatically, and making the
> confirmation opt-out (by default confirm) we can potentially break
> them, don't you think it would be more gentle to the git community to
> make it opt in?

It is usually a good idea to introduce such a disruptive feature
gradually by first making it opt-in, with a warning message to tell
users that the default will change in the future.  For this feature,
it may go like this:

 1. Phase one.

 * "git stash clear" learns "-f|--force" and "-n|--dry-run"
   options.

 * A configuration variable stashClear.requireForce is introduced.
   "git stash clear" starts paying attention to it.  When it is

    - set to true, "git stash clear" without "-f" or "-n" errors
      out.

    - set to false, "git stash clear" works just like today.  No
      extra messages, no extra errors.

    - unset, "git stash clear" gives a warning message to tell the
      users two things:

      - The stashClear.requireForce configuration variable exists
        and when it is set to true, you need to pass "-f" to make
        "git stash clear" actually clear.  When it is set to false,
        the stash is cleared just like before.  When it is unset,
        you will see this warning message, but the command clears
        the stash.

      - The default value of stashClear.requireForce will become
        true in a future version of Git, and users are encouraged to
        set their preference early by setting the configuration
        variable now.

      but otherwise it goes ahead and clears the stash.

 * Release notes talks about upcoming default flip for the variable.

 2. Phase two (probably several releases after Phase one).

 * The behaviour of "git stash clear" is changed so that when
   stashClear.requireForce is

    - set to true of false, it behaves identically as phase one.

    - unset, "git stash clear" gives a warning message to tell the
      users about the stashClear.requireForce variable just like in
      phase one (the description needs to be adjusted to say that
      the default value is now 'true'), and behaves as if the
      variable is set to 'true'.

 3. Phase three (probably several releases after Phase two).

 * Long after users have adjusted to the new "safer by default"
   behaviour, we shorten the error message given to those who do not
   have the variable configured, similar to the message "git clean"
   gives to those without clear.requireForce.