diff --git a/clients/terminal/tui/renderers/diff.py b/clients/terminal/tui/renderers/diff.py index 1935c67..dc04252 100644 --- a/clients/terminal/tui/renderers/diff.py +++ b/clients/terminal/tui/renderers/diff.py @@ -2,6 +2,8 @@ from __future__ import annotations +import re + from rich.console import RenderableType from rich.panel import Panel from rich.text import Text @@ -10,10 +12,29 @@ from .base import ContentRenderer +# ``{marker} {num}│ {content}`` — the line-number prefix the server +# (``navi/tools/filesystem.py:_number_diff``) prepends to each diff content line. +# A fixed space follows the marker, then the right-aligned number and ``│ ``. +_DIFF_NUM_RE = re.compile(r"^ (\d+)│ ?(.*)$", re.DOTALL) + + +def _diff_marker_style(marker: str, theme: Theme) -> str: + if marker == "+": + return theme.success.hex + if marker == "-": + return theme.error.hex + return theme.text.hex + def highlight_unified_diff(text: str, theme: Theme) -> Text: """Color a unified-diff string: ``+`` green, ``-`` red, ``@@`` dim, rest text. + When a content line carries a server-prefixed line number + (``{marker} {num}│ {content}``), the ``{marker} {num}│`` column is rendered + dim and only the content takes the marker color — so the number reads as + meta, not as part of the added/removed text. Lines without the prefix (e.g. + the standalone ``diff`` event, or older callers) are colored whole, as before. + Shared by the standalone ``diff`` event renderer and the ``filesystem`` tool-call renderer (``edit_lines`` / ``smart_edit`` / ``diff`` actions embed a unified diff as plain text in ``ToolResult.output``). @@ -22,14 +43,20 @@ for idx, line in enumerate(text.splitlines()): if idx: out.append("\n") - if line.startswith("+") and not line.startswith("+++"): - out.append(line, style=theme.success.hex) - elif line.startswith("-") and not line.startswith("---"): - out.append(line, style=theme.error.hex) - elif line.startswith("@@"): + if line.startswith("@@"): out.append(line, style=theme.text_dim.hex) - else: + continue + if line.startswith("+++") or line.startswith("---"): out.append(line, style=theme.text.hex) + continue + marker = line[0] if line else "" + rest = line[1:] + m = _DIFF_NUM_RE.match(rest) + if m: + out.append(f"{marker} {m.group(1)}│ ", style=theme.text_dim.hex) + out.append(m.group(2), style=_diff_marker_style(marker, theme)) + else: + out.append(line, style=_diff_marker_style(marker, theme)) return out diff --git a/navi/tools/filesystem.py b/navi/tools/filesystem.py index 3e92178..e77fe2a 100644 --- a/navi/tools/filesystem.py +++ b/navi/tools/filesystem.py @@ -260,6 +260,62 @@ return result +_DIFF_HUNK_RE = re.compile(r"^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@") + + +def _number_diff(diff_lines: list[str]) -> list[str]: + """Prefix each unified-diff content line with its real file line number. + + Numbers come from the enclosing ``@@ -old_start,n +new_start,n @@`` hunk: + removed and context lines use the old (from) line number, added lines use the + new (to) line number. The format ``{marker} {num:>width}│ {content}`` keeps the + ``+``/``-``/`` `` marker first so existing ``startswith``-based highlighting + stays valid; a fixed space separates the marker from the right-aligned number + column. Hunk headers, file headers (``---``/``+++``) and the + ``\\ No newline`` marker are left unchanged. + """ + # First pass: assign numbers, track the max for column width. + numbered: list[tuple[str, int | None, str]] = [] # (marker, num, content) + max_num = 0 + old_line = 0 + new_line = 0 + for line in diff_lines: + if line.startswith("@@"): + m = _DIFF_HUNK_RE.match(line) + if m: + old_line = int(m.group(1)) + new_line = int(m.group(3)) + numbered.append(("", None, line)) + continue + if line.startswith("---") or line.startswith("+++") or line.startswith("\\"): + numbered.append(("", None, line)) + continue + marker = line[0] if line else " " + content = line[1:] + if marker == "+": + num = new_line + new_line += 1 + elif marker == "-": + num = old_line + old_line += 1 + else: # " " context — present in both old and new + num = old_line + old_line += 1 + new_line += 1 + if num > max_num: + max_num = num + numbered.append((marker, num, content)) + + width = len(str(max_num)) if max_num else 1 + out: list[str] = [] + for marker, num, content in numbered: + if num is None: + out.append(content) + else: + out.append(f"{marker} {num:>{width}}│ {content}") + return out + + def _unified_diff(original: list[str], modified: list[str], path: Path) -> str: diff = list( difflib.unified_diff( @@ -270,7 +326,7 @@ lineterm="", ) ) - return "\n".join(diff) + return "\n".join(_number_diff(diff)) # ── Tool class ──────────────────────────────────────────────────────────────── @@ -947,7 +1003,7 @@ ) if not diff: return ToolResult(success=True, output="Files are identical.") - return ToolResult(success=True, output="\n".join(diff)) + return ToolResult(success=True, output="\n".join(_number_diff(diff))) # ── AI action handlers ──────────────────────────────────────────────────── diff --git a/tests/clients/test_filesystem_renderer.py b/tests/clients/test_filesystem_renderer.py index 122c63f..62d5be3 100644 --- a/tests/clients/test_filesystem_renderer.py +++ b/tests/clients/test_filesystem_renderer.py @@ -175,6 +175,34 @@ assert _span_styles_at(diff, "-beta") == {theme.error.hex} +def test_diff_line_number_column_is_dim() -> None: + """Server-prefixed ``{marker} {num}│`` column renders dim; content keeps the + marker color.""" + set_active_theme("gnexus-dark") + theme = get_active_theme() + renderer = FilesystemToolResultRenderer() + msg = { + "type": "tool_call", + "tool": "filesystem", + "args": {"action": "diff"}, + "result": ( + "--- a.py\n+++ b.py\n@@ -1,3 +1,3 @@\n" + " 1│ alpha\n- 2│ beta\n+ 2│ BETA\n 3│ gamma\n" + ), + "success": True, + } + body = _body(renderer.render(msg)) + assert isinstance(body, Text) + # The number prefix is dim (marker + number + separator). + assert _span_styles_at(body, "- 2│") == {theme.text_dim.hex} + assert _span_styles_at(body, "+ 2│") == {theme.text_dim.hex} + # The content after the prefix keeps the marker color. + assert _span_styles_at(body, "BETA") == {theme.success.hex} + assert _span_styles_at(body, "beta") == {theme.error.hex} + # Hunk header is dim, file headers are plain text. + assert theme.text_dim.hex in _span_styles_at(body, "@@ -1,3 +1,3 @@") + + def test_edit_without_metadata_falls_back_to_plain() -> None: set_active_theme("gnexus-dark") theme = get_active_theme() diff --git a/tests/unit/tools/test_filesystem.py b/tests/unit/tools/test_filesystem.py index 2b4fb96..7465ffa 100644 --- a/tests/unit/tools/test_filesystem.py +++ b/tests/unit/tools/test_filesystem.py @@ -135,8 +135,9 @@ # the TUI so the model's context is unchanged. assert "replaced" in result.output assert "@@" not in result.output - assert "-beta" in result.metadata["diff"] - assert "+BETA" in result.metadata["diff"] + # Diff carries real file line numbers (``{marker}{num}│ {content}``). + assert "- 2│ beta" in result.metadata["diff"] + assert "+ 2│ BETA" in result.metadata["diff"] async def test_edit_requires_old_text(self, tool, tmp_path): f = tmp_path / "edit.txt" @@ -266,6 +267,23 @@ assert "+++" in result.output assert "modified" in result.output + async def test_diff_includes_real_line_numbers(self, tool, tmp_path): + a = tmp_path / "a.txt" + a.write_text("line1\nline2\n") + b = tmp_path / "b.txt" + b.write_text("line1\nmodified\n") + + result = await tool.execute({"action": "diff", "path": str(a), "destination": str(b)}) + + assert result.success + # Context line keeps the old (= new) line number; removed/added lines + # carry the old / new number respectively, in ``{marker}{num}│ {content}``. + assert " 1│ line1" in result.output + assert "- 2│ line2" in result.output + assert "+ 2│ modified" in result.output + # Hunk / file headers are not numbered. + assert "@@ -1,2 +1,2 @@" in result.output + async def test_diff_missing_destination(self, tool, tmp_path): a = tmp_path / "a.txt" a.write_text("data")