Private
Public Access
fix(markdown): remove table double-header + add imgui_md bullet workaround
Table fix (src/markdown_table.py):
- Add TableColumnFlags_.width_stretch to each table_setup_column call
(was missing — columns had no width to wrap against, so text_wrapped
couldn't grow row height → all rows squished together)
- Remove the explicit for-h-in-headers: table_next_column + text_wrapped(h)
loop. table_headers_row() already renders the header from the
table_setup_column() names; the explicit loop was drawing it AGAIN on
top → double-rendered header rows.
Bullet fix (src/markdown_helper.py):
- Revert _render_md_no_bullet_overlap → simple imgui_md.render(chunk);
imgui.spacing() (the original af0bbe97 approach). The complex
workaround was stripping '- ' and rendering stripped text to imgui_md,
which double-rendered '- 1. ...' content (imgui.bullet from my code +
numbered list marker from imgui_md).
- Add MarkdownRenderer._normalize_bullet_delimiters: regex-converts
'* ' markers to '- ' before passing to imgui_md. This works around
the upstream bug in mekhontsev/imgui_md BLOCK_LI where the '*' case
calls ImGui::Bullet() without ImGui::SameLine(), causing the bullet
to render on its own Y with the text on the next Y. The '-' case
uses Text+SameLine which is correct. Cannot fix from Python (we
can't subclass the C++ class) — pre-conversion is the cheapest fix.
Tests:
- test_markdown_table_wrapped.py: updated to assert new behavior
(text_wrapped count == cell count, not header+cell).
- test_markdown_table_columns.py: updated to assert exactly 6
table_next_column calls (cells only, not 9).
- test_markdown_helper_bullets.py: rewrote for new public-API behavior
(imgui_md.render called with the unstripped chunk).
16/16 markdown unit tests pass.
This commit is contained in:
+14
-45
@@ -114,8 +114,8 @@ class MarkdownRenderer:
|
|||||||
"""
|
"""
|
||||||
if not text:
|
if not text:
|
||||||
return
|
return
|
||||||
|
|
||||||
from src.markdown_table import parse_tables, render_table
|
from src.markdown_table import parse_tables, render_table
|
||||||
|
text = self._normalize_bullet_delimiters(text)
|
||||||
blocks = parse_tables(text)
|
blocks = parse_tables(text)
|
||||||
lines = text.splitlines(keepends=True)
|
lines = text.splitlines(keepends=True)
|
||||||
if not lines:
|
if not lines:
|
||||||
@@ -134,7 +134,8 @@ class MarkdownRenderer:
|
|||||||
if md_buf:
|
if md_buf:
|
||||||
chunk = "".join(md_buf)
|
chunk = "".join(md_buf)
|
||||||
if chunk:
|
if chunk:
|
||||||
self._render_md_no_bullet_overlap(chunk)
|
imgui_md.render(chunk)
|
||||||
|
imgui.spacing()
|
||||||
md_buf.clear()
|
md_buf.clear()
|
||||||
|
|
||||||
def flush_code() -> None:
|
def flush_code() -> None:
|
||||||
@@ -188,51 +189,19 @@ class MarkdownRenderer:
|
|||||||
flush_md()
|
flush_md()
|
||||||
flush_code()
|
flush_code()
|
||||||
|
|
||||||
def _render_md_no_bullet_overlap(self, chunk: str) -> None:
|
def _normalize_bullet_delimiters(self, text: str) -> str:
|
||||||
"""Render markdown, but split out bulleted-list sections and render them
|
"""Convert '*' list markers to '-' before passing to imgui_md.
|
||||||
as plain text via imgui.text to avoid imgui_md's known bullet-overlap bug.
|
|
||||||
|
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
|
import re
|
||||||
list_pattern = re.compile(r"^(?P<indent>[ \t]*)(?:[-*+])\s+", re.MULTILINE)
|
return re.sub(r"(?m)^([ \t]*)\*[ \t]+", r"\1- ", text)
|
||||||
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 render_unindented(self, text: str) -> None:
|
def render_unindented(self, text: str) -> None:
|
||||||
"""Render Markdown text with automatic unindentation."""
|
"""Render Markdown text with automatic unindentation."""
|
||||||
|
|||||||
@@ -13,11 +13,8 @@ def render_table(block: "TableBlock") -> None:
|
|||||||
flags = imgui.TableFlags_.borders | imgui.TableFlags_.row_bg | imgui.TableFlags_.resizable
|
flags = imgui.TableFlags_.borders | imgui.TableFlags_.row_bg | imgui.TableFlags_.resizable
|
||||||
if not imgui.begin_table("md_table", n_cols, flags): return
|
if not imgui.begin_table("md_table", n_cols, flags): return
|
||||||
for h in block.headers:
|
for h in block.headers:
|
||||||
imgui.table_setup_column(h)
|
imgui.table_setup_column(h, imgui.TableColumnFlags_.width_stretch)
|
||||||
imgui.table_headers_row()
|
imgui.table_headers_row()
|
||||||
for h in block.headers:
|
|
||||||
imgui.table_next_column()
|
|
||||||
imgui.text_wrapped(h)
|
|
||||||
for row in block.rows:
|
for row in block.rows:
|
||||||
imgui.table_next_row()
|
imgui.table_next_row()
|
||||||
for c in row:
|
for c in row:
|
||||||
|
|||||||
@@ -1,54 +1,59 @@
|
|||||||
from unittest.mock import patch, MagicMock
|
from unittest.mock import patch, MagicMock
|
||||||
from src.markdown_helper import MarkdownRenderer
|
from src.markdown_helper import MarkdownRenderer
|
||||||
|
|
||||||
def test_bullet_list_renders_each_item_with_imgui_bullet():
|
def _mock_table(mock_imgui):
|
||||||
chunk = "- one\n- two\n- three\n"
|
mock_imgui.TableFlags_ = type("T", (), {"borders": 1, "row_bg": 2, "resizable": 4})()
|
||||||
with patch("src.markdown_helper.imgui") as mock_imgui, patch("src.markdown_helper.imgui_md") as mock_md:
|
mock_imgui.TableColumnFlags_ = type("C", (), {"width_stretch": 8})()
|
||||||
mock_imgui.bullet = MagicMock()
|
mock_imgui.begin_table.return_value = True
|
||||||
mock_imgui.same_line = MagicMock()
|
mock_imgui.table_next_column = lambda: None
|
||||||
mock_imgui.spacing = MagicMock()
|
mock_imgui.table_next_row = lambda: None
|
||||||
mock_imgui.indent = MagicMock()
|
mock_imgui.table_headers_row = lambda: None
|
||||||
mock_imgui.unindent = MagicMock()
|
mock_imgui.text = lambda *a, **k: None
|
||||||
mock_md.render = MagicMock()
|
mock_imgui.text_wrapped = lambda *a, **k: None
|
||||||
MarkdownRenderer()._render_md_no_bullet_overlap(chunk)
|
mock_imgui.end_table = lambda: None
|
||||||
assert mock_imgui.bullet.call_count >= 3
|
mock_imgui.same_line = lambda: None
|
||||||
assert mock_imgui.same_line.call_count >= 3
|
mock_imgui.spacing = lambda: None
|
||||||
assert mock_md.render.call_count >= 3
|
mock_imgui.indent = lambda *a: None
|
||||||
|
mock_imgui.unindent = lambda *a: None
|
||||||
|
|
||||||
def test_bullet_list_with_blank_lines_uses_spacing():
|
def test_render_calls_imgui_md_render_for_bullet_chunks():
|
||||||
chunk = "- one\n\n- two\n"
|
md = "- one\n- two\n- three\n"
|
||||||
with patch("src.markdown_helper.imgui") as mock_imgui, patch("src.markdown_helper.imgui_md") as mock_md:
|
with patch("src.markdown_helper.imgui_md") as mock_md, \
|
||||||
mock_imgui.bullet = MagicMock()
|
patch("src.markdown_helper.imgui") as mock_imgui, \
|
||||||
mock_imgui.same_line = MagicMock()
|
patch("src.markdown_table.imgui") as mock_table_imgui:
|
||||||
mock_imgui.spacing = MagicMock()
|
_mock_table(mock_table_imgui)
|
||||||
mock_imgui.indent = MagicMock()
|
|
||||||
mock_imgui.unindent = MagicMock()
|
|
||||||
mock_md.render = MagicMock()
|
mock_md.render = MagicMock()
|
||||||
MarkdownRenderer()._render_md_no_bullet_overlap(chunk)
|
mock_imgui.spacing = MagicMock()
|
||||||
assert mock_imgui.spacing.call_count >= 1
|
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():
|
def test_render_does_not_strip_bullet_prefix_from_markdown():
|
||||||
chunk = "# Header\n\nSome prose."
|
md = "- one\n- two\n"
|
||||||
with patch("src.markdown_helper.imgui") as mock_imgui, patch("src.markdown_helper.imgui_md") as mock_md:
|
with patch("src.markdown_helper.imgui_md") as mock_md, \
|
||||||
mock_imgui.bullet = MagicMock()
|
patch("src.markdown_helper.imgui") as mock_imgui, \
|
||||||
mock_imgui.same_line = MagicMock()
|
patch("src.markdown_table.imgui") as mock_table_imgui:
|
||||||
mock_imgui.spacing = MagicMock()
|
_mock_table(mock_table_imgui)
|
||||||
mock_imgui.indent = MagicMock()
|
|
||||||
mock_imgui.unindent = MagicMock()
|
|
||||||
mock_md.render = MagicMock()
|
mock_md.render = MagicMock()
|
||||||
MarkdownRenderer()._render_md_no_bullet_overlap(chunk)
|
mock_imgui.spacing = MagicMock()
|
||||||
assert not mock_imgui.bullet.called
|
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
|
assert mock_md.render.call_count == 1
|
||||||
|
args, _ = mock_md.render.call_args
|
||||||
def test_mixed_bullets_and_prose_splits_correctly():
|
rendered_text = args[0]
|
||||||
chunk = "Prose before.\n\n- b1\n- b2\n\nProse after."
|
assert "1. First question" in rendered_text
|
||||||
with patch("src.markdown_helper.imgui") as mock_imgui, patch("src.markdown_helper.imgui_md") as mock_md:
|
assert "2. Second question" in rendered_text
|
||||||
mock_imgui.bullet = MagicMock()
|
assert not mock_imgui.bullet.called, "no manual imgui.bullet should be added — let imgui_md handle list rendering"
|
||||||
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
|
|
||||||
|
|||||||
@@ -9,4 +9,5 @@ def test_render_table_sets_up_columns_before_rows():
|
|||||||
mock_imgui.begin_table.return_value = True
|
mock_imgui.begin_table.return_value = True
|
||||||
render_table(block)
|
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_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}"
|
||||||
|
|||||||
@@ -1,44 +1,57 @@
|
|||||||
from unittest.mock import patch, MagicMock
|
from unittest.mock import patch, MagicMock
|
||||||
from src.markdown_table import render_table, TableBlock
|
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():
|
def test_render_table_uses_text_wrapped_for_cells():
|
||||||
block = TableBlock(headers=["A"], rows=[["hello"]], span=(0, 2))
|
block = TableBlock(headers=["A"], rows=[["hello"]], span=(0, 2))
|
||||||
with patch("src.markdown_table.imgui") as mock_imgui:
|
with patch("src.markdown_table.imgui") as mock_imgui:
|
||||||
mock_imgui.TableFlags_ = type("T", (), {"borders": 1, "row_bg": 2, "resizable": 4})()
|
_setup_mocks(mock_imgui)
|
||||||
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()
|
|
||||||
render_table(block)
|
render_table(block)
|
||||||
mock_imgui.text_wrapped.assert_any_call("hello")
|
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))
|
block = TableBlock(headers=["A"], rows=[["hello"]], span=(0, 2))
|
||||||
with patch("src.markdown_table.imgui") as mock_imgui:
|
with patch("src.markdown_table.imgui") as mock_imgui:
|
||||||
mock_imgui.TableFlags_ = type("T", (), {"borders": 1, "row_bg": 2, "resizable": 4})()
|
_setup_mocks(mock_imgui)
|
||||||
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()
|
|
||||||
render_table(block)
|
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():
|
def test_render_table_does_not_use_text_for_cells():
|
||||||
block = TableBlock(headers=["A"], rows=[["hello"]], span=(0, 2))
|
block = TableBlock(headers=["A"], rows=[["hello"]], span=(0, 2))
|
||||||
with patch("src.markdown_table.imgui") as mock_imgui:
|
with patch("src.markdown_table.imgui") as mock_imgui:
|
||||||
mock_imgui.TableFlags_ = type("T", (), {"borders": 1, "row_bg": 2, "resizable": 4})()
|
_setup_mocks(mock_imgui)
|
||||||
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()
|
|
||||||
render_table(block)
|
render_table(block)
|
||||||
assert not mock_imgui.text.called, "imgui.text should not be called for table cells"
|
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}"
|
||||||
|
|||||||
Reference in New Issue
Block a user