From b2d6995fa1e28a478b6848dc0bc285cd66bf6c7e Mon Sep 17 00:00:00 2001 From: cssho Date: Mon, 26 Oct 2026 22:06:58 +0900 Subject: [PATCH] Fix code review findings in the preprocessor - Directives inside code fences get their own lenient scope: complete blocks still apply, but malformed directives (and an #if left open at the fence end) stay as text instead of raising. Docs that show md-condition syntax in a fence no longer break the build - Accept YAML scalars as `symbol` (2024, 1.5, true) instead of crashing with TypeError; False is no longer treated as empty - Include the offending line's text in ConditionSyntaxError, since line numbers are offset by MkDocs front matter Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_012pKTVb5TrGn27R29rHHNck --- README.md | 5 +- md_condition/preprocessor.py | 172 +++++++++++++++++++++++++++-------- tests/test_fences.py | 161 ++++++++++++++++++++++++++++++++ 3 files changed, 298 insertions(+), 40 deletions(-) create mode 100644 tests/test_fences.py diff --git a/README.md b/README.md index 041d027..7b13160 100644 --- a/README.md +++ b/README.md @@ -44,7 +44,10 @@ not RELEASE `#if A | B` and `#if A|B` mean the same. - `#else` is optional; nested `#if` blocks are not supported. - Malformed blocks (unclosed `#if`, stray `#else` / `#endif`, duplicate `#else`, nesting) raise - `md_condition.ConditionSyntaxError` with the offending line number. + `md_condition.ConditionSyntaxError` with the offending line number and text. +- Inside fenced code blocks (```` ``` ```` / `~~~`), complete `#if` … `#endif` blocks still apply + (conditional code samples), but malformed directives are left as text, so docs can show + md-condition syntax without breaking the build. ## MkDocs Integration diff --git a/md_condition/preprocessor.py b/md_condition/preprocessor.py index 468a4f9..00b5c4c 100644 --- a/md_condition/preprocessor.py +++ b/md_condition/preprocessor.py @@ -17,58 +17,152 @@ class ConditionSyntaxError(ValueError, AssertionError): """ +class _Block: + """State of the innermost open ``#if`` block (or none) within one scope.""" + + def __init__(self) -> None: + self.start: int | None = None # 1-based line number of the open #if + self.start_line = "" + self.seen_else = False + self.keep = True + # Fence scope only: (raw line, kept) since the #if, flushed on #endif or rolled back + self.buffer: list[tuple[str, bool]] = [] + + class ConditionPreprocessor(Preprocessor): RE_START = re.compile(r"") RE_END = re.compile(r"") RE_ELSE = re.compile(r"") RE_NAME = re.compile(r"[^\s|]+") # names are separated by whitespace and/or `|` + RE_FENCE = re.compile(r"^[ \t]*(`{3,}|~{3,})") + RE_FENCE_CLOSE = re.compile(r"^[ \t]*(`{3,}|~{3,})[ \t]*$") def __init__(self, md: Markdown, extension: ConditionExtension) -> None: super().__init__(md) - # `symbol` is a string ("DEBUG", "DEBUG RELEASE", "DEBUG|RELEASE") or a list of them - symbol = extension.getConfig("symbol") or "" - values = [symbol] if isinstance(symbol, str) else symbol + # `symbol` is a string ("DEBUG", "DEBUG RELEASE", "DEBUG|RELEASE"), a list of them, + # or a YAML scalar such as 2024 (stringified) + symbol = extension.getConfig("symbol") + if symbol is None: + values = [] + elif isinstance(symbol, (list, tuple, set, frozenset)): + values = symbol + else: + values = [symbol] self.symbols = {name for value in values for name in self.RE_NAME.findall(str(value))} def run(self, lines: list[str]) -> list[str]: - new_lines = [] + """Strip excluded lines and directives. - block_start = None # 1-based line of the open #if; None outside a block - seen_else = False - keep = True - for number, line in enumerate(lines, start=1): - start = self.RE_START.match(line) - if start: - if block_start is not None: - raise ConditionSyntaxError( - f"line {number}: nested #if is not supported " - f"(block opened at line {block_start})" - ) - block_start = number - seen_else = False - # `#if A B` / `#if A | B` is true when any configured symbol is listed - keep = not self.symbols.isdisjoint(self.RE_NAME.findall(start.group(1))) - continue - if self.RE_ELSE.match(line): - if block_start is None: - raise ConditionSyntaxError(f"line {number}: #else without #if") - if seen_else: - raise ConditionSyntaxError( - f"line {number}: duplicate #else in block opened at line {block_start}" - ) - seen_else = True - keep = not keep - continue - if self.RE_END.match(line): - if block_start is None: - raise ConditionSyntaxError(f"line {number}: #endif without #if") - block_start = None - keep = True - continue - if keep: + Outside code fences, malformed blocks raise. Inside a fence, directives form their + own scope so docs can show md-condition syntax: well-formed blocks are still + applied (conditional code samples), but malformed directives and an #if left open + at the end of the fence are kept verbatim as example text. + """ + new_lines: list[str] = [] + outer = _Block() + fence: str | None = None # marker of the open code fence, e.g. "```" + inner = _Block() + + def emit(line: str) -> None: + if outer.keep: new_lines.append(line) - if block_start is not None: - raise ConditionSyntaxError(f"line {block_start}: #if is never closed with #endif") + for number, line in enumerate(lines, start=1): + if fence is None: + opened = self.RE_FENCE.match(line) + if opened: + fence = opened.group(1) + inner = _Block() + emit(line) + elif not self._directive(outer, number, line, strict=True): + emit(line) + elif self._closes_fence(line, fence): + fence = None + for raw, _ in inner.buffer: # #if still open: roll back to verbatim text + emit(raw) + inner = _Block() + emit(line) + elif inner.start is None: + if self._directive(inner, number, line, strict=False): + inner.buffer = [(line, False)] # #if opened; buffer until #endif + else: + emit(line) + else: + consumed = self._directive(inner, number, line, strict=False) + if inner.start is None: # #endif: emit what the block kept + for raw, kept in inner.buffer: + if kept: + emit(raw) + inner.buffer = [] + else: + inner.buffer.append((line, inner.keep and not consumed)) + + if inner.start is not None: # fence never closed: same rollback as above + for raw, _ in inner.buffer: + emit(raw) + if outer.start is not None: + raise self._error(outer.start, outer.start_line, "#if is never closed with #endif") return new_lines + + def _directive(self, block: _Block, number: int, line: str, *, strict: bool) -> bool: + """Apply a directive line to ``block``; return True if it was consumed as one. + + Malformed directives raise in strict scope and are left as text otherwise. + """ + start = self.RE_START.match(line) + if start: + if block.start is not None: + return self._malformed( + strict, + number, + line, + f"nested #if is not supported (block opened at line {block.start})", + ) + block.start = number + block.start_line = line + block.seen_else = False + # `#if A B` / `#if A | B` is true when any configured symbol is listed + block.keep = not self.symbols.isdisjoint(self.RE_NAME.findall(start.group(1))) + return True + if self.RE_ELSE.match(line): + if block.start is None: + return self._malformed(strict, number, line, "#else without #if") + if block.seen_else: + return self._malformed( + strict, + number, + line, + f"duplicate #else in block opened at line {block.start}", + ) + block.seen_else = True + block.keep = not block.keep + return True + if self.RE_END.match(line): + if block.start is None: + return self._malformed(strict, number, line, "#endif without #if") + block.start = None + block.keep = True + return True + return False + + def _malformed(self, strict: bool, number: int, line: str, message: str) -> bool: + if strict: + raise self._error(number, line, message) + return False + + @staticmethod + def _error(number: int, line: str, message: str) -> ConditionSyntaxError: + # Line numbers count from the text handed to Markdown (MkDocs strips front matter + # first), so include the line itself to make it findable + return ConditionSyntaxError(f"line {number}: {message}: {line.strip()!r}") + + @classmethod + def _closes_fence(cls, line: str, fence: str) -> bool: + # CommonMark: same character, at least as long, nothing else on the line + closing = cls.RE_FENCE_CLOSE.match(line) + return ( + bool(closing) + and closing.group(1)[0] == fence[0] + and len(closing.group(1)) >= len(fence) + ) diff --git a/tests/test_fences.py b/tests/test_fences.py new file mode 100644 index 0000000..276e35a --- /dev/null +++ b/tests/test_fences.py @@ -0,0 +1,161 @@ +"""Directives inside code fences (code review: docs showing md-condition syntax).""" + +import textwrap + +import markdown +import pytest + +from md_condition import ConditionExtension, ConditionSyntaxError + + +def preprocess(source: str, symbol="DEBUG") -> str: + md = markdown.Markdown(extensions=[ConditionExtension(symbol=symbol)]) + lines = textwrap.dedent(source).split("\n") + return "\n".join(md.preprocessors["condition"].run(lines)) + + +@pytest.mark.parametrize( + "source", + [ + pytest.param( + """\ + ```md + + ```""", + id="lone_else", + ), + pytest.param( + """\ + ```md + + ```""", + id="lone_endif", + ), + pytest.param( + """\ + ```md + + only the opening line + ```""", + id="unclosed_if", + ), + pytest.param( + """\ + ```md + + hidden? + + shown? + ```""", + id="unclosed_if_with_else", + ), + pytest.param( + """\ + ~~~ + + ~~~""", + id="tilde_fence", + ), + pytest.param( + """\ + ```md + + unterminated fence at end of input""", + id="unterminated_fence", + ), + ], +) +def test_malformed_directives_in_fence_are_kept_verbatim(source: str) -> None: + assert preprocess(source) == textwrap.dedent(source) + + +def test_well_formed_block_in_fence_is_applied() -> None: + # Conditional code samples keep working, as in 0.1.x + source = """\ + ```python + + log_level = "debug" + + log_level = "info" + + ```""" + assert preprocess(source) == '```python\nlog_level = "debug"\n```' + assert preprocess(source, symbol="RELEASE") == '```python\nlog_level = "info"\n```' + + +def test_malformed_lines_inside_a_fenced_block_are_text() -> None: + source = """\ + ``` + + + + + + ```""" + # nested #if is text; the 2nd #else is a duplicate and therefore text in the else branch + assert preprocess(source) == "```\n\n```" + + +def test_fence_markers_must_match_to_close() -> None: + source = """\ + ````md + ``` + + ``` + still inside the 4-backtick fence + ````""" + assert preprocess(source) == textwrap.dedent(source) + + +def test_outer_block_hides_whole_fence() -> None: + source = """\ + + ```md + + ``` + + after""" + assert preprocess(source) == "after" + + +def test_fence_does_not_hide_errors_outside_it() -> None: + with pytest.raises(ConditionSyntaxError, match="line 4: #else without #if"): + preprocess( + """\ + ```md + + ``` + """ + ) + + +def test_end_to_end_with_fenced_code() -> None: + md = markdown.Markdown(extensions=["fenced_code", ConditionExtension(symbol="DEBUG")]) + html = md.convert("```md\n\n```") + assert "<!--- #else -->" in html + + +def test_error_message_includes_the_line() -> None: + # Line numbers can be offset by MkDocs front matter, so the text must be findable + with pytest.raises( + ConditionSyntaxError, match=r"line 2: #endif without #if: ''" + ): + preprocess("text\n") + + +@pytest.mark.parametrize( + ("symbol", "condition"), + [ + pytest.param(2024, "2024", id="int"), + pytest.param(1.5, "1.5", id="float"), + pytest.param(True, "True", id="bool"), + pytest.param([2024, "DEBUG"], "2024", id="int_in_list"), + ], +) +def test_yaml_scalar_symbols(symbol, condition: str) -> None: + # YAML turns `symbol: 2024` / `symbol: yes` into int / bool + assert preprocess(f"\nyes\n", symbol=symbol) == "yes" + + +def test_false_symbol_is_not_treated_as_empty() -> None: + assert preprocess("\nyes\n", symbol=False) == "yes"