diff --git a/src/markdown_helper.py b/src/markdown_helper.py index 0c0cac28..8afd16a4 100644 --- a/src/markdown_helper.py +++ b/src/markdown_helper.py @@ -114,8 +114,8 @@ class MarkdownRenderer: """ if not text: return - from src.markdown_table import parse_tables, render_table + text = self._normalize_bullet_delimiters(text) blocks = parse_tables(text) lines = text.splitlines(keepends=True) if not lines: @@ -134,7 +134,8 @@ class MarkdownRenderer: if md_buf: chunk = "".join(md_buf) if chunk: - self._render_md_no_bullet_overlap(chunk) + imgui_md.render(chunk) + imgui.spacing() md_buf.clear() def flush_code() -> None: @@ -188,51 +189,19 @@ class MarkdownRenderer: flush_md() flush_code() - def _render_md_no_bullet_overlap(self, chunk: str) -> None: - """Render markdown, but split out bulleted-list sections and render them - as plain text via imgui.text to avoid imgui_md's known bullet-overlap bug. - """ - import re - list_pattern = re.compile(r"^(?P[ \t]*)(?:[-*+])\s+", re.MULTILINE) - current_pos = 0 - for m in list_pattern.finditer(chunk): - if m.start() > current_pos: - pre = chunk[current_pos:m.start()] - if pre: imgui_md.render(pre) - list_start = m.start() - indent_len = len(m.group("indent")) - i = list_start - while i < len(chunk): - line_end = chunk.find("\n", i) - if line_end == -1: line_end = len(chunk) - line = chunk[i:line_end] - stripped = line.lstrip(" \t") - leading_ws = len(line) - len(stripped) - if leading_ws == indent_len and (stripped.startswith(("- ", "* ", "+ ")) or stripped == ""): - i = line_end + 1 - continue - if leading_ws < indent_len: - break - i = line_end + 1 - list_block = chunk[list_start:i] - for line in list_block.splitlines(): - if not line.strip(): - imgui.spacing() - continue - stripped = line.lstrip(" \t") - indent = len(line) - len(stripped) - if stripped.startswith(("- ", "* ", "+ ")): - imgui.bullet() - imgui.same_line() - imgui_md.render(stripped[2:]) - else: - if indent > 0: imgui.indent(indent * 2) - imgui_md.render(stripped) - if indent > 0: imgui.unindent(indent * 2) - current_pos = i - if current_pos < len(chunk): - tail = chunk[current_pos:] - if tail: imgui_md.render(tail) + def _normalize_bullet_delimiters(self, text: str) -> str: + """Convert '*' list markers to '-' before passing to imgui_md. + + Upstream imgui_md (mekhontsev/imgui_md) has a rendering bug in BLOCK_LI + for the '*' delimiter: it calls ImGui::Bullet() without ImGui::SameLine(), + causing the bullet to render on its own Y position with the text on the + next Y. The '-' delimiter uses Text+SameLine which works correctly. + Converting '* ' to '- ' is the cheapest workaround available from Python + (we cannot subclass the C++ imgui_md class). + [C: src/markdown_helper.py:MarkdownRenderer.render] + """ + import re + return re.sub(r"(?m)^([ \t]*)\*[ \t]+", r"\1- ", text) def render_unindented(self, text: str) -> None: """Render Markdown text with automatic unindentation.""" diff --git a/src/markdown_table.py b/src/markdown_table.py index 6a8e9704..c4c6cc88 100644 --- a/src/markdown_table.py +++ b/src/markdown_table.py @@ -13,11 +13,8 @@ def render_table(block: "TableBlock") -> None: flags = imgui.TableFlags_.borders | imgui.TableFlags_.row_bg | imgui.TableFlags_.resizable if not imgui.begin_table("md_table", n_cols, flags): return for h in block.headers: - imgui.table_setup_column(h) + imgui.table_setup_column(h, imgui.TableColumnFlags_.width_stretch) imgui.table_headers_row() - for h in block.headers: - imgui.table_next_column() - imgui.text_wrapped(h) for row in block.rows: imgui.table_next_row() for c in row: diff --git a/tests/test_markdown_helper_bullets.py b/tests/test_markdown_helper_bullets.py index 1e83009b..fa13f706 100644 --- a/tests/test_markdown_helper_bullets.py +++ b/tests/test_markdown_helper_bullets.py @@ -1,54 +1,59 @@ from unittest.mock import patch, MagicMock from src.markdown_helper import MarkdownRenderer -def test_bullet_list_renders_each_item_with_imgui_bullet(): - chunk = "- one\n- two\n- three\n" - with patch("src.markdown_helper.imgui") as mock_imgui, patch("src.markdown_helper.imgui_md") as mock_md: - mock_imgui.bullet = MagicMock() - mock_imgui.same_line = MagicMock() - mock_imgui.spacing = MagicMock() - mock_imgui.indent = MagicMock() - mock_imgui.unindent = MagicMock() - mock_md.render = MagicMock() - MarkdownRenderer()._render_md_no_bullet_overlap(chunk) - assert mock_imgui.bullet.call_count >= 3 - assert mock_imgui.same_line.call_count >= 3 - assert mock_md.render.call_count >= 3 +def _mock_table(mock_imgui): + mock_imgui.TableFlags_ = type("T", (), {"borders": 1, "row_bg": 2, "resizable": 4})() + mock_imgui.TableColumnFlags_ = type("C", (), {"width_stretch": 8})() + mock_imgui.begin_table.return_value = True + mock_imgui.table_next_column = lambda: None + mock_imgui.table_next_row = lambda: None + mock_imgui.table_headers_row = lambda: None + mock_imgui.text = lambda *a, **k: None + mock_imgui.text_wrapped = lambda *a, **k: None + mock_imgui.end_table = lambda: None + mock_imgui.same_line = lambda: None + mock_imgui.spacing = lambda: None + mock_imgui.indent = lambda *a: None + mock_imgui.unindent = lambda *a: None -def test_bullet_list_with_blank_lines_uses_spacing(): - chunk = "- one\n\n- two\n" - with patch("src.markdown_helper.imgui") as mock_imgui, patch("src.markdown_helper.imgui_md") as mock_md: - mock_imgui.bullet = MagicMock() - mock_imgui.same_line = MagicMock() - mock_imgui.spacing = MagicMock() - mock_imgui.indent = MagicMock() - mock_imgui.unindent = MagicMock() +def test_render_calls_imgui_md_render_for_bullet_chunks(): + md = "- one\n- two\n- three\n" + with patch("src.markdown_helper.imgui_md") as mock_md, \ + patch("src.markdown_helper.imgui") as mock_imgui, \ + patch("src.markdown_table.imgui") as mock_table_imgui: + _mock_table(mock_table_imgui) mock_md.render = MagicMock() - MarkdownRenderer()._render_md_no_bullet_overlap(chunk) - assert mock_imgui.spacing.call_count >= 1 + mock_imgui.spacing = MagicMock() + MarkdownRenderer().render(md, context_id="bullets") + assert mock_md.render.call_count == 1, f"expected 1 imgui_md.render call (full chunk passed through), got {mock_md.render.call_count}" + assert mock_imgui.spacing.call_count >= 1, "imgui.spacing must be called to force vertical gap between chunks" -def test_non_bullet_markdown_routes_to_imgui_md(): - chunk = "# Header\n\nSome prose." - with patch("src.markdown_helper.imgui") as mock_imgui, patch("src.markdown_helper.imgui_md") as mock_md: - mock_imgui.bullet = MagicMock() - mock_imgui.same_line = MagicMock() - mock_imgui.spacing = MagicMock() - mock_imgui.indent = MagicMock() - mock_imgui.unindent = MagicMock() +def test_render_does_not_strip_bullet_prefix_from_markdown(): + md = "- one\n- two\n" + with patch("src.markdown_helper.imgui_md") as mock_md, \ + patch("src.markdown_helper.imgui") as mock_imgui, \ + patch("src.markdown_table.imgui") as mock_table_imgui: + _mock_table(mock_table_imgui) mock_md.render = MagicMock() - MarkdownRenderer()._render_md_no_bullet_overlap(chunk) - assert not mock_imgui.bullet.called + mock_imgui.spacing = MagicMock() + MarkdownRenderer().render(md, context_id="bullets") + args, _ = mock_md.render.call_args + rendered_text = args[0] + assert "- one" in rendered_text, f"bullet prefix must NOT be stripped (regression: was double-rendering as bullet + imgui_md numbered list), got {rendered_text!r}" + assert "- two" in rendered_text + +def test_render_passes_numbered_list_intact_to_imgui_md(): + md = "1. First question\n2. Second question\n" + with patch("src.markdown_helper.imgui_md") as mock_md, \ + patch("src.markdown_helper.imgui") as mock_imgui, \ + patch("src.markdown_table.imgui") as mock_table_imgui: + _mock_table(mock_table_imgui) + mock_md.render = MagicMock() + mock_imgui.spacing = MagicMock() + MarkdownRenderer().render(md, context_id="numbered") assert mock_md.render.call_count == 1 - -def test_mixed_bullets_and_prose_splits_correctly(): - chunk = "Prose before.\n\n- b1\n- b2\n\nProse after." - with patch("src.markdown_helper.imgui") as mock_imgui, patch("src.markdown_helper.imgui_md") as mock_md: - mock_imgui.bullet = MagicMock() - mock_imgui.same_line = MagicMock() - mock_imgui.spacing = MagicMock() - mock_imgui.indent = MagicMock() - mock_imgui.unindent = MagicMock() - mock_md.render = MagicMock() - MarkdownRenderer()._render_md_no_bullet_overlap(chunk) - assert mock_imgui.bullet.call_count >= 2 - assert mock_md.render.call_count >= 2 \ No newline at end of file + args, _ = mock_md.render.call_args + rendered_text = args[0] + assert "1. First question" in rendered_text + assert "2. Second question" in rendered_text + assert not mock_imgui.bullet.called, "no manual imgui.bullet should be added — let imgui_md handle list rendering" diff --git a/tests/test_markdown_table_columns.py b/tests/test_markdown_table_columns.py index 4b10e90b..524bb561 100644 --- a/tests/test_markdown_table_columns.py +++ b/tests/test_markdown_table_columns.py @@ -9,4 +9,5 @@ def test_render_table_sets_up_columns_before_rows(): mock_imgui.begin_table.return_value = True render_table(block) assert mock_imgui.table_setup_column.call_count == 3, f"expected 3 table_setup_column calls (one per column), got {mock_imgui.table_setup_column.call_count}" - assert mock_imgui.table_next_column.call_count >= 9, f"expected at least 9 table_next_column calls (3 headers + 6 cells), got {mock_imgui.table_next_column.call_count}" + assert mock_imgui.table_next_column.call_count == 6, f"expected exactly 6 table_next_column calls (cells only — headers come from table_headers_row, not from an explicit loop), got {mock_imgui.table_next_column.call_count}" + assert mock_imgui.table_next_row.call_count == 2, f"expected 2 table_next_row calls (one per data row, not counting the header row), got {mock_imgui.table_next_row.call_count}" diff --git a/tests/test_markdown_table_wrapped.py b/tests/test_markdown_table_wrapped.py index e8112e24..f63e5cf4 100644 --- a/tests/test_markdown_table_wrapped.py +++ b/tests/test_markdown_table_wrapped.py @@ -1,44 +1,57 @@ from unittest.mock import patch, MagicMock from src.markdown_table import render_table, TableBlock +def _setup_mocks(mock_imgui): + mock_imgui.TableFlags_ = type("T", (), {"borders": 1, "row_bg": 2, "resizable": 4})() + mock_imgui.TableColumnFlags_ = type("C", (), {"width_stretch": 8, "width_fixed": 16})() + mock_imgui.begin_table.return_value = True + mock_imgui.table_next_column = MagicMock() + mock_imgui.table_next_row = MagicMock() + mock_imgui.table_headers_row = MagicMock() + mock_imgui.text_wrapped = MagicMock() + mock_imgui.text = MagicMock() + mock_imgui.end_table = MagicMock() + def test_render_table_uses_text_wrapped_for_cells(): block = TableBlock(headers=["A"], rows=[["hello"]], span=(0, 2)) with patch("src.markdown_table.imgui") as mock_imgui: - mock_imgui.TableFlags_ = type("T", (), {"borders": 1, "row_bg": 2, "resizable": 4})() - mock_imgui.begin_table.return_value = True - mock_imgui.table_next_column = MagicMock() - mock_imgui.table_next_row = MagicMock() - mock_imgui.table_headers_row = MagicMock() - mock_imgui.text_wrapped = MagicMock() - mock_imgui.text = MagicMock() - mock_imgui.end_table = MagicMock() + _setup_mocks(mock_imgui) render_table(block) mock_imgui.text_wrapped.assert_any_call("hello") -def test_render_table_uses_text_wrapped_for_headers(): +def test_render_table_uses_table_headers_row_for_headers(): block = TableBlock(headers=["A"], rows=[["hello"]], span=(0, 2)) with patch("src.markdown_table.imgui") as mock_imgui: - mock_imgui.TableFlags_ = type("T", (), {"borders": 1, "row_bg": 2, "resizable": 4})() - mock_imgui.begin_table.return_value = True - mock_imgui.table_next_column = MagicMock() - mock_imgui.table_next_row = MagicMock() - mock_imgui.table_headers_row = MagicMock() - mock_imgui.text_wrapped = MagicMock() - mock_imgui.text = MagicMock() - mock_imgui.end_table = MagicMock() + _setup_mocks(mock_imgui) render_table(block) - mock_imgui.text_wrapped.assert_any_call("A") + assert mock_imgui.table_headers_row.called, "table_headers_row must be called exactly once to render the header row" + assert mock_imgui.table_headers_row.call_count == 1, f"table_headers_row must be called exactly once (not {mock_imgui.table_headers_row.call_count} — would cause double-header bug)" def test_render_table_does_not_use_text_for_cells(): block = TableBlock(headers=["A"], rows=[["hello"]], span=(0, 2)) with patch("src.markdown_table.imgui") as mock_imgui: - mock_imgui.TableFlags_ = type("T", (), {"borders": 1, "row_bg": 2, "resizable": 4})() - mock_imgui.begin_table.return_value = True - mock_imgui.table_next_column = MagicMock() - mock_imgui.table_next_row = MagicMock() - mock_imgui.table_headers_row = MagicMock() - mock_imgui.text_wrapped = MagicMock() - mock_imgui.text = MagicMock() - mock_imgui.end_table = MagicMock() + _setup_mocks(mock_imgui) render_table(block) - assert not mock_imgui.text.called, "imgui.text should not be called for table cells" \ No newline at end of file + assert not mock_imgui.text.called, "imgui.text should not be called for table cells" + +def test_render_table_uses_width_stretch_for_columns(): + block = TableBlock(headers=["A", "B"], rows=[["1", "2"]], span=(0, 3)) + with patch("src.markdown_table.imgui") as mock_imgui: + _setup_mocks(mock_imgui) + render_table(block) + assert mock_imgui.table_setup_column.call_count == 2 + for call in mock_imgui.table_setup_column.call_args_list: + args, _ = call + assert imgui_flags_passed(args), f"table_setup_column must include width_stretch flag, got args={args}" + +def imgui_flags_passed(args) -> bool: + if len(args) < 2: return False + return bool(args[1] & 8) + +def test_render_table_text_wrapped_count_matches_cell_count(): + block = TableBlock(headers=["A", "B"], rows=[["1", "2"], ["3", "4"], ["5", "6"]], span=(0, 5)) + with patch("src.markdown_table.imgui") as mock_imgui: + _setup_mocks(mock_imgui) + render_table(block) + cell_count = 6 + assert mock_imgui.text_wrapped.call_count == cell_count, f"text_wrapped must be called exactly {cell_count} times (cells only, not headers), got {mock_imgui.text_wrapped.call_count}"