format-patch: make boolean and mention in diff-options.adoc

From: Kristoffer Haugsbakk <[email protected]>

See: https://lore.kernel.org/git/[email protected]/

Topic name (applied): kh/format-patch-noprefix-is-boolean

Topic summary: The config format.noprefix should act like a boolean. But now it is active no matter what the value is. Change it to act like a boolean like diff.noprefix. Also mention it instead of diff.noprefix in git-format-patch(1) doc.

§ Changes in v2

Got feedback from Junio.

Copied from note on Patch 1/2:

• Use advise() for the hint; you get the hint: prefix and color
• Rework the hint, or the advise, to say that any value *used to be* treated as true. That better helps people who maybe set nope when they meant, “no, I don’t want any prefix” (for example)

Also a commit message tweak on Patch 2/2.

§ Link to v1

https://lore.kernel.org/git/[email protected]/

§ Breaking change (unchanged since v1)

This is a breaking change. But I have opted to die if the config is not a boolean and just hint about the change. See also Peff’s comment on such a breaking change in that link.

I have also asked here about marking breaking changes:

https://lore.kernel.org/git/[email protected]/T/#ma8856238748d0794a0da6c64e1c0c8a4824b996f

Kristoffer Haugsbakk (2):
format-patch: make format.noprefix a boolean
doc: diff-options.adoc: show format.noprefix for format-patch

 Documentation/diff-options.adoc |  4 +++-
 builtin/log.c                   | 14 +++++++++++++-
 t/t4014-format-patch.sh         | 16 ++++++++++++++++
 3 files changed, 32 insertions(+), 2 deletions(-)

Interdiff against v1:

index e56af7465ae..275122b807e 100644
@@ -40,6 +40,7 @@
 #include "mailmap.h"
 #include "progress.h"
 #include "commit-slab.h"
+#include "advice.h"
 
 #include "commit-reach.h"
 #include "range-diff.h"

@@ -1101,11 +1102,11 @@ static int git_format_config(const char *var, const char *value,
 			int status = die_message(
 				_("bad boolean config value '%s' for '%s'"),
 				value, var);
-			fprintf(stderr,
-				_("hint: '%s' used to accept any value but "
-				  "now only\n"
-				  "hint: accepts boolean values, like '%s'\n"),
-				var, "diff.noprefix");
+			advise(_("'%s' used to accept any value and "
+				 "treat that as 'true'.\n"
+				 "Now it only accepts boolean values, "
+				 "like what '%s' does.\n"),
+			       var, "diff.noprefix");
 			exit(status);
 		}
 		return 0;

index 645ac402a19..c20091e36fe 100755
@@ -2555,8 +2555,8 @@ test_expect_success 'format-patch --default-prefix overrides format.noprefix' '
 test_expect_success 'errors on format.noprefix which is not boolean' '
 	cat >expect <<-EOF &&
 	fatal: bad boolean config value ${SQ}not-a-bool${SQ} for ${SQ}format.noprefix${SQ}
-	hint: ${SQ}format.noprefix${SQ} used to accept any value but now only
-	hint: accepts boolean values, like ${SQ}diff.noprefix${SQ}
+	hint: ${SQ}format.noprefix${SQ} used to accept any value and treat that as ${SQ}true${SQ}.
+	hint: Now it only accepts boolean values, like what ${SQ}diff.noprefix${SQ} does.
 	EOF
 	test_must_fail git -c format.noprefix=not-a-bool \
 		format-patch -1 --stdout 2>actual &&

Range-diff against v1:
1: 49fa161a392 ! 1: eee61561eb3 format-patch: make format.noprefix a boolean

    @@ Commit message
           format.noprefix* (current behavior) will now have the opposite
           experience. Which is not a reasonable setup.
     
    -    Let’s only offer a breaking change fig leaf by hinting about the
    +    Let’s only offer a breaking change fig leaf by advising about the
         previous behavior before dying.
     
         Signed-off-by: Kristoffer Haugsbakk <[email protected]>
     
      ## builtin/log.c ##
    +@@
    + #include "mailmap.h"
    + #include "progress.h"
    + #include "commit-slab.h"
    ++#include "advice.h"
    + 
    + #include "commit-reach.h"
    + #include "range-diff.h"
     @@ builtin/log.c: static int git_format_config(const char *var, const char *value,
      		return 0;
      	}
    @@ builtin/log.c: static int git_format_config(const char *var, const char *value,
     +			int status = die_message(
     +				_("bad boolean config value '%s' for '%s'"),
     +				value, var);
    -+			fprintf(stderr,
    -+				_("hint: '%s' used to accept any value but "
    -+				  "now only\n"
    -+				  "hint: accepts boolean values, like '%s'\n"),
    -+				var, "diff.noprefix");
    ++			advise(_("'%s' used to accept any value and "
    ++				 "treat that as 'true'.\n"
    ++				 "Now it only accepts boolean values, "
    ++				 "like what '%s' does.\n"),
    ++			       var, "diff.noprefix");
     +			exit(status);
     +		}
      		return 0;
    @@ t/t4014-format-patch.sh: test_expect_success 'format-patch respects format.nopre
     +test_expect_success 'errors on format.noprefix which is not boolean' '
     +	cat >expect <<-EOF &&
     +	fatal: bad boolean config value ${SQ}not-a-bool${SQ} for ${SQ}format.noprefix${SQ}
    -+	hint: ${SQ}format.noprefix${SQ} used to accept any value but now only
    -+	hint: accepts boolean values, like ${SQ}diff.noprefix${SQ}
    ++	hint: ${SQ}format.noprefix${SQ} used to accept any value and treat that as ${SQ}true${SQ}.
    ++	hint: Now it only accepts boolean values, like what ${SQ}diff.noprefix${SQ} does.
     +	EOF
     +	test_must_fail git -c format.noprefix=not-a-bool \
     +		format-patch -1 --stdout 2>actual &&

2: 0cef1915a9c ! 2: b9b583bd007 doc: diff-options.adoc: show format.noprefix for format-patch
@@ Commit message
git-format-patch(1) uses format.noprefix and ignores diff.noprefix.

The configuration variable format.prefix was added as an “escape
- hatch” because “it’s unlikely that anybody really wants format.
+ hatch”, and “it’s unlikely that anybody really wants format.
noprefix=true in the first place.”[1] Based on that there doesn’t seem to be a need to widely advertise this configuration variable.

base-commit: 67ad42147a7acc2af6074753ebd03d904476118f

2.53.0.26.g2afa8602a26