threads / patch / 59264

patchformat.attach: allow empty value to disable multi-part messages

Subject: [PATCH] format.attach: allow empty value to disable multi-part messages

## tl;dr

3 messages between Feb 17, 2023 and Feb 18, 2023. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Junio C Hamano· Feb 17, 2023, 23:33 UTC · lore

When a lower precedence configuration file (e.g. /etc/gitconfig) defines format.attach in any way, there was no way to disable it in a more specific configuration file (e.g. $HOME/.gitconfig).

Change the behaviour of setting it to an empty string. It used to mean that the result is still a multipart message with only dashes used as a multi-part separator, but now it resets the setting to the default (which would be to give an inline patch, unless other command line options are in effect).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 * This is a backward incompatible change, so we may not take it in the
   end.  We'll see.
 Documentation/config/format.txt |  3 ++-
 builtin/log.c                   |  2 ++
 t/t4014-format-patch.sh         | 18 ++++++++++++++++++
 3 files changed, 22 insertions(+), 1 deletion(-)
Show changes to 3 files +22 −1

Documentation/config/format.txt, builtin/log.c, t/t4014-format-patch.sh

diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
index 3bd78269e2..73678d88a1 100644
--- a/Documentation/config/format.txt
+++ b/Documentation/config/format.txt
@@ -3,7 +3,8 @@ format.attach::
 	'format-patch'.  The value can also be a double quoted string
 	which will enable attachments as the default and set the
 	value as the boundary.  See the --attach option in
-	linkgit:git-format-patch[1].
+	linkgit:git-format-patch[1].  To countermand an earlier
+	value, set it to an empty string.
 
 format.from::
 	Provides the default value for the `--from` option to format-patch.
diff --git a/builtin/log.c b/builtin/log.c
index 04412dd9c9..a70fba198f 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1007,6 +1007,8 @@ static int git_format_config(const char *var, const char *value, void *cb)
 	if (!strcmp(var, "format.attach")) {
 		if (value && *value)
 			default_attach = xstrdup(value);
+		else if (value && !*value)
+			FREE_AND_NULL(default_attach);
 		else
 			default_attach = xstrdup(git_version_string);
 		return 0;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 012f155e10..f3313b8c58 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -2281,6 +2281,24 @@ test_expect_success 'format-patch --attach cover-letter only is non-multipart' '
 	test_line_count = 1 output
 '
 
+test_expect_success 'format-patch with format.attach' '
+	test_when_finished "rm -fr patches" &&
+	separator=attachment-separator &&
+	test_config format.attach "$separator" &&
+	filename=$(git format-patch -o patches -1) &&
+	grep "^Content-Type: multipart/.*$separator" "$filename"
+'
+
+test_expect_success 'format-patch with format.attach=disabled' '
+	test_when_finished "rm -fr patches" &&
+	separator=attachment-separator &&
+	test_config_global format.attach "$separator" &&
+	test_config format.attach "" &&
+	filename=$(git format-patch -o patches -1) &&
+	# The output should not even declare content type for text/plain.
+	! grep "^Content-Type: multipart/" "$filename"
+'
+
 test_expect_success '-c format.mboxrd format-patch' '
 	sp=" " &&
 	cat >msg <<-INPUT_END &&
-- 
2.39.2-501-gd9d677b2d8
Brian Inglis· Feb 18, 2023, 17:11 UTC · re: Junio C Hamano · lore

Re: [PATCH] format.attach: allow empty value to disable multi-part messages

Thanks Junio,
I'll hope and watch for it to get added, and test if/when released.
-- 
Take care. Thanks, Brian Inglis			Calgary, Alberta, Canada

La perfection est atteinte			Perfection is achieved
non pas lorsqu'il n'y a plus rien à ajouter	not when there is no more to add
mais lorsqu'il n'y a plus rien à retirer	but when there is no more to cut
			-- Antoine de Saint-Exupéry


On 2023-02-17 16:33, Junio C Hamano wrote:
> When a lower precedence configuration file (e.g. /etc/gitconfig)
> defines format.attach in any way, there was no way to disable it in
> a more specific configuration file (e.g. $HOME/.gitconfig).
> 
> Change the behaviour of setting it to an empty string.  It used to
> mean that the result is still a multipart message with only dashes
> used as a multi-part separator, but now it resets the setting to
> the default (which would be to give an inline patch, unless other
> command line options are in effect).
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>   * This is a backward incompatible change, so we may not take it in the
>     end.  We'll see.
> 
>   Documentation/config/format.txt |  3 ++-
>   builtin/log.c                   |  2 ++
>   t/t4014-format-patch.sh         | 18 ++++++++++++++++++
>   3 files changed, 22 insertions(+), 1 deletion(-)
> 
> diff --git a/Documentation/config/format.txt b/Documentation/config/format.txt
> index 3bd78269e2..73678d88a1 100644
> --- a/Documentation/config/format.txt
> +++ b/Documentation/config/format.txt
> @@ -3,7 +3,8 @@ format.attach::
>   	'format-patch'.  The value can also be a double quoted string
>   	which will enable attachments as the default and set the
>   	value as the boundary.  See the --attach option in
> -	linkgit:git-format-patch[1].
> +	linkgit:git-format-patch[1].  To countermand an earlier
> +	value, set it to an empty string.
>   
>   format.from::
>   	Provides the default value for the `--from` option to format-patch.
> diff --git a/builtin/log.c b/builtin/log.c
> index 04412dd9c9..a70fba198f 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1007,6 +1007,8 @@ static int git_format_config(const char *var, const char *value, void *cb)
>   	if (!strcmp(var, "format.attach")) {
>   		if (value && *value)
>   			default_attach = xstrdup(value);
> +		else if (value && !*value)
> +			FREE_AND_NULL(default_attach);
>   		else
>   			default_attach = xstrdup(git_version_string);
>   		return 0;
> diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
> index 012f155e10..f3313b8c58 100755
> --- a/t/t4014-format-patch.sh
> +++ b/t/t4014-format-patch.sh
> @@ -2281,6 +2281,24 @@ test_expect_success 'format-patch --attach cover-letter only is non-multipart' '
>   	test_line_count = 1 output
>   '
>   
> +test_expect_success 'format-patch with format.attach' '
> +	test_when_finished "rm -fr patches" &&
> +	separator=attachment-separator &&
> +	test_config format.attach "$separator" &&
> +	filename=$(git format-patch -o patches -1) &&
> +	grep "^Content-Type: multipart/.*$separator" "$filename"
> +'
> +
> +test_expect_success 'format-patch with format.attach=disabled' '
> +	test_when_finished "rm -fr patches" &&
> +	separator=attachment-separator &&
> +	test_config_global format.attach "$separator" &&
> +	test_config format.attach "" &&
> +	filename=$(git format-patch -o patches -1) &&
> +	# The output should not even declare content type for text/plain.
> +	! grep "^Content-Type: multipart/" "$filename"
> +'
> +
>   test_expect_success '-c format.mboxrd format-patch' '
>   	sp=" " &&
>   	cat >msg <<-INPUT_END &&
Junio C Hamano· Feb 18, 2023, 17:34 UTC · re: Brian Inglis · lore

Re: [PATCH] format.attach: allow empty value to disable multi-part messages

Brian Inglis <Brian.Inglis@Shaw.ca> writes:
> Thanks Junio,
>
> I'll hope and watch for it to get added, and test if/when released.

That won't be very helpful as reviewing and testing is to prevent nonsense patches to be added to the released version. Once it is released, testing becomes much less valuable, as it is too late to catch nonsense bugs in the added code.

Thanks.

← back to recent threads