Re: [RFC][PATCH] send-email: add --[no-]xmailer option

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 3, 2014, 17:08 UTC
Message-ID: <xmqqfvcwk6en.fsf@gitster.dls.corp.google.com>
In-Reply-To: <20140324213814.GA1267@achilles.my.domain>

Luis Henriques <henrix@camandro.org> writes:

> +--xmailer::
> +	Prevent adding the "X-Mailer:" header.  Default value is
> +	'sendemail.xmailer'.

Two problems here.

 - "git send-email --xmailer" would _ADD_, not prevent adding, the
   header, regardless of the value of sendemail.xmailer.

 - It is unspecified what happens when you do not have
   sendemail.xmailer and do not give the --xmailer option.

Perhaps

--xmailer:: --no-xmailer::

	By default, `git send-email` adds an "X-Mailer:" header to
        the message to identify the version of itself.  The
        `--no-xmailer` option can be used to turn this off (setting
        the `sendemail.xmailer` configuration to false has the same
        effect).  The `--xmailer` option from the command line is
        useful to countermand `sendemail.xmailer` that is set to
        `false`.

or something?

Show 9 quoted lines

> diff --git a/git-send-email.perl b/git-send-email.perl
> index fdb0029..8789124 100755
> --- a/git-send-email.perl
> +++ b/git-send-email.perl
> @@ -54,6 +54,7 @@ git send-email [options] <file | directory | rev-list options >
>      --[no-]bcc              <str>  * Email Bcc:
>      --subject               <str>  * Email "Subject:"
>      --in-reply-to           <str>  * Email "In-Reply-To:"
> +    --[no-]xmailer                 * Don't add "X-Mailer:" header.  Default on.

The same confusion exists here. "Don't ... default on" hints that by default we won't see "X-mailer:" which is not true.

A way to avoid confusion is to describe what the option is about for the positive variant, just like it is done for "--[no-]annotate" option below.

Show 40 quoted lines

> @@ -174,6 +175,9 @@ my $force = 0;
>  my $multiedit;
>  my $editor;
>  
> +# Usage of X-Mailer email header
> +my $xmailer;
> +
>  sub do_edit {
>  	if (!defined($editor)) {
>  		$editor = Git::command_oneline('var', 'GIT_EDITOR');
> @@ -214,7 +218,8 @@ my %config_bool_settings = (
>      "signedoffcc" => [\$signed_off_by_cc, undef],      # Deprecated
>      "validate" => [\$validate, 1],
>      "multiedit" => [\$multiedit, undef],
> -    "annotate" => [\$annotate, undef]
> +    "annotate" => [\$annotate, undef],
> +    "xmailer" => [\$xmailer, 1]
>  );
>  
>  my %config_settings = (
> @@ -311,6 +316,7 @@ my $rc = GetOptions("h" => \$help,
>  		    "8bit-encoding=s" => \$auto_8bit_encoding,
>  		    "compose-encoding=s" => \$compose_encoding,
>  		    "force" => \$force,
> +		    "xmailer!" => \$xmailer,
>  	 );
>  
>  usage() if $help;
> @@ -1144,8 +1150,10 @@ To: $to${ccline}
>  Subject: $subject
>  Date: $date
>  Message-Id: $message_id
> -X-Mailer: git-send-email $gitversion
>  ";
> +	if ($xmailer) {
> +		$header .= "X-Mailer: git-send-email $gitversion\n";
> +	}
>  	if ($reply_to) {
>  
>  		$header .= "In-Reply-To: $reply_to\n";

tests?

Thanks.