Skip to content

Commit 0ab318a

Browse files
Improve primer diff rendering and add test fixtures
- Show ^^ caret hints under changed spans in diff blocks - Make symbol name a clickable link to the source location - Use backtick-formatted spans for location changes - Extract _details_section helper to deduplicate <details> wrapping - Add mixed_changes and type_changed test fixtures
1 parent 99711fb commit 0ab318a

54 files changed

Lines changed: 109 additions & 236 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

pylint/testutils/_primer/comparator.py

Lines changed: 32 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import json
88
from collections import defaultdict
99
from collections.abc import Iterator
10+
from difflib import SequenceMatcher
1011
from pathlib import Path
1112

1213
from pylint.reporters.json_reporter import OldJsonExport
@@ -66,8 +67,8 @@ def format_span(msg: OldJsonExport) -> str:
6667
end_line = msg.get("endLine")
6768
end_col = msg.get("endColumn")
6869
if end_line is not None and end_col is not None:
69-
return f"{start} to {end_line}:{end_col}"
70-
return start
70+
return f"from `{start}` to `{end_line}:{end_col}`"
71+
return f"at `{start}`"
7172

7273

7374
def message_diff(old: OldJsonExport, new: OldJsonExport) -> str:
@@ -85,17 +86,43 @@ def message_diff(old: OldJsonExport, new: OldJsonExport) -> str:
8586
parts: list[str] = []
8687
# Location: combine line/column/endLine/endColumn into one sentence.
8788
if changed_keys & _LOCATION_KEYS:
88-
parts.append(f"Was raised on {format_span(old)}, now on {format_span(new)}.")
89+
parts.append(f"Was raised {format_span(old)}, now {format_span(new)}.")
8990

9091
# Other fields (typically ``message``).
9192
for key in sorted(changed_keys - _LOCATION_KEYS):
9293
old_val = old[key] # type: ignore[literal-required]
9394
new_val = new[key] # type: ignore[literal-required]
9495
if isinstance(old_val, str) and isinstance(new_val, str):
95-
parts.append(f"The {key} changed:\n```diff\n- {old_val}\n+ {new_val}\n```")
96+
caret_line = _caret_hint(old_val, new_val)
97+
diff_block = f"```diff\n- {old_val}\n+ {new_val}\n"
98+
if caret_line:
99+
diff_block += f" {caret_line}\n"
100+
diff_block += "```"
101+
parts.append(diff_block)
96102
else:
97103
parts.append(f"{key}: {old_val!r} -> {new_val!r}")
98-
return "\n".join(parts)
104+
return "\n\n".join(parts)
105+
106+
107+
def _caret_hint(old: str, new: str) -> str:
108+
"""Build a ``^`` marker line highlighting changed spans in *new*.
109+
110+
Uses SequenceMatcher to find which parts of *new* differ from *old*
111+
and places ``^`` characters under them (aligned with the ``+ `` prefix).
112+
Returns an empty string when the whole line changed (carets wouldn't help).
113+
"""
114+
matcher = SequenceMatcher(None, old, new)
115+
carets = [" "] * len(new)
116+
changed_count = 0
117+
for tag, _i1, _i2, j1, j2 in matcher.get_opcodes():
118+
if tag != "equal":
119+
for j in range(j1, j2):
120+
carets[j] = "^"
121+
changed_count += j2 - j1
122+
# If most of the string changed, carets are just noise.
123+
if changed_count > len(new) * 0.6:
124+
return ""
125+
return "".join(carets).rstrip()
99126

100127

101128
class Comparator:

pylint/testutils/_primer/primer_compare_command.py

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -66,18 +66,23 @@ def _source_link(msg: OldJsonExport) -> str:
6666
filepath = str(PurePosixPath(msg["path"]).relative_to(clone_dir))
6767
return f"{url}/blob/{commit}/{filepath}#L{msg['line']}"
6868

69+
def _details_section(title: str, body: str) -> str:
70+
# Two line breaks required after <details> for links to work.
71+
return f"{title}\n\n<details>\n\n{body}</details>\n\n"
72+
6973
comment = f"\n**Effect on [{package}]({url}):**\n\n"
7074

7175
# -- Changed messages (same diagnostic, different details) -----------
7276
if changed_messages:
73-
comment += "The following messages have been changed:\n\n<details>\n"
77+
body = ""
7478
for count, (old, new) in enumerate(changed_messages, 1):
75-
comment += (
76-
f"{count}) {new['symbol']}:\n"
79+
body += (
80+
f"{count}) [{new['symbol']}]({_source_link(new)}):\n"
7781
f"{message_diff(old, new)}\n"
78-
f"{_source_link(new)}\n"
7982
)
80-
comment += "</details>\n\n"
83+
comment += _details_section(
84+
"The following messages have been changed:", body
85+
)
8186

8287
# -- New messages ----------------------------------------------------
8388
astroid_errors = 0
@@ -101,21 +106,21 @@ def _source_link(msg: OldJsonExport) -> str:
101106
"Please open the GitHub Actions log to see what failed or crashed.\n\n"
102107
)
103108
if new_non_astroid_messages:
104-
comment += (
105-
"The following messages are now emitted:\n\n<details>\n"
106-
+ new_non_astroid_messages
107-
+ "</details>\n\n"
109+
comment += _details_section(
110+
"The following messages are now emitted:", new_non_astroid_messages
108111
)
109112

110113
# -- Missing messages ------------------------------------------------
111114
if missing_messages:
112-
comment += "The following messages are no longer emitted:\n\n<details>\n"
115+
body = ""
113116
for count, message in enumerate(missing_messages, 1):
114-
comment += (
117+
body += (
115118
f"{count}) {message['symbol']}:\n*{message['message']}*\n"
116119
f"{_source_link(message)}\n"
117120
)
118-
comment += "</details>\n\n"
121+
comment += _details_section(
122+
"The following messages are no longer emitted:", body
123+
)
119124

120125
return comment
121126

File renamed without changes.
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
[{ "package": "astroid", "missing": 2, "new": 0, "changed": 0 }]

tests/testutils/_primer/fixtures/batched/main_batch0.json renamed to tests/testutils/_primer/cases/batched/main_batch0.json

File renamed without changes.

tests/testutils/_primer/fixtures/batched/main_batch1.json renamed to tests/testutils/_primer/cases/batched/main_batch1.json

File renamed without changes.

tests/testutils/_primer/fixtures/batched/pr_batch0.json renamed to tests/testutils/_primer/cases/batched/pr_batch0.json

File renamed without changes.

tests/testutils/_primer/fixtures/batched/pr_batch1.json renamed to tests/testutils/_primer/cases/batched/pr_batch1.json

File renamed without changes.

tests/testutils/_primer/fixtures/both_empty/expected.txt renamed to tests/testutils/_primer/cases/both_empty/expected.txt

File renamed without changes.

tests/testutils/_primer/fixtures/both_empty/expected_comparator.json renamed to tests/testutils/_primer/cases/both_empty/expected_comparator.json

File renamed without changes.

0 commit comments

Comments
 (0)