[PATCH v2 0/2] format-patch: make boolean and mention in diff-options.adoc

[kristofferhaugsbakk@fastmail.com]

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

See: https://lore.kernel.org/git/a9602602-5fea-40c0-a505-34133ed4d58c@app.fastmail.com/

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/CV_format.noprefix_boolean.39c@msgid.xyz/

§ 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/3124b359-2929-4f3f-9ac6-793277fe422b@jontes.page/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:
diff --git a/builtin/log.c b/builtin/log.c
index e56af7465ae..275122b807e 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -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;
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index 645ac402a19..c20091e36fe 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -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 <code@khaugsbakk.name>
     
      ## 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