Skip to content

Commit c819028

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 6798ab4 commit c819028

23 files changed

Lines changed: 275 additions & 41 deletions

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
[{"package": "astroid", "missing": 2, "new": 0, "changed": 0}]
1+
[{ "package": "astroid", "missing": 2, "new": 0, "changed": 0 }]

tests/testutils/_primer/fixtures/line_moved/expected.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,9 @@
66
The following messages have been changed:
77

88
<details>
9-
1) too-complex:
10-
Was raised on 100:0 to 100:15, now on 103:0 to 103:15.
11-
https://github.com/pylint-dev/astroid/blob/123456789abcdef/astroid/node_classes.py#L103
9+
10+
1) [too-complex](https://github.com/pylint-dev/astroid/blob/123456789abcdef/astroid/node_classes.py#L103):
11+
Was raised from `100:0` to `100:15`, now from `103:0` to `103:15`.
1212
</details>
1313

1414
*This comment was generated for commit v2.14.2*
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
[{"package": "astroid", "missing": 0, "new": 0, "changed": 1}]
1+
[{ "package": "astroid", "missing": 0, "new": 0, "changed": 1 }]

tests/testutils/_primer/fixtures/message_changed/expected.txt

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,13 @@
66
The following messages have been changed:
77

88
<details>
9-
1) locally-disabled:
10-
The message changed:
9+
10+
1) [locally-disabled](https://github.com/pylint-dev/astroid/blob/123456789abcdef/astroid/__init__.py#L91):
1111
```diff
1212
- Locally disabling redefined-builtin (W0622)
1313
+ Locally disabling redefined-builtin [we added some text in the message] (W0622)
14+
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
1415
```
15-
https://github.com/pylint-dev/astroid/blob/123456789abcdef/astroid/__init__.py#L91
1616
</details>
1717

1818
*This comment was generated for commit v2.14.2*
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
[{"package": "astroid", "missing": 0, "new": 0, "changed": 1}]
1+
[{ "package": "astroid", "missing": 0, "new": 0, "changed": 1 }]

tests/testutils/_primer/fixtures/message_changed/expected_truncated.txt

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,8 @@
66
The following messages have been changed:
77

88
<details>
9-
1) locally-disabled:
10-
The message changed:
11-
```diff
12-
- Locally disabling ...
9+
10+
1) [locally-disabled](https://github.com/pylint-dev/astroid/blob/1234...
1311
</details>
1412

1513
*This comment was truncated because GitHub allows only 400 characters in a comment.*

tests/testutils/_primer/fixtures/message_changed/expected_truncated_in_details.txt

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,8 @@
66
The following messages have been changed:
77

88
<details>
9-
1) locally-disabled:
10-
The message changed:
11-
```diff
12-
- Locally disabling redefined-builtin (W...
9+
10+
1) [locally-disabled](https://github.com/pylint-dev/astroid/blob/123456789abcdef/astroid/...
1311
</details>
1412

1513
*This comment was truncated because GitHub allows only 420 characters in a comment.*

tests/testutils/_primer/fixtures/message_changed_line/expected.txt

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,15 @@
66
The following messages have been changed:
77

88
<details>
9-
1) too-complex:
10-
Was raised on 100:0 to 100:15, now on 103:0 to 103:15.
11-
The message changed:
9+
10+
1) [too-complex](https://github.com/pylint-dev/astroid/blob/123456789abcdef/astroid/node_classes.py#L103):
11+
Was raised from `100:0` to `100:15`, now from `103:0` to `103:15`.
12+
1213
```diff
1314
- 'my_function' is too complex. The McCabe rating is 18
1415
+ 'my_function' is too complex. The McCabe rating is 17
16+
^
1517
```
16-
https://github.com/pylint-dev/astroid/blob/123456789abcdef/astroid/node_classes.py#L103
1718
</details>
1819

1920
*This comment was generated for commit v2.14.2*

0 commit comments

Comments
 (0)