send-email: do defaults -> config -> getopt in that order
Change the git-send-email command-line argument parsing and config reading code to parse those two in the right order. I.e. first we set our hardcoded defaults, then we read our config, and finally we read the command-line, with later sets overriding earlier sets. This fixes a bug introduced in e67a228cd8 ("send-email: automatically determine transfer-encoding", 2018-07-08). That change broke the reading of sendmail.transferencoding because it wasn't careful to update the code to parse them in the previous "defaults -> getopt -> config" order. But as we can see from the history for this file doing it this way was never what we actually wanted, it's just something we grew organically as of 5483c71d7a ("git-send-email: make options easier to configure.", 2007-06-27) and have been dealing with the fallout since, e.g. in 463b0ea22b ("send-email: Fix %config_path_settings handling", 2011-10-14). As can be seen in this change the only place where we actually want to do something clever is with the to/cc/bcc variables, where setting them on the command-line (or using --no-{to,cc,bcc}) should clear out values we grab from the config. All the rest are things where the command-line should simply override the config values, and by reading the config first the config code doesn't need all this "let's not set it, if it was on the command-line" special-casing, as [1] shows we'd otherwise need to care about the difference between whether something was a default or present in config to fix the bug in e67a228cd8. 1. https://public-inbox.org/git/20190508105607.178244-2-gitster@pobox.com/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Ævar Arnfjörð Bjarmason committed
May 9, 2019 at 13:48 UTC
3494dfd3ee058bbeab6a462673d9184dcd694a8b
2 files changed
+61
-43
git-send-email.perl
+49
-42
@@ -169,11 +169,15 @@ my $re_encoded_text = qr/[^? \000-\037\177-\377]+/;
169
my $re_encoded_word = qr/=\?($re_token)\?($re_token)\?($re_encoded_text)\?=/;
170
171
# Variables we fill in automatically, or via prompting:
172
-my (@to,$no_to,@initial_to,@cc,$no_cc,@initial_cc,@initial_bcc,$no_bcc,@xh,
172
+my (@to,@cc,@xh,$envelope_sender,
173
$initial_in_reply_to,$reply_to,$initial_subject,@files,
174
- $author,$sender,$smtp_authpass,$annotate,$use_xmailer,$compose,$time);
175
-
176
-my $envelope_sender;
174
+ $author,$sender,$smtp_authpass,$annotate,$compose,$time);
175
+# Things we either get from config, *or* are overridden on the
176
+# command-line.
177
+my ($no_cc, $no_to, $no_bcc);
178
+my (@config_to, @getopt_to);
179
+my (@config_cc, @getopt_cc);
180
+my (@config_bcc, @getopt_bcc);
181
182
# Example reply to:
183
#$initial_in_reply_to = ''; #<20050203173208.GA23964@foobar.com>';
@@ -220,33 +224,37 @@ sub do_edit {
224
}
225
226
# Variables with corresponding config settings
223
-my ($thread, $chain_reply_to, $suppress_from, $signed_off_by_cc);
227
+my ($suppress_from, $signed_off_by_cc);
228
my ($cover_cc, $cover_to);
229
my ($to_cmd, $cc_cmd);
230
my ($smtp_server, $smtp_server_port, @smtp_server_options);
231
my ($smtp_authuser, $smtp_encryption, $smtp_ssl_cert_path);
232
my ($batch_size, $relogin_delay);
233
my ($identity, $aliasfiletype, @alias_files, $smtp_domain, $smtp_auth);
230
-my ($validate, $confirm);
234
+my ($confirm);
235
my (@suppress_cc);
236
my ($auto_8bit_encoding);
237
my ($compose_encoding);
234
-my $target_xfer_encoding = 'auto';
235
-
238
+# Variables with corresponding config settings & hardcoded defaults
239
my ($debug_net_smtp) = 0; # Net::SMTP, see send_message()
240
+my $thread = 1;
241
+my $chain_reply_to = 0;
242
+my $use_xmailer = 1;
243
+my $validate = 1;
244
+my $target_xfer_encoding = 'auto';
245
246
my %config_bool_settings = (
239
- "thread" => [\$thread, 1],
240
- "chainreplyto" => [\$chain_reply_to, 0],
241
- "suppressfrom" => [\$suppress_from, undef],
242
- "signedoffbycc" => [\$signed_off_by_cc, undef],
243
- "cccover" => [\$cover_cc, undef],
244
- "tocover" => [\$cover_to, undef],
245
- "signedoffcc" => [\$signed_off_by_cc, undef], # Deprecated
246
- "validate" => [\$validate, 1],
247
- "multiedit" => [\$multiedit, undef],
248
- "annotate" => [\$annotate, undef],
249
- "xmailer" => [\$use_xmailer, 1]
247
+ "thread" => \$thread,
248
+ "chainreplyto" => \$chain_reply_to,
249
+ "suppressfrom" => \$suppress_from,
250
+ "signedoffbycc" => \$signed_off_by_cc,
251
+ "cccover" => \$cover_cc,
252
+ "tocover" => \$cover_to,
253
+ "signedoffcc" => \$signed_off_by_cc,
254
+ "validate" => \$validate,
255
+ "multiedit" => \$multiedit,
256
+ "annotate" => \$annotate,
257
+ "xmailer" => \$use_xmailer,
258
);
259
260
my %config_settings = (
@@ -259,12 +267,12 @@ my %config_settings = (
267
"smtpauth" => \$smtp_auth,
268
"smtpbatchsize" => \$batch_size,
269
"smtprelogindelay" => \$relogin_delay,
262
- "to" => \@initial_to,
270
+ "to" => \@config_to,
271
"tocmd" => \$to_cmd,
264
- "cc" => \@initial_cc,
272
+ "cc" => \@config_cc,
273
"cccmd" => \$cc_cmd,
274
"aliasfiletype" => \$aliasfiletype,
267
- "bcc" => \@initial_bcc,
275
+ "bcc" => \@config_bcc,
276
"suppresscc" => \@suppress_cc,
277
"envelopesender" => \$envelope_sender,
278
"confirm" => \$confirm,
@@ -312,8 +320,9 @@ sub read_config {
320
my ($prefix) = @_;
321
322
foreach my $setting (keys %config_bool_settings) {
315
- my $target = $config_bool_settings{$setting}->[0];
316
- $$target = Git::config_bool(@repo, "$prefix.$setting") unless (defined $$target);
323
+ my $target = $config_bool_settings{$setting};
324
+ my $v = Git::config_bool(@repo, "$prefix.$setting");
325
+ $$target = $v if defined $v;
326
}
327
328
foreach my $setting (keys %config_path_settings) {
@@ -325,15 +334,13 @@ sub read_config {
334
}
335
}
336
else {
328
- $$target = Git::config_path(@repo, "$prefix.$setting") unless (defined $$target);
337
+ my $v = Git::config_path(@repo, "$prefix.$setting");
338
+ $$target = $v if defined $v;
339
}
340
}
341
342
foreach my $setting (keys %config_settings) {
343
my $target = $config_settings{$setting};
334
- next if $setting eq "to" and defined $no_to;
335
- next if $setting eq "cc" and defined $no_cc;
336
- next if $setting eq "bcc" and defined $no_bcc;
344
if (ref($target) eq "ARRAY") {
345
unless (@$target) {
346
my @values = Git::config(@repo, "$prefix.$setting");
@@ -341,7 +348,8 @@ sub read_config {
348
}
349
}
350
else {
344
- $$target = Git::config(@repo, "$prefix.$setting") unless (defined $$target);
351
+ my $v = Git::config(@repo, "$prefix.$setting");
352
+ $$target = $v if defined $v;
353
}
354
}
355
@@ -355,6 +363,10 @@ sub read_config {
363
}
364
}
365
366
+$identity = Git::config(@repo, "sendemail.identity");
367
+read_config("sendemail.$identity") if defined $identity;
368
+read_config("sendemail");
369
+
370
# Begin by accumulating all the variables (defined above), that we will end up
371
# needing, first, from the command line:
372
@@ -369,12 +381,12 @@ $rc = GetOptions(
381
"in-reply-to=s" => \$initial_in_reply_to,
382
"reply-to=s" => \$reply_to,
383
"subject=s" => \$initial_subject,
372
- "to=s" => \@initial_to,
384
+ "to=s" => \@getopt_to,
385
"to-cmd=s" => \$to_cmd,
386
"no-to" => \$no_to,
375
- "cc=s" => \@initial_cc,
387
+ "cc=s" => \@getopt_cc,
388
"no-cc" => \$no_cc,
377
- "bcc=s" => \@initial_bcc,
389
+ "bcc=s" => \@getopt_bcc,
390
"no-bcc" => \$no_bcc,
391
"chain-reply-to!" => \$chain_reply_to,
392
"no-chain-reply-to" => sub {$chain_reply_to = 0},
@@ -423,6 +435,11 @@ $rc = GetOptions(
435
"relogin-delay=i" => \$relogin_delay,
436
);
437
438
+# Munge any "either config or getopt, not both" variables
439
+my @initial_to = @getopt_to ? @getopt_to : ($no_to ? () : @config_to);
440
+my @initial_cc = @getopt_cc ? @getopt_cc : ($no_cc ? () : @config_cc);
441
+my @initial_bcc = @getopt_bcc ? @getopt_bcc : ($no_bcc ? () : @config_bcc);
442
+
443
usage() if $help;
444
unless ($rc) {
445
usage();
@@ -435,16 +452,6 @@ die __("`batch-size` and `relogin` must be specified together " .
452
"(via command-line or configuration option)\n")
453
if defined $relogin_delay and not defined $batch_size;
454
438
-# read configuration from [sendemail "$identity"], fall back on [sendemail]
439
-$identity = Git::config(@repo, "sendemail.identity") unless (defined $identity);
440
-read_config("sendemail.$identity") if (defined $identity);
441
-read_config("sendemail");
442
-
443
-# fall back on builtin bool defaults
444
-foreach my $setting (values %config_bool_settings) {
445
- ${$setting->[0]} = $setting->[1] unless (defined (${$setting->[0]}));
446
-}
447
-
455
# 'default' encryption is none -- this only prevents a warning
456
$smtp_encryption = '' unless (defined $smtp_encryption);
457
t/t9001-send-email.sh
+12
-1
@@ -1433,7 +1433,18 @@ test_expect_success $PREREQ '--transfer-encoding overrides sendemail.transferEnc
1433
test -z "$(ls msgtxt*)"
1434
'
1435
1436
-test_expect_success $PREREQ 'sendemail.transferencoding=8bit' '
1436
+test_expect_success $PREREQ 'sendemail.transferencoding=8bit via config' '
1437
+ clean_fake_sendmail &&
1438
+ git -c sendemail.transferencoding=8bit send-email \
1439
+ --smtp-server="$(pwd)/fake.sendmail" \
1440
+ email-using-8bit \
1441
+ 2>errors >out &&
1442
+ sed '1,/^$/d' msgtxt1 >actual &&
1443
+ sed '1,/^$/d' email-using-8bit >expected &&
1444
+ test_cmp expected actual
1445
+'
1446
+
1447
+test_expect_success $PREREQ 'sendemail.transferencoding=8bit via cli' '
1448
clean_fake_sendmail &&
1449
git send-email \
1450
--transfer-encoding=8bit \