On Fri, Apr 22 2022, Alyssa Ross wrote:
> [[ Whoops, sent to the list this time! ]]
It is somewhat ironic to have an accidental E-Mail send given the patch
subject :)
> Ævar, thanks for encouraging me to send a patch. At your suggestion,
> I've trawled through the list archives looking for any previous
> discussion of this default, but I didn't find anything.
Glad to see a follow-up to this. Thanks.
> Documentation/git-send-email.txt | 3 +--
> git-send-email.perl | 2 +-
> t/t9001-send-email.sh | 14 ++++----------
> 3 files changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt
> index 41cd8cb424..b791d83bb7 100644
> --- a/Documentation/git-send-email.txt
> +++ b/Documentation/git-send-email.txt
> @@ -407,8 +407,7 @@ Administering
> --
> +
> Default is the value of `sendemail.confirm` configuration value; if that
> -is unspecified, default to 'auto' unless any of the suppress options
> -have been specified, in which case default to 'compose'.
> +is unspecified, default to 'always'.
>
> --dry-run::
> Do everything except actually send the emails.
> diff --git a/git-send-email.perl b/git-send-email.perl
> index 5861e99a6e..4aa7d83cdc 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -606,7 +606,7 @@ sub config_regexp {
> # Set confirm's default value
> my $confirm_unconfigured = !defined $confirm;
> if ($confirm_unconfigured) {
> - $confirm = scalar %suppress_cc ? 'compose' : 'auto';
> + $confirm = 'always';
> };
I squinted a bit at this and wonder now that we don't have that ternary
whether we can simplify this a bit.
Isn't this the same now as:
my $confirm_unconfigured = !defined $confirm;
$confirm ||= 'always'; # well, defined-or, but I don't think we care here.
But probably not worth obsessing over it... :) I.e. it seems fine as-is.
> -test_expect_success $PREREQ 'No confirm with --suppress-cc' '
> - test_no_confirm --suppress-cc=sob &&
> +test_expect_success $PREREQ 'No confirm with --confirm=compose --suppress-cc' '
> + test_no_confirm --confirm=compose --suppress-cc=sob &&
> check_no_confirm
> '
>
> @@ -1032,16 +1032,10 @@ test_expect_success $PREREQ '--confirm=compose' '
> test_confirm --confirm=compose --compose
> '
>
> -test_expect_success $PREREQ 'confirm by default (due to cc)' '
> +test_expect_success $PREREQ 'confirm by default' '
> test_when_finished git config sendemail.confirm never &&
> git config --unset sendemail.confirm &&
> - test_confirm
> -'
> -
> -test_expect_success $PREREQ 'confirm by default (due to --compose)' '
> - test_when_finished git config sendemail.confirm never &&
> - git config --unset sendemail.confirm &&
> - test_confirm --suppress-cc=all --compose
> + test_confirm --suppress-cc=all
> '
It looks like we're losing some coverage here (I may be wrong). I.e. yes
we should update tests and descriptions, but for lines that test what we
did with a given command-line before let's turn those into tests that
positively assert what we do now.
I.e. we lost the "--suppress-cc=sob" test (with no other option), and
also "--suppress-cc=all --compose"?
