threads / patch / 64221

patchdoc: patch-id: fix accidental literal blocks

Subject: [PATCH] doc: patch-id: fix accidental literal blocks

## tl;dr

7 messages between Sep 28, 2025 and Sep 29, 2025. Diffs are folded; open one to read it.

replies: 6people: 2as markdown or json

kristofferhaugsbakk@fastmail.com· Sep 28, 2025, 14:03 UTC · lore
From: Kristoffer Haugsbakk <code@khaugsbakk.name>

The options here are apparently written with the assumption that you can continue a block like e.g. a description list by maintaining the same indentation level. This is not the the case; you need to use list continuation (+) for the elements following the first paragraph if you want to be guaranteed a predictable output

This is kind of subtle since only the last paragraph of each option gets rendered in an unintended way, namely as literal blocks. This is easier to see in the HTML output since the man page output just has a slightly larger indentation on these paragraphs.

Let’s use list continuation throughout, collapse the indentation in front of all paragraphs except the first one, and wrap the unordered list in an open block (`--`).

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
 Documentation/git-patch-id.adoc | 43 ++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 20 deletions(-)
Show changes to Documentation/git-patch-id.adoc +23 −20
diff --git a/Documentation/git-patch-id.adoc b/Documentation/git-patch-id.adoc
index 1d15fa45d51..45da0f27acd 100644
--- a/Documentation/git-patch-id.adoc
+++ b/Documentation/git-patch-id.adoc
@@ -33,27 +33,30 @@ OPTIONS
 --verbatim::
 	Calculate the patch-id of the input as it is given, do not strip
 	any whitespace.
-
-	This is the default if patchid.verbatim is true.
++
+This is the default if patchid.verbatim is true.
 
 --stable::
 	Use a "stable" sum of hashes as the patch ID. With this option:
-	 - Reordering file diffs that make up a patch does not affect the ID.
-	   In particular, two patches produced by comparing the same two trees
-	   with two different settings for "-O<orderfile>" result in the same
-	   patch ID signature, thereby allowing the computed result to be used
-	   as a key to index some meta-information about the change between
-	   the two trees;
-
-	 - Result is different from the value produced by git 1.9 and older
-	   or produced when an "unstable" hash (see --unstable below) is
-	   configured - even when used on a diff output taken without any use
-	   of "-O<orderfile>", thereby making existing databases storing such
-	   "unstable" or historical patch-ids unusable.
-
-	 - All whitespace within the patch is ignored and does not affect the id.
-
-	This is the default if patchid.stable is set to true.
++
+--
+- Reordering file diffs that make up a patch does not affect the ID.
+  In particular, two patches produced by comparing the same two trees
+  with two different settings for "-O<orderfile>" result in the same
+  patch ID signature, thereby allowing the computed result to be used
+  as a key to index some meta-information about the change between
+  the two trees;
+
+- Result is different from the value produced by git 1.9 and older
+  or produced when an "unstable" hash (see --unstable below) is
+  configured - even when used on a diff output taken without any use
+  of "-O<orderfile>", thereby making existing databases storing such
+  "unstable" or historical patch-ids unusable.
+
+- All whitespace within the patch is ignored and does not affect the id.
+--
++
+This is the default if patchid.stable is set to true.
 
 --unstable::
 	Use an "unstable" hash as the patch ID. With this option,
@@ -61,8 +64,8 @@ OPTIONS
 	by git 1.9 and older and whitespace is ignored.  Users with pre-existing
 	databases storing patch-ids produced by git 1.9 and older (who do not deal
 	with reordered patches) may want to use this option.
-
-	This is the default.
++
+This is the default.
 
 GIT
 ---

base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0
-- 
2.51.0.327.gddebdc8c038
Junio C Hamano· Sep 28, 2025, 16:01 UTC · re: kristofferhaugsbakk@fastmail.com · lore

Re: [PATCH] doc: patch-id: fix accidental literal blocks

kristofferhaugsbakk@fastmail.com writes:
Show 12 quoted lines
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> The options here are apparently written with the assumption that you
> can continue a block like e.g. a description list by maintaining the
> same indentation level.  This is not the the case; you need to use list
> continuation (+) for the elements following the first paragraph if you
> want to be guaranteed a predictable output
>
> This is kind of subtle since only the last paragraph of each option gets
> rendered in an unintended way, namely as literal blocks.  This is easier
> to see in the HTML output since the man page output just has a slightly
> larger indentation on these paragraphs.

Hmph, I've always thought that the practice breaks every paragraph except for the first one (which is the same as "only the last", only when you have exactly 2 paragraphs; when you have more than 2, it makes a difference).

The changes in the diff part all look reasonable to me.
Thanks.
Kristoffer Haugsbakk· Sep 28, 2025, 19:25 UTC · re: Junio C Hamano · lore

Re: [PATCH] doc: patch-id: fix accidental literal blocks

On Sun, Sep 28, 2025, at 18:01, Junio C Hamano wrote:
Show 19 quoted lines
> kristofferhaugsbakk@fastmail.com writes:
>
>> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>>
>> The options here are apparently written with the assumption that you
>> can continue a block like e.g. a description list by maintaining the
>> same indentation level.  This is not the the case; you need to use list
>> continuation (+) for the elements following the first paragraph if you
>> want to be guaranteed a predictable output
>>
>> This is kind of subtle since only the last paragraph of each option gets
>> rendered in an unintended way, namely as literal blocks.  This is easier
>> to see in the HTML output since the man page output just has a slightly
>> larger indentation on these paragraphs.
>
> Hmph, I've always thought that the practice breaks every paragraph
> except for the first one (which is the same as "only the last",
> only when you have exactly 2 paragraphs; when you have more than 2,
> it makes a difference).

I don’t understand why but `doc-diff` agrees that these last ones are the only ones affected by this change.

Junio C Hamano· Sep 28, 2025, 19:37 UTC · re: Kristoffer Haugsbakk · lore

Re: [PATCH] doc: patch-id: fix accidental literal blocks

"Kristoffer Haugsbakk" <kristofferhaugsbakk@fastmail.com> writes:
Show 13 quoted lines
> ...
>>> This is kind of subtle since only the last paragraph of each option gets
>>> rendered in an unintended way, namely as literal blocks.  This is easier
>>> to see in the HTML output since the man page output just has a slightly
>>> larger indentation on these paragraphs.
>>
>> Hmph, I've always thought that the practice breaks every paragraph
>> except for the first one (which is the same as "only the last",
>> only when you have exactly 2 paragraphs; when you have more than 2,
>> it makes a difference).
>
> I don’t understand why but `doc-diff` agrees that these last ones are
> the only ones affected by this change.

But in this patch, you are not dealing with a section with three or more paragraphs, are you? I think there is nothing doc-diff can show you in this particular fix.

For example, the first hunk deals with
	--verbatim::
		Calculate the patch-id of the input as it is given, do not strip
		any whitespace.
		This is the default if patchid.verbatim is true.

two paragraphs, "Calculate...", and "This is the default...". It is correct that the second paragraph needs to be treated, like so:

	--verbatim::
		Calculate the patch-id of the input as it is given, do not strip
		any whitespace.
	+
	This is the default if patchid.verbatim is true.
and that is what your patch does.
But if you had three or more, e.g.,
	--verbatim::
		Calculate the patch-id of the input as it is given, do not strip
		any whitespace.
		This is the default if patchid.verbatim is true.
		For fun, we add a third paragraph.

would it render the first two paragraph correctly, and only the "For fun, ..." paragraph gets shown incorrectly? And would it give us a good rendition to fix only the last paragraph, like so:

	--verbatim::
		Calculate the patch-id of the input as it is given, do not strip
		any whitespace.
		This is the default if patchid.verbatim is true.
	+
	For fun, we add a third paragraph.

That is what your proposed log message said, and that is what I wanted to double check, as from memory and experience, I am reasonably sure that in such a case, I had to correct it more like

	--verbatim::
		Calculate the patch-id of the input as it is given, do not strip
		any whitespace.
	+
	This is the default if patchid.verbatim is true.
	+
	For fun, we add a third paragraph.

i.e., the second and subsequent paragraphs needed the "a + on a line byitself before the paragraph, and paragraph itself dedented" treatment.

Thanks.
kristofferhaugsbakk@fastmail.com· Sep 29, 2025, 20:47 UTC · re: kristofferhaugsbakk@fastmail.com · lore

[PATCH v2] doc: patch-id: fix accidental literal blocks

From: Kristoffer Haugsbakk <code@khaugsbakk.name>

All the final paragraphs on these three options are rendered as literal blocks. The intent was surely to keep each of them wed to their respective description list items. But the attempt at maintaining the indentation level of the block causes each them to be interpreted as a code block, since code blocks can be represented using indentation.

We need to use list continuation (+) in order to keep them wed to their blocks.

There is also an unordered list which sandwiches two paragraphs on an option. We don’t need to do anything about that since it attaches to the description list item without list continuation (i.e. it is already correct). But for consistency let’s use list continuation and an open block on it.

Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
---
Notes (series):
    v2:
    
    Rewrite the commit message.  Especially with an eye towards
    explaining (and not hedging) why and how things work the way they do; in
    particular that the unordered list is already fine and doesn’t need a
    list continuation.  But let’s go ahead and do it anyway since it doesn’t
    hurt and keeps things more consistent.
    
    Also describe the problem immediately instead of burying the lede/lead
    with “The options here are apparently written ...”.
    
    Also less “the the” mistakes.
 Documentation/git-patch-id.adoc | 43 ++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 20 deletions(-)
Show changes to Documentation/git-patch-id.adoc +23 −20
diff --git a/Documentation/git-patch-id.adoc b/Documentation/git-patch-id.adoc
index 1d15fa45d51..45da0f27acd 100644
--- a/Documentation/git-patch-id.adoc
+++ b/Documentation/git-patch-id.adoc
@@ -33,27 +33,30 @@ OPTIONS
 --verbatim::
 	Calculate the patch-id of the input as it is given, do not strip
 	any whitespace.
-
-	This is the default if patchid.verbatim is true.
++
+This is the default if patchid.verbatim is true.
 
 --stable::
 	Use a "stable" sum of hashes as the patch ID. With this option:
-	 - Reordering file diffs that make up a patch does not affect the ID.
-	   In particular, two patches produced by comparing the same two trees
-	   with two different settings for "-O<orderfile>" result in the same
-	   patch ID signature, thereby allowing the computed result to be used
-	   as a key to index some meta-information about the change between
-	   the two trees;
-
-	 - Result is different from the value produced by git 1.9 and older
-	   or produced when an "unstable" hash (see --unstable below) is
-	   configured - even when used on a diff output taken without any use
-	   of "-O<orderfile>", thereby making existing databases storing such
-	   "unstable" or historical patch-ids unusable.
-
-	 - All whitespace within the patch is ignored and does not affect the id.
-
-	This is the default if patchid.stable is set to true.
++
+--
+- Reordering file diffs that make up a patch does not affect the ID.
+  In particular, two patches produced by comparing the same two trees
+  with two different settings for "-O<orderfile>" result in the same
+  patch ID signature, thereby allowing the computed result to be used
+  as a key to index some meta-information about the change between
+  the two trees;
+
+- Result is different from the value produced by git 1.9 and older
+  or produced when an "unstable" hash (see --unstable below) is
+  configured - even when used on a diff output taken without any use
+  of "-O<orderfile>", thereby making existing databases storing such
+  "unstable" or historical patch-ids unusable.
+
+- All whitespace within the patch is ignored and does not affect the id.
+--
++
+This is the default if patchid.stable is set to true.
 
 --unstable::
 	Use an "unstable" hash as the patch ID. With this option,
@@ -61,8 +64,8 @@ OPTIONS
 	by git 1.9 and older and whitespace is ignored.  Users with pre-existing
 	databases storing patch-ids produced by git 1.9 and older (who do not deal
 	with reordered patches) may want to use this option.
-
-	This is the default.
++
+This is the default.
 
 GIT
 ---

Interdiff against v1:

Range-diff against v1:
1:  0520e8f9caf ! 1:  e5ad12cc3b3 doc: patch-id: fix accidental literal blocks
    @@ Metadata
      ## Commit message ##
         doc: patch-id: fix accidental literal blocks
     
    -    The options here are apparently written with the assumption that you
    -    can continue a block like e.g. a description list by maintaining the
    -    same indentation level.  This is not the the case; you need to use list
    -    continuation (+) for the elements following the first paragraph if you
    -    want to be guaranteed a predictable output
    +    All the final paragraphs on these three options are rendered as
    +    literal blocks. The intent was surely to keep each of them wed to their
    +    respective description list items. But the attempt at maintaining the
    +    indentation level of the block causes each them to be interpreted as a
    +    code block, since code blocks can be represented using indentation.
     
    -    This is kind of subtle since only the last paragraph of each option gets
    -    rendered in an unintended way, namely as literal blocks.  This is easier
    -    to see in the HTML output since the man page output just has a slightly
    -    larger indentation on these paragraphs.
    +    We need to use list continuation (+) in order to keep them wed to
    +    their blocks.
     
    -    Let’s use list continuation throughout, collapse the indentation in
    -    front of all paragraphs except the first one, and wrap the unordered
    -    list in an open block (`--`).
    +    There is also an unordered list which sandwiches two paragraphs on an
    +    option. We don’t need to do anything about that since it attaches to the
    +    description list item without list continuation (i.e. it is already
    +    correct). But for consistency let’s use list continuation and an open
    +    block on it.
     
         Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
     

base-commit: c44beea485f0f2feaf460e2ac87fdd5608d63cf0
-- 
2.51.0.327.gddebdc8c038
Junio C Hamano· Sep 29, 2025, 22:18 UTC · re: kristofferhaugsbakk@fastmail.com · lore

Re: [PATCH v2] doc: patch-id: fix accidental literal blocks

kristofferhaugsbakk@fastmail.com writes:
Show 19 quoted lines
> From: Kristoffer Haugsbakk <code@khaugsbakk.name>
>
> All the final paragraphs on these three options are rendered as
> literal blocks. The intent was surely to keep each of them wed to their
> respective description list items. But the attempt at maintaining the
> indentation level of the block causes each them to be interpreted as a
> code block, since code blocks can be represented using indentation.
>
> We need to use list continuation (+) in order to keep them wed to
> their blocks.
>
> There is also an unordered list which sandwiches two paragraphs on an
> option. We don’t need to do anything about that since it attaches to the
> description list item without list continuation (i.e. it is already
> correct). But for consistency let’s use list continuation and an open
> block on it.
>
> Signed-off-by: Kristoffer Haugsbakk <code@khaugsbakk.name>
> ---
Thanks.  Let me mark this for 'next'.

← back to recent threads