Re: [PATCH v2 0/2] stash clear: added safety flag for stash clear subcommand

From: Derrick Stolee <derrickstolee@github.com>
Date: May 23, 2022, 19:33 UTC
Message-ID: <c795733a-fcf2-9ca3-3f31-b347b191f725@github.com>
In-Reply-To: <pull.1232.v2.git.1653286345.gitgitgadget@gmail.com>

On 5/23/22 2:12 AM, Nadav Goldstein via GitGitGadget wrote:

Show 11 quoted lines

> PLAN: Add a flag to git stash clear (-i|--interactive), which prompts the
> user to confirm he indeed wants to clear the stash, otherwise it will abort.
> 
> I found it useful as a frequent stash clear user which means my terminal
> always have it in recent commands, which could mean accidental erase of
> work. This flag ensures accidental fires won't clear hard work :)
> 
> I also thought it would be better to do it opt in, to not change the way the
> command works in prod, only add to it.
> 
> Changes since v1:

It looks like you completely changed your approach here. It's perfectly fine to use the same PR and keep the Git mailing list thread intact.

However, it would be good to keep the cover letter and title up-to-date with what you are submitting. (Now that you have multiple patches, GGG will split the PR Title and Description into your cover letter message.)

What you actually implement is a refactor to make 'git clean's interactive menu be implemented in libgit.a.

Show 6 quoted lines

> Removed temporarily the interactive flag from stash. introduced add-menu lib
> to the project, which is simply the extracted code that responsible for
> presenting the menu in the clean command, and made the clean command use it.
> 
> This change was made to allow stash to use interactive as well, with the
> same style as git clean.

It is interesting that you'd model 'git stash -i' after 'git clean -i' instead of something like 'git add -p'. It would be interesting to see your intended 'git stash' menu based on this API before we commit to the extraction.

Thanks, -Stolee