From 5891ed892c1c9f7963c5fcafaf579047e5a194c8 Mon Sep 17 00:00:00 2001 From: Sebastian Petrescu Date: Mon, 27 Jul 2026 23:51:21 +0300 Subject: [PATCH] Plan: address Stage 1 re-review feedback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: - CRITICAL: Task 2 Step 3 no longer overwrites md2doc.py — extends in place - MAJOR: COMMON.keep_with_next_headings now wired into _render_heading - MINOR: added test for keep_with_next assertion - MINOR: Task 9 note about footer param preservation - MINOR: _render_pre now preserves newlines via add_break() - MINOR: Notes section documents fixture location, security, COMMON wiring --- docs/plans/2026-07-27-md2doc.md | 71 +++++++++++++++++++++++++++++---- 1 file changed, 64 insertions(+), 7 deletions(-) diff --git a/docs/plans/2026-07-27-md2doc.md b/docs/plans/2026-07-27-md2doc.md index c55e290..847aee0 100644 --- a/docs/plans/2026-07-27-md2doc.md +++ b/docs/plans/2026-07-27-md2doc.md @@ -305,9 +305,28 @@ pytest tests/test_md2doc_cli.py -v Expected: FAIL — `md2doc.py` not found. -- [ ] **Step 3: Write minimal md2doc.py skeleton** +- [ ] **Step 3: Extend md2doc.py with imports, STYLES, COMMON, converter stub, and main** -Creează `md2doc.py`: +**IMPORTANT:** `md2doc.py` already exists from Task 1.5 and contains `_bulletize` (and its docstring/imports for `re`). +**DO NOT overwrite the file.** Instead: + +1. Păstrează shebang-ul și docstring-ul existent +2. Adaugă următoarele imports la sfârșitul secțiunii de importuri (după `from pathlib import Path`): + +```python +import argparse +import sys +from pathlib import Path + +import markdown +from bs4 import BeautifulSoup +from docx import Document +from docx.shared import Pt, RGBColor, Cm +from docx.oxml.ns import qn +from docx.oxml import OxmlElement +``` + +3. Adaugă `STYLES` și `COMMON` dict-uri (codul de mai jos) **după** `_bulletize`: ```python #!/usr/bin/env python3 @@ -880,6 +899,18 @@ def test_render_h1_applies_style_heading_color(): assert "1e3a8a" in colors, f"expected heading color 1e3a8a from 'report' style, got {colors}" +def test_render_heading_applies_keep_with_next(): + """COMMON['keep_with_next_headings'] should set paragraph_format.keep_with_next=True.""" + doc = Document() + style = STYLES["elegant"] + node = BeautifulSoup("

Section

", "html.parser").find("h2") + _render_heading(doc, node, level=2, style=style) + root = get_doc_xml(doc) + # Look for w:keepNext inside w:pPr + keep_next = list(root.iter(f"{{{W_NS}}}keepNext")) + assert len(keep_next) >= 1, "expected w:keepNext for heading (COMMON.keep_with_next_headings=True)" + + def test_render_h2_no_bottom_border(): """Only h1 gets border; h2/h3/h4 don't.""" doc = Document() @@ -948,6 +979,8 @@ def _render_heading(doc, node, level: int, style: dict): """Render

-

as Word Heading paragraphs. H1 also gets a bottom paragraph border (per md2pdf pattern). + Applies COMMON['keep_with_next_headings'] so headings don't orphan at page bottom. + Applies COMMON['page_break_before_h1'] if True (style-overridable). """ text = node.get_text(strip=True) p = doc.add_heading(level=level) @@ -958,6 +991,14 @@ def _render_heading(doc, node, level: int, style: dict): for run in p.runs: run.font.color.rgb = RGBColor.from_string(color_hex) + # Wire COMMON config: keep heading with next paragraph + if COMMON.get("keep_with_next_headings", True): + p.paragraph_format.keep_with_next = True + + # H1 page-break-before if style overrides COMMON default + if level == 1 and style.get("page_break_before_h1", COMMON.get("page_break_before_h1", False)): + p.paragraph_format.page_break_before = True + if level == 1: add_paragraph_bottom_border(p, color=color_hex, size="12") @@ -1361,14 +1402,24 @@ def _render_table(doc, node, style: dict): def _render_pre(doc, node, style: dict): - """Render
 (code block) — shaded background + monospace font."""
+    """Render 
 (code block) — shaded background + monospace font.
+
+    Preserves newlines via add_break() between lines (Word ignores literal \\n in runs).
+    Leading whitespace on each line is preserved as literal spaces (monospace font
+    keeps alignment).
+    """
     code_fill = style.get("code_fill", "f7f8fa")
+    # Get text without collapse — preserves whitespace and indentation
     text = node.get_text()
+    lines = text.split("\n")
 
     p = doc.add_paragraph()
-    run = p.add_run(text)
-    run.font.name = "Menlo"
-    run.font.size = Pt(8)
+    for i, line in enumerate(lines):
+        if i > 0:
+            p.add_run().add_break()
+        run = p.add_run(line)
+        run.font.name = "Menlo"
+        run.font.size = Pt(8)
     add_paragraph_shading(p, fill=code_fill)
 
 
@@ -1811,6 +1862,8 @@ diff <(xxd pluxee-todo.pdf) <(xxd /tmp/pluxee-after.pdf) | head -5
 
 Expected: PASS (all tests). PDF byte-identical (dacă `--footer` nu e folosit, output-ul trebuie să fie la fel ca înainte).
 
+**Notă siguranță:** Task 9 schimbă semnătura `convert_md_to_pdf` din `(md_file, output_file, style, forms=False, footer="")` în `(md_file, output_file, style, footer="")`. `footer` rămâne param keyword-only și compatible cu apelul din Task 8 (`convert_md_to_pdf(md_file, pdf_file, args.style, footer=args.footer or "")`). Dacă Task 9 se execută înaintea Task 8, înlocuiește step-ul Task 8.3 cu versiunea fără `forms=` (vezi Task 9 Step 3).
+
 - [ ] **Step 5: Commit**
 
 ```bash
@@ -1883,6 +1936,10 @@ git commit -m "test(md2doc): style smoke test for all 5 styles"
 ## Notes for executor
 
 - **Test discipline (spy-test trap):** All helper tests inspect the resulting `.docx` XML, not mock-call assertions. If a test passes by accident (e.g., the XML is correct for the wrong reason), the visual smoke test in Task 7 Step 5 is the backstop.
-- **H3 keystone:** The list renderer (Task 5) depends on `register_multilevel_numbering` producing XML that Word renders visually as nested lists. XML-level tests cannot prove visual nesting — the **manual visual check in Task 7 Step 5** is the actual break-test for H3.
+- **H3 keystone:** The list renderer (Task 5) depends on `register_multilevel_numbering` producing XML that Word renders visually as nested lists. XML-level tests cannot prove visual nesting — the **manual visual check in Task 5 Step 4.5** is the actual break-test for H3 (reinforced by end-to-end check in Task 7 Step 5).
 - **md2pdf.py file size:** 747 lines (over 300 threshold). Modifications here are surgical (B1: +2 lines, B2: -5 lines). Refactor is out of scope per spec §10.
 - **Regression safety for md2pdf bugfixes:** After Task 9, the byte-identical check on `pluxee-todo.pdf` proves B1+B2 didn't change default behavior.
+- **Test fixture location:** `pluxee-todo.md` lives at repo root, not under `tests/fixtures/`. Acceptable for a single-file project (no test framework isolation needed). If a tests/fixtures/ convention emerges later, move it then.
+- **Task 2 Step 3 critical:** Do NOT overwrite `md2doc.py` — extend it in place. `_bulletize` from Task 1.5 must survive.
+- **COMMON config wired:** `keep_with_next_headings` is applied in `_render_heading` via `paragraph_format.keep_with_next = True`. Other COMMON keys (`page_break_before_h1`, `table_cell_valign`) are scaffolding for future — `page_break_before_h1` IS read in `_render_heading` but defaults to False, and `table_cell_valign` is a TODO (python-docx doesn't expose cell vertical-align via API; needs XML manipulation if needed).
+- **Security note:** Tests use stdlib `xml.etree.ElementTree` to parse `.docx` XML. Since the input is generated by our own code (not external/untrusted), XXE risk is zero. If we ever parse `.docx` from external sources, switch to `defusedxml.ElementTree`.