diff mbox series

Re* [PATCH 1/1] send-email: fix transferencoding config option

Message ID xmqqef59gy10.fsf_-_@gitster-ct.c.googlers.com (mailing list archive)
State New, archived
Headers show
Series Re* [PATCH 1/1] send-email: fix transferencoding config option | expand

Commit Message

Junio C Hamano May 8, 2019, 8:18 a.m. UTC
Junio C Hamano <gitster@pobox.com> writes:

> I wonder if we can follow the pattern used by the code to handle the
> fallback for %config_bool_settings we can see immediately after
> these two calls to read_config()?  That is, each of the element in
> the %config_* hash is not merely a pointer to where the value is
> stored, but also knows what the default fallback value should be,
> and a loop _in the caller of_ read_config(), after it finishes
> making calls to the read_config function, fills in the missing
> default?

So, here is a two-patch series that tries to do so, primarily done
to gauge if there still is the level of interest needed to make it
worth for us to pursue this topic.  Here is the first one; I'll send
the second one that takes advantage of this change separately (but
it should be trivial to imagine what that step would involve).

-- >8 --
Subject: [PATCH 1/2] send-email: update the mechanism to set default configuration values

The program has a good mechanism to specify the fallback default
values for boolean configuration variables after two invocations of
read_config() for "sendmail.$ident.$var" and "sendemail.$var" have
not found any configuration.  Imitate it so that we can set the
default values for non-boolean variables as well.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 git-send-email.perl | 51 ++++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 22 deletions(-)

Comments

Junio C Hamano May 8, 2019, 10:13 a.m. UTC | #1
Junio C Hamano <gitster@pobox.com> writes:

> So, here is a two-patch series that tries to do so, primarily done
> to gauge if there still is the level of interest needed to make it
> worth for us to pursue this topic.  Here is the first one; I'll send
> the second one that takes advantage of this change separately (but
> it should be trivial to imagine what that step would involve).
>
> -- >8 --
> Subject: [PATCH 1/2] send-email: update the mechanism to set default configuration values
>
> The program has a good mechanism to specify the fallback default
> values for boolean configuration variables after two invocations of
> read_config() for "sendmail.$ident.$var" and "sendemail.$var" have
> not found any configuration.  Imitate it so that we can set the
> default values for non-boolean variables as well.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  git-send-email.perl | 51 ++++++++++++++++++++++++++-------------------
>  1 file changed, 29 insertions(+), 22 deletions(-)

This one was embarrassingly buggy, and needs the following squashed
in.

Sorry about that.

 git-send-email.perl | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index ca7faff094..831947c7ed 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -411,7 +411,7 @@ sub read_config {
 	}
 
 	foreach my $setting (keys %config_settings) {
-		my $target = $config_settings{$setting};
+		my $target = $config_settings{$setting}->[0];
 		next if $setting eq "to" and defined $no_to;
 		next if $setting eq "cc" and defined $no_cc;
 		next if $setting eq "bcc" and defined $no_bcc;
@@ -447,10 +447,13 @@ sub read_config {
 }
 
 # fall back to builtin defaults
-for my $setting (values %config_settings) {
-	if (@$setting == 2 && !defined (${$setting->[0]})) {
-		${$setting->[0]} = $setting->[1];
-	}
+while (my ($name, $setting) = each %config_settings) {
+	next unless @$setting == 2;
+
+	my ($target, $default) = @$setting;
+	if (ref($target) eq "SCALAR") {
+		$$target = $default unless defined $target;
+	} # elsif ... for other types later.
 }
 
 # 'default' encryption is none -- this only prevents a warning
diff mbox series

Patch

diff --git a/git-send-email.perl b/git-send-email.perl
index f4c07908d2..ca7faff094 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -250,28 +250,28 @@  sub do_edit {
 );
 
 my %config_settings = (
-    "smtpserver" => \$smtp_server,
-    "smtpserverport" => \$smtp_server_port,
-    "smtpserveroption" => \@smtp_server_options,
-    "smtpuser" => \$smtp_authuser,
-    "smtppass" => \$smtp_authpass,
-    "smtpdomain" => \$smtp_domain,
-    "smtpauth" => \$smtp_auth,
-    "smtpbatchsize" => \$batch_size,
-    "smtprelogindelay" => \$relogin_delay,
-    "to" => \@initial_to,
-    "tocmd" => \$to_cmd,
-    "cc" => \@initial_cc,
-    "cccmd" => \$cc_cmd,
-    "aliasfiletype" => \$aliasfiletype,
-    "bcc" => \@bcclist,
-    "suppresscc" => \@suppress_cc,
-    "envelopesender" => \$envelope_sender,
-    "confirm"   => \$confirm,
-    "from" => \$sender,
-    "assume8bitencoding" => \$auto_8bit_encoding,
-    "composeencoding" => \$compose_encoding,
-    "transferencoding" => \$target_xfer_encoding,
+    "smtpserver" => [\$smtp_server],
+    "smtpserverport" => [\$smtp_server_port],
+    "smtpserveroption" => [\@smtp_server_options],
+    "smtpuser" => [\$smtp_authuser],
+    "smtppass" => [\$smtp_authpass],
+    "smtpdomain" => [\$smtp_domain],
+    "smtpauth" => [\$smtp_auth],
+    "smtpbatchsize" => [\$batch_size],
+    "smtprelogindelay" => [\$relogin_delay],
+    "to" => [\@initial_to],
+    "tocmd" => [\$to_cmd],
+    "cc" => [\@initial_cc],
+    "cccmd" => [\$cc_cmd],
+    "aliasfiletype" => [\$aliasfiletype],
+    "bcc" => [\@bcclist],
+    "suppresscc" => [\@suppress_cc],
+    "envelopesender" => [\$envelope_sender],
+    "confirm"   => [\$confirm],
+    "from" => [\$sender],
+    "assume8bitencoding" => [\$auto_8bit_encoding],
+    "composeencoding" => [\$compose_encoding],
+    "transferencoding" => [\$target_xfer_encoding],
 );
 
 my %config_path_settings = (
@@ -446,6 +446,13 @@  sub read_config {
 	${$setting->[0]} = $setting->[1] unless (defined (${$setting->[0]}));
 }
 
+# fall back to builtin defaults
+for my $setting (values %config_settings) {
+	if (@$setting == 2 && !defined (${$setting->[0]})) {
+		${$setting->[0]} = $setting->[1];
+	}
+}
+
 # 'default' encryption is none -- this only prevents a warning
 $smtp_encryption = '' unless (defined $smtp_encryption);