Re: [PATCH] whitespace: correct bit assignment comments
From: Patrick Steinhardt <hidden>
Date: 2025-10-28 06:39:11
On Mon, Oct 27, 2025 at 01:36:46PM -0700, Junio C Hamano wrote:
quoted hunk ↗ jump to hunk
diff --git a/diff.h b/diff.h index 2fa256c3ef..60749154e7 100644 --- a/diff.h +++ b/diff.h@@ -331,9 +331,9 @@ struct diff_options { int ita_invisible_in_index; /* white-space error highlighting */ -#define WSEH_NEW (1<<12) -#define WSEH_CONTEXT (1<<13) -#define WSEH_OLD (1<<14) +#define WSEH_NEW (1<<12) +#define WSEH_CONTEXT (1<<13) +#define WSEH_OLD (1<<14) unsigned ws_error_highlight; const char *prefix; int prefix_length;
Here you're using tabs for indentation, whereas below you use spaces. We should probably be consistent.
quoted hunk ↗ jump to hunk
diff --git a/ws.h b/ws.h index 5ba676c559..611c6b6d50 100644 --- a/ws.h +++ b/ws.h@@ -7,19 +7,19 @@ struct strbuf; /* * whitespace rules. * used by both diff and apply - * last two digits are tab width + * last two octal-digits are tab width (we support only up to 63). */ -#define WS_BLANK_AT_EOL 0100 -#define WS_SPACE_BEFORE_TAB 0200 -#define WS_INDENT_WITH_NON_TAB 0400 -#define WS_CR_AT_EOL 01000 -#define WS_BLANK_AT_EOF 02000 -#define WS_TAB_IN_INDENT 04000 -#define WS_TRAILING_SPACE (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF) +#define WS_BLANK_AT_EOL (1<<6) +#define WS_SPACE_BEFORE_TAB (1<<7) +#define WS_INDENT_WITH_NON_TAB (1<<8) +#define WS_CR_AT_EOL (1<<9) +#define WS_BLANK_AT_EOF (1<<10) +#define WS_TAB_IN_INDENT (1<<11) +#define WS_TRAILING_SPACE (WS_BLANK_AT_EOL|WS_BLANK_AT_EOF) #define WS_DEFAULT_RULE (WS_TRAILING_SPACE|WS_SPACE_BEFORE_TAB|8)
The "8" here is a bit curious, but this matches what the comment says: the last two digits are the tab width, and there of course is no macro for that.
-#define WS_TAB_WIDTH_MASK 077 +#define WS_TAB_WIDTH_MASK ((1<<6)-1) /* All WS_* -- when extended, adapt diff.c emit_symbol */ -#define WS_RULE_MASK 07777 +#define WS_RULE_MASK ((1<<12)-1) extern unsigned whitespace_rule_cfg; unsigned whitespace_rule(struct index_state *, const char *); unsigned parse_whitespace_rule(const char *);
All of these conversion look correct to me, and I agree that this is easier to read. Thanks! Patrick