git-send-email: bug with sendemail.multiedit

git-send-email: bug with sendemail.multiedit

[Jean-Francois Dagenais]

Bonjour Pierre! ... and all git developers!

I think there is a bug with git-send-email.perl's evaluation of the sendemail.multiedit config variable.

I was only able to make the "do_edit()" function detect it as false by setting the variable to "0" instead
of "false", like so:

git config --global sendemail.multiedit 0

otherwise do_edit evaluates it as true and invokes the editor with all files as argument.

All other git config boolean variables are set to either "true" or "false", not "0" or "1".

Not being too familiar with the perl language, I don't know how to fix this without spending an hour, which
is probably the amount of time I already spent narrowing the problem down already. So I leave this into
more capable hands.

cheers.

Re: git-send-email: bug with sendemail.multiedit

[Jeff King]

On Mon, Jan 09, 2012 at 02:09:30PM -0500, Jean-Francois Dagenais wrote:

> I think there is a bug with git-send-email.perl's evaluation of the
> sendemail.multiedit config variable.
> 
> I was only able to make the "do_edit()" function detect it as false by
> setting the variable to "0" instead of "false", like so:

I think it's this:

-- >8 --
Subject: [PATCH] send-email: multiedit is a boolean config option

The sendemail.multiedit variable is meant to be a boolean.
However, it is not marked as such in the code, which means
we store its value literally. Thus in the do_edit function,
perl ends up coercing it to a boolean value according to
perl rules, not git rules. This works for "0", but "false",
"no", or "off" will erroneously be interpreted as true.

Signed-off-by: Jeff King <peff@peff.net>
---
 git-send-email.perl |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/git-send-email.perl b/git-send-email.perl
index d491db9..ef30c55 100755
--- a/git-send-email.perl
+++ b/git-send-email.perl
@@ -210,6 +210,7 @@ my %config_bool_settings = (
     "signedoffbycc" => [\$signed_off_by_cc, undef],
     "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
     "validate" => [\$validate, 1],
+    "multiedit" => [\$multiedit, undef]
 );
 
 my %config_settings = (
@@ -227,7 +228,6 @@ my %config_settings = (
     "bcc" => \@bcclist,
     "suppresscc" => \@suppress_cc,
     "envelopesender" => \$envelope_sender,
-    "multiedit" => \$multiedit,
     "confirm"   => \$confirm,
     "from" => \$sender,
     "assume8bitencoding" => \$auto_8bit_encoding,
-- 
1.7.8

Re: git-send-email: bug with sendemail.multiedit

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Mon, Jan 09, 2012 at 02:09:30PM -0500, Jean-Francois Dagenais wrote:
>
>> I think there is a bug with git-send-email.perl's evaluation of the
>> sendemail.multiedit config variable.
>> 
>> I was only able to make the "do_edit()" function detect it as false by
>> setting the variable to "0" instead of "false", like so:
>
> I think it's this:
>
> -- >8 --
> Subject: [PATCH] send-email: multiedit is a boolean config option
>
> The sendemail.multiedit variable is meant to be a boolean.
> However, it is not marked as such in the code, which means
> we store its value literally. Thus in the do_edit function,
> perl ends up coercing it to a boolean value according to
> perl rules, not git rules. This works for "0", but "false",
> "no", or "off" will erroneously be interpreted as true.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---

Looks like it.

And in case anybody is wondering by looking at the context, confirm is not
a boolean "do you want a confirmation or not?", so it can stay where it is.

Thanks.

>  git-send-email.perl |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/git-send-email.perl b/git-send-email.perl
> index d491db9..ef30c55 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -210,6 +210,7 @@ my %config_bool_settings = (
>      "signedoffbycc" => [\$signed_off_by_cc, undef],
>      "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
>      "validate" => [\$validate, 1],
> +    "multiedit" => [\$multiedit, undef]
>  );
>  
>  my %config_settings = (
> @@ -227,7 +228,6 @@ my %config_settings = (
>      "bcc" => \@bcclist,
>      "suppresscc" => \@suppress_cc,
>      "envelopesender" => \$envelope_sender,
> -    "multiedit" => \$multiedit,
>      "confirm"   => \$confirm,
>      "from" => \$sender,
>      "assume8bitencoding" => \$auto_8bit_encoding,

Re: git-send-email: bug with sendemail.multiedit

[Thomas Rast]

Jean-Francois Dagenais <jeff.dagenais@gmail.com> writes:

> Bonjour Pierre! ... and all git developers!
>
> I think there is a bug with git-send-email.perl's evaluation of the sendemail.multiedit config variable.
>
> I was only able to make the "do_edit()" function detect it as false by setting the variable to "0" instead
> of "false", like so:
>
> git config --global sendemail.multiedit 0
>
> otherwise do_edit evaluates it as true and invokes the editor with all files as argument.

The patch below looks like the obvious fix.  Perhaps you can test it?

diff --git i/git-send-email.perl w/git-send-email.perl
index d491db9..7ac0a7d 100755
--- i/git-send-email.perl
+++ w/git-send-email.perl
@@ -210,6 +210,7 @@ sub do_edit {
     "signedoffbycc" => [\$signed_off_by_cc, undef],
     "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
     "validate" => [\$validate, 1],
+    "multiedit" => [\$multiedit, undef],
 );
 
 my %config_settings = (
@@ -227,7 +228,6 @@ sub do_edit {
     "bcc" => \@bcclist,
     "suppresscc" => \@suppress_cc,
     "envelopesender" => \$envelope_sender,
-    "multiedit" => \$multiedit,
     "confirm"   => \$confirm,
     "from" => \$sender,
     "assume8bitencoding" => \$auto_8bit_encoding,

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: git-send-email: bug with sendemail.multiedit

[Jean-Francois Dagenais]

Tested with 0/1/true/false, all works as expected, i.e. fix is backward compatible.
git version 1.7.8.2

On Jan 9, 2012, at 17:55, Jeff King wrote:

> On Mon, Jan 09, 2012 at 02:09:30PM -0500, Jean-Francois Dagenais wrote:
> 
>> I think there is a bug with git-send-email.perl's evaluation of the
>> sendemail.multiedit config variable.
>> 
>> I was only able to make the "do_edit()" function detect it as false by
>> setting the variable to "0" instead of "false", like so:
> 
> I think it's this:
> 
> -- >8 --
> Subject: [PATCH] send-email: multiedit is a boolean config option
> 
> The sendemail.multiedit variable is meant to be a boolean.
> However, it is not marked as such in the code, which means
> we store its value literally. Thus in the do_edit function,
> perl ends up coercing it to a boolean value according to
> perl rules, not git rules. This works for "0", but "false",
> "no", or "off" will erroneously be interpreted as true.
> 
> Signed-off-by: Jeff King <peff@peff.net>
Tested-by: Jean-François Dagenais <jeff.dagenais@gmail.com>
> ---
> git-send-email.perl |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/git-send-email.perl b/git-send-email.perl
> index d491db9..ef30c55 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -210,6 +210,7 @@ my %config_bool_settings = (
>     "signedoffbycc" => [\$signed_off_by_cc, undef],
>     "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
>     "validate" => [\$validate, 1],
> +    "multiedit" => [\$multiedit, undef]
> );
> 
> my %config_settings = (
> @@ -227,7 +228,6 @@ my %config_settings = (
>     "bcc" => \@bcclist,
>     "suppresscc" => \@suppress_cc,
>     "envelopesender" => \$envelope_sender,
> -    "multiedit" => \$multiedit,
>     "confirm"   => \$confirm,
>     "from" => \$sender,
>     "assume8bitencoding" => \$auto_8bit_encoding,
> -- 
> 1.7.8
>