{"thread":{"id":"64396","subject":"[PATCH] whitespace: correct bit assignment comments","startedAt":"2025-10-27T20:36:48Z","lastAt":"2025-10-29T13:21:42Z","messageCount":6,"participants":["Junio C Hamano","Patrick Steinhardt"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"529757","messageId":"xmqqfrb4hyjl.fsf@gitster.g","threadId":"64396","inReplyTo":null,"subject":"[PATCH] whitespace: correct bit assignment comments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-27T20:36:46Z","receivedAt":"2025-10-27T20:36:48Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A comment in diff.c claimed that bits up to 12th (counting from 0th)\nare whitespace rules, and 13th thru 15th are for new/old/context,\nbut it turns out it was miscounting.  Correct the comment.\n\nAlso update the way these bit constants are defined to use (1\nshifted by count) notation, instead of octal constants, as it tends\nto make it easier to notice a breakage like this.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * I happened to be looking at this code today, and noticed this,\n   but I very often make stupid mistakes while counting things, so\n   sending it out (which should not change any behaviour) in the\n   hope that somebody would (again) notice that I cannot count.\n\n   Next step would be to make a gap between whitespace rules bits\n   and WSEH_NEW/CONTEXT/OLD bits, so that we can smoothly add new\n   whitespace rules later.\n\n diff.c |  4 ++--\n diff.h |  6 +++---\n ws.h   | 20 ++++++++++----------\n 3 files changed, 15 insertions(+), 15 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a74e701806..08136ad5c5 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -803,8 +803,8 @@ enum diff_symbol {\n };\n /*\n  * Flags for content lines:\n- * 0..12 are whitespace rules\n- * 13-15 are WSEH_NEW | WSEH_OLD | WSEH_CONTEXT\n+ * 0..11 are whitespace rules\n+ * 12-14 are WSEH_NEW | WSEH_CONTEXT | WSEH_OLD\n  * 16 is marking if the line is blank at EOF\n  */\n #define DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF\t(1<<16)\ndiff --git a/diff.h b/diff.h\nindex 2fa256c3ef..60749154e7 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -331,9 +331,9 @@ struct diff_options {\n \n \tint ita_invisible_in_index;\n /* white-space error highlighting */\n-#define WSEH_NEW (1<<12)\n-#define WSEH_CONTEXT (1<<13)\n-#define WSEH_OLD (1<<14)\n+#define WSEH_NEW\t(1<<12)\n+#define WSEH_CONTEXT\t(1<<13)\n+#define WSEH_OLD\t(1<<14)\n \tunsigned ws_error_highlight;\n \tconst char *prefix;\n \tint prefix_length;\ndiff --git a/ws.h b/ws.h\nindex 5ba676c559..611c6b6d50 100644\n--- a/ws.h\n+++ b/ws.h\n@@ -7,19 +7,19 @@ struct strbuf;\n /*\n  * whitespace rules.\n  * used by both diff and apply\n- * last two digits are tab width\n+ * last two octal-digits are tab width (we support only up to 63).\n  */\n-#define WS_BLANK_AT_EOL         0100\n-#define WS_SPACE_BEFORE_TAB     0200\n-#define WS_INDENT_WITH_NON_TAB  0400\n-#define WS_CR_AT_EOL           01000\n-#define WS_BLANK_AT_EOF        02000\n-#define WS_TAB_IN_INDENT       04000\n-#define WS_TRAILING_SPACE      (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)\n+#define WS_BLANK_AT_EOL         (1<<6)\n+#define WS_SPACE_BEFORE_TAB     (1<<7)\n+#define WS_INDENT_WITH_NON_TAB  (1<<8)\n+#define WS_CR_AT_EOL            (1<<9)\n+#define WS_BLANK_AT_EOF         (1<<10)\n+#define WS_TAB_IN_INDENT        (1<<11)\n+#define WS_TRAILING_SPACE       (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)\n #define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|8)\n-#define WS_TAB_WIDTH_MASK        077\n+#define WS_TAB_WIDTH_MASK       ((1<<6)-1)\n /* All WS_* -- when extended, adapt diff.c emit_symbol */\n-#define WS_RULE_MASK           07777\n+#define WS_RULE_MASK            ((1<<12)-1)\n extern unsigned whitespace_rule_cfg;\n unsigned whitespace_rule(struct index_state *, const char *);\n unsigned parse_whitespace_rule(const char *);\n-- \n2.51.2-686-gf92ff40d17\n\n"},{"id":"529762","messageId":"aQBlCCDWMhLX_aBa@pks.im","threadId":"64396","inReplyTo":"xmqqfrb4hyjl.fsf@gitster.g","subject":"Re: [PATCH] whitespace: correct bit assignment comments","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-28T06:39:04Z","receivedAt":"2025-10-28T06:39:11Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Mon, Oct 27, 2025 at 01:36:46PM -0700, Junio C Hamano wrote:\n> diff --git a/diff.h b/diff.h\n> index 2fa256c3ef..60749154e7 100644\n> --- a/diff.h\n> +++ b/diff.h\n> @@ -331,9 +331,9 @@ struct diff_options {\n>  \n>  \tint ita_invisible_in_index;\n>  /* white-space error highlighting */\n> -#define WSEH_NEW (1<<12)\n> -#define WSEH_CONTEXT (1<<13)\n> -#define WSEH_OLD (1<<14)\n> +#define WSEH_NEW\t(1<<12)\n> +#define WSEH_CONTEXT\t(1<<13)\n> +#define WSEH_OLD\t(1<<14)\n>  \tunsigned ws_error_highlight;\n>  \tconst char *prefix;\n>  \tint prefix_length;\n\nHere you're using tabs for indentation, whereas below you use spaces. We\nshould probably be consistent.\n\n> diff --git a/ws.h b/ws.h\n> index 5ba676c559..611c6b6d50 100644\n> --- a/ws.h\n> +++ b/ws.h\n> @@ -7,19 +7,19 @@ struct strbuf;\n>  /*\n>   * whitespace rules.\n>   * used by both diff and apply\n> - * last two digits are tab width\n> + * last two octal-digits are tab width (we support only up to 63).\n>   */\n> -#define WS_BLANK_AT_EOL         0100\n> -#define WS_SPACE_BEFORE_TAB     0200\n> -#define WS_INDENT_WITH_NON_TAB  0400\n> -#define WS_CR_AT_EOL           01000\n> -#define WS_BLANK_AT_EOF        02000\n> -#define WS_TAB_IN_INDENT       04000\n> -#define WS_TRAILING_SPACE      (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)\n> +#define WS_BLANK_AT_EOL         (1<<6)\n> +#define WS_SPACE_BEFORE_TAB     (1<<7)\n> +#define WS_INDENT_WITH_NON_TAB  (1<<8)\n> +#define WS_CR_AT_EOL            (1<<9)\n> +#define WS_BLANK_AT_EOF         (1<<10)\n> +#define WS_TAB_IN_INDENT        (1<<11)\n> +#define WS_TRAILING_SPACE       (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)\n>  #define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|8)\n\nThe \"8\" here is a bit curious, but this matches what the comment says:\nthe last two digits are the tab width, and there of course is no macro\nfor that.\n\n> -#define WS_TAB_WIDTH_MASK        077\n> +#define WS_TAB_WIDTH_MASK       ((1<<6)-1)\n>  /* All WS_* -- when extended, adapt diff.c emit_symbol */\n> -#define WS_RULE_MASK           07777\n> +#define WS_RULE_MASK            ((1<<12)-1)\n>  extern unsigned whitespace_rule_cfg;\n>  unsigned whitespace_rule(struct index_state *, const char *);\n>  unsigned parse_whitespace_rule(const char *);\n\nAll of these conversion look correct to me, and I agree that this is\neasier to read.\n\nThanks!\n\nPatrick\n"},{"id":"529790","messageId":"xmqq4irjf8mc.fsf@gitster.g","threadId":"64396","inReplyTo":"aQBlCCDWMhLX_aBa@pks.im","subject":"Re: [PATCH] whitespace: correct bit assignment comments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-28T13:39:39Z","receivedAt":"2025-10-28T13:39:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Mon, Oct 27, 2025 at 01:36:46PM -0700, Junio C Hamano wrote:\n>> diff --git a/diff.h b/diff.h\n>> index 2fa256c3ef..60749154e7 100644\n>> --- a/diff.h\n>> +++ b/diff.h\n>> @@ -331,9 +331,9 @@ struct diff_options {\n>>  \n>>  \tint ita_invisible_in_index;\n>>  /* white-space error highlighting */\n>> -#define WSEH_NEW (1<<12)\n>> -#define WSEH_CONTEXT (1<<13)\n>> -#define WSEH_OLD (1<<14)\n>> +#define WSEH_NEW\t(1<<12)\n>> +#define WSEH_CONTEXT\t(1<<13)\n>> +#define WSEH_OLD\t(1<<14)\n>>  \tunsigned ws_error_highlight;\n>>  \tconst char *prefix;\n>>  \tint prefix_length;\n>\n> Here you're using tabs for indentation, whereas below you use spaces. We\n> should probably be consistent.\n\nThanks for sharp eyes.\n\n>> +#define WS_TAB_IN_INDENT        (1<<11)\n>> +#define WS_TRAILING_SPACE       (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)\n>>  #define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|8)\n>\n> The \"8\" here is a bit curious, but this matches what the comment says:\n> the last two digits are the tab width, and there of course is no macro\n> for that.\n\nYeah, we may need to do something about it later if we further touch\nthe code around here.\n\n> All of these conversion look correct to me, and I agree that this is\n> easier to read.\n\nThanks.\n"},{"id":"529798","messageId":"xmqq8qgvdm0y.fsf_-_@gitster.g","threadId":"64396","inReplyTo":"xmqq4irjf8mc.fsf@gitster.g","subject":"[PATCH v2] whitespace: correct bit assignment comments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-28T16:33:01Z","receivedAt":"2025-10-28T16:33:04Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"A comment in diff.c claimed that bits up to 12th (counting from 0th)\nare whitespace rules, and 13th thru 15th are for new/old/context,\nbut it turns out it was miscounting.  Correct them, and clarify\nwhere the whitespace rule bits come from in the comment.  Extend bit\nassignment comments to cover bits used for color-moved, which\nweren't described.\n\nAlso update the way these bit constants are defined to use (1 << N)\nnotation, instead of octal constants, as it tends to make it easier\nto notice a breakage like this.\n\nSprinkle a few blank lines between logically distinct groups of CPP\nmacro definitions to make them easier to read.\n\nSigned-off-by: Junio C Hamano <gitster@pobox.com>\n---\n\n * The next step would be to reserve the low 16 bits for ws.h and\n   then shift everything else up; we'd then have 4 extra bits to\n   play with, and I'll take one for \\No newline at the end of file.\n\n diff.c |  7 +++++--\n diff.h |  6 +++---\n ws.h   | 25 ++++++++++++++-----------\n 3 files changed, 22 insertions(+), 16 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex a74e701806..74261b332a 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -801,16 +801,19 @@ enum diff_symbol {\n \tDIFF_SYMBOL_CONTEXT_MARKER,\n \tDIFF_SYMBOL_SEPARATOR\n };\n+\n /*\n  * Flags for content lines:\n- * 0..12 are whitespace rules\n- * 13-15 are WSEH_NEW | WSEH_OLD | WSEH_CONTEXT\n+ * 0..11 are whitespace rules (see ws.h)\n+ * 12..14 are WSEH_NEW | WSEH_CONTEXT | WSEH_OLD\n  * 16 is marking if the line is blank at EOF\n+ * 17..19 are used for color-moved.\n  */\n #define DIFF_SYMBOL_CONTENT_BLANK_LINE_EOF\t(1<<16)\n #define DIFF_SYMBOL_MOVED_LINE\t\t\t(1<<17)\n #define DIFF_SYMBOL_MOVED_LINE_ALT\t\t(1<<18)\n #define DIFF_SYMBOL_MOVED_LINE_UNINTERESTING\t(1<<19)\n+\n #define DIFF_SYMBOL_CONTENT_WS_MASK (WSEH_NEW | WSEH_OLD | WSEH_CONTEXT | WS_RULE_MASK)\n \n /*\ndiff --git a/diff.h b/diff.h\nindex 2fa256c3ef..cbd355cf50 100644\n--- a/diff.h\n+++ b/diff.h\n@@ -331,9 +331,9 @@ struct diff_options {\n \n \tint ita_invisible_in_index;\n /* white-space error highlighting */\n-#define WSEH_NEW (1<<12)\n-#define WSEH_CONTEXT (1<<13)\n-#define WSEH_OLD (1<<14)\n+#define WSEH_NEW        (1<<12)\n+#define WSEH_CONTEXT    (1<<13)\n+#define WSEH_OLD        (1<<14)\n \tunsigned ws_error_highlight;\n \tconst char *prefix;\n \tint prefix_length;\ndiff --git a/ws.h b/ws.h\nindex 5ba676c559..23708efb73 100644\n--- a/ws.h\n+++ b/ws.h\n@@ -7,19 +7,22 @@ struct strbuf;\n /*\n  * whitespace rules.\n  * used by both diff and apply\n- * last two digits are tab width\n+ * last two octal-digits are tab width (we support only up to 63).\n  */\n-#define WS_BLANK_AT_EOL         0100\n-#define WS_SPACE_BEFORE_TAB     0200\n-#define WS_INDENT_WITH_NON_TAB  0400\n-#define WS_CR_AT_EOL           01000\n-#define WS_BLANK_AT_EOF        02000\n-#define WS_TAB_IN_INDENT       04000\n-#define WS_TRAILING_SPACE      (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)\n+#define WS_BLANK_AT_EOL         (1<<6)\n+#define WS_SPACE_BEFORE_TAB     (1<<7)\n+#define WS_INDENT_WITH_NON_TAB  (1<<8)\n+#define WS_CR_AT_EOL            (1<<9)\n+#define WS_BLANK_AT_EOF         (1<<10)\n+#define WS_TAB_IN_INDENT        (1<<11)\n+\n+#define WS_TRAILING_SPACE       (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF)\n #define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|8)\n-#define WS_TAB_WIDTH_MASK        077\n-/* All WS_* -- when extended, adapt diff.c emit_symbol */\n-#define WS_RULE_MASK           07777\n+#define WS_TAB_WIDTH_MASK       ((1<<6)-1)\n+\n+/* All WS_* -- when extended, adapt constants defined after diff.c:diff_symbol */\n+#define WS_RULE_MASK            ((1<<12)-1)\n+\n extern unsigned whitespace_rule_cfg;\n unsigned whitespace_rule(struct index_state *, const char *);\n unsigned parse_whitespace_rule(const char *);\n-- \n2.51.2-681-g0e6cd5c3b3\n\n"},{"id":"529853","messageId":"aQG_eRPmsxKqJ0H9@pks.im","threadId":"64396","inReplyTo":"xmqq8qgvdm0y.fsf_-_@gitster.g","subject":"Re: [PATCH v2] whitespace: correct bit assignment comments","fromName":"Patrick Steinhardt","fromEmail":"ps@pks.im","sentAt":"2025-10-29T07:17:13Z","receivedAt":"2025-10-29T07:17:24Z","isPatch":true,"sender":{"key":"ps@pks.im","avatar":"https://avatars.githubusercontent.com/u/4056630?v=4"},"body":"On Tue, Oct 28, 2025 at 09:33:01AM -0700, Junio C Hamano wrote:\n> A comment in diff.c claimed that bits up to 12th (counting from 0th)\n> are whitespace rules, and 13th thru 15th are for new/old/context,\n> but it turns out it was miscounting.  Correct them, and clarify\n> where the whitespace rule bits come from in the comment.  Extend bit\n> assignment comments to cover bits used for color-moved, which\n> weren't described.\n> \n> Also update the way these bit constants are defined to use (1 << N)\n> notation, instead of octal constants, as it tends to make it easier\n> to notice a breakage like this.\n> \n> Sprinkle a few blank lines between logically distinct groups of CPP\n> macro definitions to make them easier to read.\n\nThanks, this version looks good to me!\n\nPatrick\n"},{"id":"529862","messageId":"xmqqcy65c07v.fsf@gitster.g","threadId":"64396","inReplyTo":"aQG_eRPmsxKqJ0H9@pks.im","subject":"Re: [PATCH v2] whitespace: correct bit assignment comments","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2025-10-29T13:21:40Z","receivedAt":"2025-10-29T13:21:42Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Patrick Steinhardt <ps@pks.im> writes:\n\n> On Tue, Oct 28, 2025 at 09:33:01AM -0700, Junio C Hamano wrote:\n>> A comment in diff.c claimed that bits up to 12th (counting from 0th)\n>> are whitespace rules, and 13th thru 15th are for new/old/context,\n>> but it turns out it was miscounting.  Correct them, and clarify\n>> where the whitespace rule bits come from in the comment.  Extend bit\n>> assignment comments to cover bits used for color-moved, which\n>> weren't described.\n>> \n>> Also update the way these bit constants are defined to use (1 << N)\n>> notation, instead of octal constants, as it tends to make it easier\n>> to notice a breakage like this.\n>> \n>> Sprinkle a few blank lines between logically distinct groups of CPP\n>> macro definitions to make them easier to read.\n>\n> Thanks, this version looks good to me!\n>\n> Patrick\n\nThanks.\n"}]}