Re: [PATCH v6 1/1] send-email: retrieve Message-ID from outlook SMTP server

[Junio C Hamano <gitster@pobox.com>]

Aditya Garg <gargaditya08@live.com> writes:

> I tested this patch and it works well. I've added some docs as
> well and it now looks like the one below.
>
> --->8---
> From e06ccabb5a0cef100e50e2b9d6d3c0a1769bda59 Mon Sep 17 00:00:00 2001
> From: Aditya Garg <gargaditya08@live.com>
> Date: Sat, 26 Apr 2025 08:25:25 +0000
> Subject: [PATCH] send-email: add --[no-]smtp-outlook-id-tweak option
>
> From: Junio C Hamano <gitster@pobox.com>
>
> Add an option to allow users to specifically enable or disable
> retrieving the Message-ID from the Outlook SMTP server. This can be
> for other hosts mimicking the behaviour of Outlook, or for users who set
> a custom domain to be a CNAME for the Outlook SMTP server.
>
> Co-authored-by: Aditya Garg <gargaditya08@live.com>
> Signed-off-by: Aditya Garg <gargaditya08@live.com>
> ---

Take the authorship yourself.  My involvement is at most Helped-by:
level, I would think.

> @@ -290,6 +292,7 @@ sub do_edit {

>  my $mailmap = 0;
>  my $target_xfer_encoding = 'auto';
>  my $forbid_sendmail_variables = 1;
> +my $outlook_id_tweak = -1;

These lines we see around here are all in a section of variable
enumeration titled:

    # Variables with corresponding config settings & hardcoded defaults

I think this "-1" deserves a bit of an explanation.  It is neither
true or false (and Perl's "is this true?" check on that particular
value would say "true", but that is not how we want it to be taken
and we special case -1 ourselves in the code).

Alternatively perhaps we could initialize it to 'auto' (without any
extra comment here) and then ... 

>  sub is_outlook {
>  	my ($host) = @_;
> -	return ($host eq 'smtp.office365.com' || $host eq 'smtp-mail.outlook.com');
> +	if ($outlook_id_tweak < 0) {

... change this to "eq 'auto'"?  Then the value would be
self-evident.

> +		$outlook_id_tweak =
> +			($host eq 'smtp.office365.com' ||
> +			 $host eq 'smtp-mail.outlook.com') ? 1 : 0;
> +	}	return $outlook_id_tweak;
>  }

Somebody in the near-by thread mentioned that we could enable it
always (and if we do not find a replaced message-id where Outlook
may place one, we keep the original message-id we assigned), but I
personally think it is a poor design taste.  We do not know what a
random SMTP server implementation would do in that response, and all
we examined with any care during this discussion is how an Outlook
server responds.  Once we find a server that gives a random string
there that is not the replacement message-id at all, we would need a
separate knob to opt out of the feature---so let's not go there.

Thanks.