Re: [PATCH v2] send-email: add --confirm option
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Mar 2, 2009, 07:34 UTC
- Message-ID
- <7vwsb85qe9.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <1235924234-16923-1-git-send-email-jaysoffian@gmail.com>
Jay Soffian <jaysoffian@gmail.com> writes:
Show 33 quoted lines
> send-email violates the principle of least surprise by automatically > cc'ing additional recipients without confirming this with the user. > > This patch teaches send-email a --confirm option. It takes the > following values: > > --confirm=always always confirm before sending > --confirm=never never confirm before sending > --confirm=cc confirm before sending when send-email has > automatically added addresses from the patch to > the Cc list > --confirm=compose confirm before sending the first message when > using --compose. (Needed to maintain backwards > compatibility with existing behavior.) > --confirm=auto 'cc' + 'compose' > > The option defaults to 'compose' if any suppress Cc related options have > been used, otherwise it defaults to 'auto'. > > Unfortunately, it is impossible to introduce this patch such that it > helps new users without potentially upsetting some existing users. We > attempt to mitigate the latter by: > > * Allowing the user to "git config sendemail.config never" > * Allowing the user to say "all" after the first prompt to not be > prompted on remaining emails during the same invocation. > * Telling the user about the "sendemail.confirm" setting whenever they > use "all" > * Only prompting if no --suppress related options have been passed, as > using such an option is likely to indicate an experienced send-email > user. > > Signed-off-by: Jay Soffian <jaysoffian@gmail.com>
Other than that the "sendemail.config never" is probably a typo, I think this is the best you (or anybody) could do at this moment, unless we take the route to introduce an "improved and different command", which I actually am slightly in favor.
In any case, with the lesson I learned from the post v1.6.0 fiasco, it might make sense to make the command louder when it needs to look at the setting of $confirm option and when the user does not have anything in the config nor command line.
What I mean is this.
In this codepath,
Show 5 quoted lines
> + $needs_confirm = ( > + $confirm eq "always" or > + ($confirm =~ /^(?:auto|cc)$/ && @cc) or > + ($confirm =~ /^(?:auto|compose)$/ && $compose && $message_num == 1)); > +
if @cc is not empty, or $compose is true, you _need to know_ what the user wants to happen, and you need to look at $confirm to decide (otherwise the value of $confirm does not matter). In such a case, if the repository is unconfigured with a sendemail.confirm configuration and the there was no explicit --confirm from the command line, you do not know what the user wants. Instead of silently assuming 'auto' to confirm and potentially annoying the users with this new extra step, the command could also say "by the way, if you do not want to see this message, you can squelch it by 'git config sendemail.confirm never'" when it defaults to 'auto'.
The command could also refuse to work in such a case and instead explain what the user's choices are, and instruct "say 'git config sendemail.confirm auto' if you are not sure" or something like that, but I think for this particular new option it is a bit overkill and I wouldn't suggest going that far.