From: Junio C Hamano <gitster@pobox.com>
To: Jay Soffian <jaysoffian@gmail.com>
Cc: git@vger.kernel.org, Nanako Shiraishi <nanako3@lavabit.com>,
Paul Gortmaker <paul.gortmaker@windriver.com>
Subject: Re: [PATCH v2] send-email: add --confirm option
Date: Sun, 01 Mar 2009 23:34:54 -0800 [thread overview]
Message-ID: <7vwsb85qe9.fsf@gitster.siamese.dyndns.org> (raw)
In-Reply-To: <1235924234-16923-1-git-send-email-jaysoffian@gmail.com> (Jay Soffian's message of "Sun, 1 Mar 2009 11:17:14 -0500")
Jay Soffian <jaysoffian@gmail.com> writes:
> 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,
> + $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.
next prev parent reply other threads:[~2009-03-02 7:36 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2009-03-01 8:23 [PATCH] send-email: add --confirm option Jay Soffian
2009-03-01 9:03 ` Junio C Hamano
2009-03-01 14:05 ` Jay Soffian
2009-03-01 16:17 ` [PATCH v2] " Jay Soffian
2009-03-01 17:09 ` Paul Gortmaker
2009-03-01 17:49 ` Jay Soffian
2009-03-02 8:24 ` Nanako Shiraishi
2009-03-02 9:01 ` Junio C Hamano
2009-03-02 9:23 ` Nanako Shiraishi
2009-03-02 10:35 ` Felipe Contreras
2009-03-02 12:33 ` Jay Soffian
2009-03-02 7:34 ` Junio C Hamano [this message]
2009-03-02 12:35 ` Jay Soffian
2009-03-03 2:47 ` Junio C Hamano
2009-03-03 4:52 ` [PATCH v3] send-email: add --confirm option and configuration setting Jay Soffian
2009-03-03 6:53 ` Junio C Hamano
2009-03-03 10:11 ` Nanako Shiraishi
2009-03-03 11:54 ` Bert Wesarg
2009-03-03 16:22 ` Jay Soffian
2009-03-03 16:48 ` Junio C Hamano
2009-03-03 18:05 ` Bert Wesarg
2009-03-03 18:18 ` Jay Soffian
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vwsb85qe9.fsf@gitster.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=git@vger.kernel.org \
--cc=jaysoffian@gmail.com \
--cc=nanako3@lavabit.com \
--cc=paul.gortmaker@windriver.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).