Skip to content

Commit 61d9eb6

Browse files
refactor(shared): extract escape_rich_markup to shared/rich_utils.py (#752)
Extract duplicated Rich markup escaping into shared/rich_utils.py and update TUI callers/tests to use the shared utility. Follow-up: applied Ruff formatting after CI feedback. Co-authored-by: openhands <openhands@all-hands.dev>
1 parent 154a983 commit 61d9eb6

5 files changed

Lines changed: 32 additions & 35 deletions

File tree

openhands_cli/shared/__init__.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
# Shared utilities for openhands_cli
22

33
from openhands_cli.shared.conversation_summary import extract_conversation_summary
4+
from openhands_cli.shared.rich_utils import escape_rich_markup
45
from openhands_cli.shared.slash_commands import parse_slash_command
56

67

7-
__all__ = ["extract_conversation_summary", "parse_slash_command"]
8+
__all__ = ["escape_rich_markup", "extract_conversation_summary", "parse_slash_command"]

openhands_cli/shared/rich_utils.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
"""Rich text utilities shared across the codebase."""
2+
3+
4+
def escape_rich_markup(text: str) -> str:
5+
"""Escape Rich markup characters in text to prevent markup errors.
6+
7+
This is needed to handle content with special characters (e.g., brackets)
8+
that would otherwise cause MarkupError when rendered in widgets with markup=True.
9+
"""
10+
return text.replace("[", r"\[").replace("]", r"\]")

openhands_cli/tui/panels/history_side_panel.py

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323

2424
from openhands_cli.conversations.models import ConversationMetadata
2525
from openhands_cli.conversations.store.local import LocalFileStore
26+
from openhands_cli.shared.rich_utils import escape_rich_markup
2627
from openhands_cli.theme import OPENHANDS_THEME
2728
from openhands_cli.tui.panels.history_panel_style import HISTORY_PANEL_STYLE
2829

@@ -31,11 +32,6 @@
3132
from openhands_cli.tui.textual_app import OpenHandsApp
3233

3334

34-
def _escape_rich_markup(text: str) -> str:
35-
"""Escape Rich markup characters in text to prevent markup errors."""
36-
return text.replace("[", r"\[").replace("]", r"\]")
37-
38-
3935
class HistoryItemContent(Static):
4036
"""Content widget for a conversation item in the history panel."""
4137

@@ -53,12 +49,12 @@ def __init__(
5349
"""
5450
# Build the content string - show title, id as secondary
5551
time_str = _format_time(conversation.created_at)
56-
conv_id = _escape_rich_markup(conversation.id)
52+
conv_id = escape_rich_markup(conversation.id)
5753

5854
# Use title if available, otherwise use ID
5955
has_title = bool(conversation.title)
6056
if conversation.title:
61-
title = _escape_rich_markup(_truncate(conversation.title, 100))
57+
title = escape_rich_markup(_truncate(conversation.title, 100))
6258
content = f"{title}\n[dim]{conv_id}{time_str}[/dim]"
6359
else:
6460
content = f"[dim]New conversation[/dim]\n[dim]{conv_id}{time_str}[/dim]"
@@ -82,8 +78,8 @@ def has_title(self) -> bool:
8278
def set_title(self, title: str) -> None:
8379
"""Update the displayed title for this history item."""
8480
time_str = _format_time(self._created_at)
85-
title_text = _escape_rich_markup(_truncate(title, 100))
86-
conv_id = _escape_rich_markup(self.conversation_id)
81+
title_text = escape_rich_markup(_truncate(title, 100))
82+
conv_id = escape_rich_markup(self.conversation_id)
8783
self.update(f"{title_text}\n[dim]{conv_id}{time_str}[/dim]")
8884
self._has_title = True
8985

openhands_cli/tui/widgets/richlog_visualizer.py

Lines changed: 9 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
from openhands.tools.task_tracker.definition import TaskTrackerObservation
3131
from openhands.tools.terminal.definition import TerminalAction
3232
from openhands_cli.shared.delegate_formatter import format_delegate_title
33+
from openhands_cli.shared.rich_utils import escape_rich_markup
3334
from openhands_cli.stores import CliSettings
3435
from openhands_cli.theme import OPENHANDS_THEME
3536
from openhands_cli.tui.widgets.collapsible import (
@@ -471,15 +472,15 @@ def _build_action_title(self, event: ActionEvent) -> str:
471472
"""
472473
agent_prefix = self._get_agent_prefix()
473474
summary = (
474-
self._escape_rich_markup(str(event.summary).strip().replace("\n", " "))
475+
escape_rich_markup(str(event.summary).strip().replace("\n", " "))
475476
if event.summary
476477
else ""
477478
)
478479
action = event.action
479480

480481
# Terminal actions: show summary + command (truncated for display)
481482
if isinstance(action, TerminalAction) and action.command:
482-
cmd = self._escape_rich_markup(action.command.strip().replace("\n", " "))
483+
cmd = escape_rich_markup(action.command.strip().replace("\n", " "))
483484
cmd = self._truncate_for_display(cmd)
484485
if summary:
485486
return f"{agent_prefix}[bold]{summary}[/bold][dim]: $ {cmd}[/dim]"
@@ -488,7 +489,7 @@ def _build_action_title(self, event: ActionEvent) -> str:
488489
# File operations: include path with Reading/Editing
489490
elif isinstance(action, FileEditorAction) and action.path:
490491
op = "Reading" if action.command == "view" else "Editing"
491-
path = self._escape_rich_markup(action.path)
492+
path = escape_rich_markup(action.path)
492493
if summary:
493494
return f"{agent_prefix}[bold]{summary}[/bold][dim]: {op} {path}[/dim]"
494495
return f"{agent_prefix}[bold]{op}[/bold][dim] {path}[/dim]"
@@ -523,16 +524,6 @@ def _build_observation_content(
523524
# The Collapsible widget can handle Rich renderables
524525
return str(event.visualize)
525526

526-
def _escape_rich_markup(self, text: str) -> str:
527-
"""Escape Rich markup characters in text to prevent markup errors.
528-
529-
This is needed to handle content with special characters (e.g., Chinese text
530-
with brackets) that would otherwise cause MarkupError when rendered in
531-
Collapsible widgets with markup=True.
532-
"""
533-
# Escape square brackets which are used for Rich markup
534-
return text.replace("[", r"\[").replace("]", r"\]")
535-
536527
def _truncate_for_display(
537528
self, text: str, max_length: int = MAX_LINE_LENGTH, *, from_start: bool = True
538529
) -> str:
@@ -555,7 +546,7 @@ def _clean_and_truncate(self, text: str, *, from_start: bool = True) -> str:
555546
"""Strip, collapse newlines, truncate, and escape Rich markup for display."""
556547
text = str(text).strip().replace("\n", " ")
557548
text = self._truncate_for_display(text, from_start=from_start)
558-
return self._escape_rich_markup(text)
549+
return escape_rich_markup(text)
559550

560551
def _extract_meaningful_title(self, event, fallback_title: str) -> str:
561552
"""Extract a meaningful title from an event, with fallback to truncated
@@ -635,7 +626,7 @@ def _extract_meaningful_title(self, event, fallback_title: str) -> str:
635626
content_str = self._truncate_for_display(content_str)
636627

637628
if content_str.strip():
638-
return f"{fallback_title}: {self._escape_rich_markup(content_str)}"
629+
return f"{fallback_title}: {escape_rich_markup(content_str)}"
639630
except Exception:
640631
pass
641632

@@ -770,7 +761,7 @@ def _create_titled_collapsible(
770761
) -> Collapsible:
771762
"""Create a standard titled collapsible for non-action events."""
772763
title = self._extract_meaningful_title(event, fallback_title)
773-
content_string = self._escape_rich_markup(str(event.visualize))
764+
content_string = escape_rich_markup(str(event.visualize))
774765
return self._make_collapsible(
775766
content_string,
776767
f"{self._get_agent_prefix()}{title}",
@@ -816,7 +807,7 @@ def _create_event_collapsible(self, event: Event) -> Collapsible | None:
816807
if isinstance(event, ActionEvent):
817808
title = self._build_action_title(event)
818809
collapsible = self._make_collapsible(
819-
self._escape_rich_markup(str(content)),
810+
escape_rich_markup(str(content)),
820811
title,
821812
event,
822813
)
@@ -838,9 +829,7 @@ def _create_event_collapsible(self, event: Event) -> Collapsible | None:
838829
title = self._extract_meaningful_title(
839830
event, f"UNKNOWN Event: {event.__class__.__name__}"
840831
)
841-
content_string = (
842-
f"{self._escape_rich_markup(str(content))}\n\nSource: {event.source}"
843-
)
832+
content_string = f"{escape_rich_markup(str(content))}\n\nSource: {event.source}"
844833
return self._make_collapsible(
845834
content_string,
846835
f"{self._get_agent_prefix()}{title}",

tests/tui/widgets/test_richlog_visualizer.py

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
from openhands.sdk.event.conversation_error import ConversationErrorEvent
1616
from openhands.sdk.llm import MessageToolCall
1717
from openhands.tools.terminal.definition import TerminalAction
18+
from openhands_cli.shared.rich_utils import escape_rich_markup
1819
from openhands_cli.stores import CliSettings
1920
from openhands_cli.tui.textual_app import OpenHandsApp
2021
from openhands_cli.tui.widgets.richlog_visualizer import (
@@ -126,16 +127,16 @@ def test_escape_rich_markup_escapes_brackets(self, visualizer):
126127
]
127128

128129
for input_text, expected_output in test_cases:
129-
result = visualizer._escape_rich_markup(input_text)
130+
result = escape_rich_markup(input_text)
130131
assert result == expected_output, (
131132
f"Failed to escape '{input_text}': expected '{expected_output}', "
132133
f"got '{result}'"
133134
)
134135

135136
def test_safe_content_string_escapes_problematic_content(self, visualizer):
136-
"""Test that _escape_rich_markup escapes MarkupError content."""
137+
"""Test that escape_rich_markup escapes MarkupError content."""
137138
problematic_content = "+0.3%,月变化+0.8%,处于历史40%分位]"
138-
safe_content = visualizer._escape_rich_markup(str(problematic_content))
139+
safe_content = escape_rich_markup(str(problematic_content))
139140

140141
# Verify brackets are escaped
141142
assert r"\]" in safe_content
@@ -161,7 +162,7 @@ def test_escaped_chinese_content_renders_successfully(self, visualizer):
161162
This test demonstrates that the fix resolves the issue.
162163
"""
163164
problematic_content = "+0.3%,月变化+0.8%,处于历史40%分位]"
164-
safe_content = visualizer._escape_rich_markup(str(problematic_content))
165+
safe_content = escape_rich_markup(str(problematic_content))
165166

166167
# This should NOT raise an error
167168
widget = Static(safe_content, markup=True)
@@ -215,7 +216,7 @@ def test_visualizer_handles_chinese_message_event(self, visualizer):
215216
)
216217
def test_various_chinese_patterns_are_escaped(self, visualizer, test_content):
217218
"""Test that various patterns of Chinese text with special chars are handled."""
218-
safe_content = visualizer._escape_rich_markup(str(test_content))
219+
safe_content = escape_rich_markup(str(test_content))
219220

220221
# Verify brackets are escaped
221222
assert "[" not in safe_content or r"\[" in safe_content

0 commit comments

Comments
 (0)