{"thread":{"id":"57893","subject":"Re: [PATCH] stash: added safety flag for stash clear subcommand","startedAt":"2022-05-18T18:54:18Z","lastAt":"2022-05-18T20:51:42Z","messageCount":2,"participants":["Nadav Goldstein","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"455413","messageId":"CAApwGmCsQNK42=5x1OxdovgVxdzj2Vs5H1FaUJhAGCXvmA5pzw@mail.gmail.com","threadId":"57893","inReplyTo":null,"subject":"Re: [PATCH] stash: added safety flag for stash clear subcommand","fromName":"Nadav Goldstein","fromEmail":"nadav.goldstein@blazepod.co","sentAt":"2022-05-18T18:53:36Z","receivedAt":"2022-05-18T18:54:18Z","isPatch":true,"sender":{"key":"nadav.goldstein@blazepod.co","avatar":null},"body":"Thank you all for your detailed replies!\n\nI thought deeply on the issues that you presented, and I wonder\nwhether modeling after git clean --dry-run accomplish the goal I aimed\nto solve.\nIf we make \"git stash clear\" to by default not clear the stash and\njust present the stashes to be dropped (and require -f to force\nclearing the stash), wouldn't we essentially make a duplicate of \"git\nstash list\"?\n\nMy goal is to not be afraid to have git stash clear in your terminal\nhistory, while indeed performing the clearing of the whole stack when\nfired.\n\nI thought presenting a confirmation dialog before clearing would\nsuffice as a solution, but I like the idea of listing all of the\nstashes to be dropped and requesting confirmation (and in the future\nallow selection of specific stashes to be dropped as you suggested).\n\nRegarding making it opt out instead of opt in, I am afraid there are\ntools that use git stash clean automatically, and making the\nconfirmation opt-out (by default confirm) we can potentially break\nthem, don't you think it would be more gentle to the git community to\nmake it opt in? Of course one major downside is that users need to\n\"find\" it so the adaptation will be much slower, but I feel it's for\nthe best.\n\nWhat do you think? I do agree that --interactive is not a good name\nand a bit misleading, can you help me think of a better name that will\nbe more of a suit? maybe -c|--confirm, and using\nstashClear.requireConfirm? Or -p|--prompt?\n"},{"id":"455425","messageId":"xmqqy1yytvho.fsf@gitster.g","threadId":"57893","inReplyTo":"CAApwGmCsQNK42=5x1OxdovgVxdzj2Vs5H1FaUJhAGCXvmA5pzw@mail.gmail.com","subject":"Re: [PATCH] stash: added safety flag for stash clear subcommand","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2022-05-18T20:51:31Z","receivedAt":"2022-05-18T20:51:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nadav Goldstein <nadav.goldstein@blazepod.co> writes:\n\n> If we make \"git stash clear\" to by default not clear the stash and\n> just present the stashes to be dropped (and require -f to force\n> clearing the stash), wouldn't we essentially make a duplicate of \"git\n> stash list\"?\n\nCorrect, but that is not what I would imagine to be a parallel to\n\"git clean [-f|-n]\" with clean.requireForce.  If you haven't played\nwith \"git clean\" with various settings of that variable, you should.\nIts behaviour illustrates pretty well what I meant in the message\nthat prompted your response, to which I am responding to.  I would\nimagine that \"git stash clear [-f|-n]\" with stashClear.requireForce\nto be more like:\n\n * stashClear.requireForce that is not set is treated as if it is\n   set to true;\n\n * \"git stash clear\" without -f or -n will die with a single line of\n   message, telling the user to give either -f or -n, unless the\n   configuration is set explicitly to 'false'.\n\nIt will be more like \"confirmation\" to those who run \"git stash\nclear\", and then if they are certain, run \"git stash clear -f\".\n\nOf course, \"interactive\" can be added like \"git clean -i\".\n\nJust like \"-i\" defeats the refusal by \"clear.requireForce\" when\nrunning \"git clean\", \"git stash clear -i\" would go interactive and\nshould be usable regardless of what value stashClear.requireForce\nhas.\n\n> Regarding making it opt out instead of opt in, I am afraid there are\n> tools that use git stash clean automatically, and making the\n> confirmation opt-out (by default confirm) we can potentially break\n> them, don't you think it would be more gentle to the git community to\n> make it opt in?\n\nIt is usually a good idea to introduce such a disruptive feature\ngradually by first making it opt-in, with a warning message to tell\nusers that the default will change in the future.  For this feature,\nit may go like this:\n\n 1. Phase one.\n\n * \"git stash clear\" learns \"-f|--force\" and \"-n|--dry-run\"\n   options.\n\n * A configuration variable stashClear.requireForce is introduced.\n   \"git stash clear\" starts paying attention to it.  When it is\n\n    - set to true, \"git stash clear\" without \"-f\" or \"-n\" errors\n      out.\n\n    - set to false, \"git stash clear\" works just like today.  No\n      extra messages, no extra errors.\n\n    - unset, \"git stash clear\" gives a warning message to tell the\n      users two things:\n\n      - The stashClear.requireForce configuration variable exists\n        and when it is set to true, you need to pass \"-f\" to make\n        \"git stash clear\" actually clear.  When it is set to false,\n        the stash is cleared just like before.  When it is unset,\n        you will see this warning message, but the command clears\n        the stash.\n\n      - The default value of stashClear.requireForce will become\n        true in a future version of Git, and users are encouraged to\n        set their preference early by setting the configuration\n        variable now.\n\n      but otherwise it goes ahead and clears the stash.\n\n * Release notes talks about upcoming default flip for the variable.\n\n 2. Phase two (probably several releases after Phase one).\n\n * The behaviour of \"git stash clear\" is changed so that when\n   stashClear.requireForce is\n\n    - set to true of false, it behaves identically as phase one.\n\n    - unset, \"git stash clear\" gives a warning message to tell the\n      users about the stashClear.requireForce variable just like in\n      phase one (the description needs to be adjusted to say that\n      the default value is now 'true'), and behaves as if the\n      variable is set to 'true'.\n\n 3. Phase three (probably several releases after Phase two).\n\n * Long after users have adjusted to the new \"safer by default\"\n   behaviour, we shorten the error message given to those who do not\n   have the variable configured, similar to the message \"git clean\"\n   gives to those without clear.requireForce.\n\n"}]}