diff --git a/scripts/profile_sync.py b/scripts/profile_sync.py index cf071d02..a7c9ba2a 100644 --- a/scripts/profile_sync.py +++ b/scripts/profile_sync.py @@ -19,6 +19,7 @@ import re import subprocess import sys import tempfile +from collections import Counter from collections.abc import Callable, Iterable, Iterator, Sequence from dataclasses import asdict, dataclass, field, replace from datetime import date @@ -316,6 +317,37 @@ class Citation: in_comment: bool = False +def _list_segments(items: list) -> list[object]: + """The field path segment naming each list item. + + A name keys its entry. supermodel declares Games.xml twice, once per + build mode, and both notes answered to files[Games.xml].note: the + history read one note's citations against the other's text. A repeated + name takes its build mode, or its index when the mode repeats too. + """ + keys: list[tuple[str | None, str | None]] = [] + for item in items: + name = item.get("name") if isinstance(item, dict) else None + mode = item.get("mode") if isinstance(item, dict) else None + keys.append(( + None if name is None else str(name), + mode if isinstance(mode, str) else None, + )) + names = Counter(name for name, _ in keys if name is not None) + modes = Counter(key for key in keys if key[0] is not None) + segments: list[object] = [] + for index, (name, mode) in enumerate(keys): + if name is None: + segments.append(index) + elif names[name] == 1: + segments.append(name) + elif mode and modes[(name, mode)] == 1: + segments.append(f"{name}@{mode}") + else: + segments.append(f"{name}#{index}") + return segments + + def _walk_document( node: object, where: str = "" ) -> Iterator[tuple[str, str, dict | list, object, Any]]: @@ -336,9 +368,9 @@ def _walk_document( else: yield from _walk_document(value, spot) elif isinstance(node, list): + segments = _list_segments(node) for index, item in enumerate(node): - segment = item.get("name") if isinstance(item, dict) else None - spot = f"{where}[{segment if segment is not None else index}]" + spot = f"{where}[{segments[index]}]" if isinstance(item, str): yield "str", spot, node, index, item else: @@ -2515,7 +2547,25 @@ def insert_after_line(text: str, key: str, new_line: str) -> str: def _field_value(line: str) -> str: - return line.split(":", 1)[1].strip().strip('"').strip("'") + """The value of a `key: value` line as the loader reads it. + + etlegacy quotes `(FS_FOpenFileRead \\"pak0.pk3\\")` inside a double-quoted + source_ref: stripping the outer quotes keeps the backslashes, and the + line never equalled the parsed value it was searched for. + """ + raw = line.split(":", 1)[1].strip() + try: + parsed = yaml.safe_load(raw) + except yaml.YAMLError: + parsed = None + if isinstance(parsed, str): + return parsed + return raw.strip('"').strip("'") + + +def _double_quoted(value: str) -> str: + """A YAML double-quoted scalar reading back as value.""" + return '"' + value.replace("\\", "\\\\").replace('"', '\\"') + '"' def find_field_line(text: str, key: str, expected: str, start: int = 0) -> int: @@ -2543,7 +2593,7 @@ def replace_field_line(text: str, key: str, expected: str, value: str, lines = text.splitlines() line = lines[index] indent = line[: len(line) - len(line.lstrip())] - lines[index] = f'{indent}{key}: "{value}"' + lines[index] = f"{indent}{key}: {_double_quoted(value)}" joined = "\n".join(lines) + ("\n" if text.endswith("\n") else "") return joined, index @@ -2631,6 +2681,9 @@ def _prose_moves( return moves +_RANGE_TOKEN_RE = re.compile(r"\d+(?:-\d+)?") + + def _run_after_moves( parts: Sequence[RefPart], moves: dict[int, tuple[int, int, str | None]], @@ -2652,6 +2705,17 @@ def _run_after_moves( ranges.append(f"{start}" if end == start else f"{start}-{end}") else: ranges.append(part.raw) + # The separators stay as written. boom3 writes `libretro.cpp:1635-1643, + # 2384-2405` in a folded scalar, rewritten whole there, and a joined + # `3238-3246,4245-4266` no longer read as the intended document. + written = run.partition(":")[2] + tokens = list(_RANGE_TOKEN_RE.finditer(written)) + if len(tokens) == len(parts) and all( + token.group() == part.raw for token, part in zip(tokens, parts) + ): + for token, rendered in reversed(list(zip(tokens, ranges))): + written = written[:token.start()] + rendered + written[token.end():] + return f"{new_path or parts[0].path}:{written}" return f"{new_path or parts[0].path}:{','.join(ranges)}" diff --git a/scripts/sourceref.py b/scripts/sourceref.py index 9ab514d6..c0bf3fcd 100644 --- a/scripts/sourceref.py +++ b/scripts/sourceref.py @@ -392,13 +392,19 @@ def _prose_runs( ): path_start -= 1 path = "." + path - role = _prose_role(line, path_start) - if not role and _prose_external(line, path_start, known): + # A path opening its line reads its role or project word at the + # end of the line before: genesis_plus_gx_expanded wrapped + # `(Batocera` and `package/batocera/...mk:7` one line apart. + lead = line[:path_start] + if number and not lead.strip(): + lead = lines[number - 1].rstrip() + " " + role = _prose_role(lead, len(lead)) + if not role and _prose_external(lead, len(lead), known): # Kept with its project word, the source_ref convention: the # report looks for the project inside the declared trees # (mupen64plus-ae vendors `rice Config.cpp`) and calls the # citation EXTERNAL only when none carries it. - before = line[:path_start].rstrip() + before = lead.rstrip() word = before[max(before.rfind(" "), before.rfind("\t"), before.rfind("(")) + 1:] path = f"{word} {path}" diff --git a/tests/test_profile_sync.py b/tests/test_profile_sync.py index 70ce2768..030b4761 100644 --- a/tests/test_profile_sync.py +++ b/tests/test_profile_sync.py @@ -2284,6 +2284,17 @@ class TestLineEdits(unittest.TestCase): with self.assertRaises(YamlWriteError): find_field_line(SAMPLE, "source_ref", "a.c:10-12", index + 1) + def test_escaped_quotes_are_matched_as_parsed(self): + # etlegacy writes (FS_FOpenFileRead \\"pak0.pk3\\") inside a + # double-quoted source_ref: the line kept its backslashes and never + # equalled the parsed value, so the recale was refused. + value = 'files.c:5076 (FS_FOpenFileRead "pak0.pk3")' + text = SAMPLE.replace('"b.c:5"', '"files.c:5076 (FS_FOpenFileRead \\"pak0.pk3\\")"') + self.assertEqual(yaml.safe_load(text)["files"][1]["source_ref"], value) + moved = value.replace("5076", "5080") + out, _ = replace_field_line(text, "source_ref", value, moved) + self.assertEqual(yaml.safe_load(out)["files"][1]["source_ref"], moved) + def test_source_ref_outside_files_does_not_shift_the_match(self): text = SAMPLE.replace( "files:", @@ -3645,6 +3656,22 @@ class TestCollectCitations(unittest.TestCase): self.assertEqual(citations[1].parts[0].path, "munt ROMInfo.cpp") self.assertTrue(profile_sync.is_external_citation("munt ROMInfo.cpp")) + def test_project_word_ending_the_line_before_marks_the_path_external(self): + # genesis_plus_gx_expanded wrapped `(Batocera` and + # `package/batocera/...mk:7` one line apart in its notes, and the + # path was read against the declared repository: 1 to review. + document = { + "notes": "Batocera builds it at a0b5a033\n(Batocera\n" + "package/batocera/emulators/gpgx.mk:7), the same tree.\n" + } + citation = collect_citations(document)[0] + self.assertEqual( + citation.parts[0].path, "Batocera package/batocera/emulators/gpgx.mk" + ) + self.assertTrue(profile_sync.is_external_citation(citation.parts[0].path)) + sentence = {"notes": "It is read by the file\nsrc/gpgx.c:7 at boot.\n"} + self.assertEqual(collect_citations(sentence)[0].parts[0].path, "src/gpgx.c") + def test_project_word_resolves_inside_a_declared_tree(self): tree = [ "mupen64plus-video-rice/upstream/src/Config.cpp", @@ -3914,6 +3941,32 @@ class TestRebaseProse(unittest.TestCase): "(q.c:9).", yaml.safe_load(content)["quirk_note"] ) + def test_folded_rename_keeps_the_separator_as_written(self): + # boom3 cites libretro.cpp:1635-1643, 2384-2405 in a folded note. + # The rename rewrote the run as one token without the space, and + # the written document no longer matched the intended one. + text = PROSE_SAMPLE.replace( + "files:", + "quirk_note: >\n Passes fs_game\n (q.c:10-12, 30-40). Checked" + " later.\nfiles:", + ) + self.path.write_text(text, encoding="utf-8") + parts = [ + PartResult(RefPart("q.c", 10, 12, "10-12"), "RENAMED", "src/q.c", 20, 22, []), + PartResult(RefPart("q.c", 30, 40, "30-40"), "RENAMED", "src/q.c", 50, 60, []), + ] + applied = rebase_refs( + self.path, + self._report([self._prose_entry("q.c:10-12, 30-40", parts, "quirk_note")]), + ) + self.assertEqual( + applied, ["quirk_note: q.c:10-12, 30-40 -> src/q.c:20-22, 50-60"] + ) + self.assertIn( + "(src/q.c:20-22, 50-60).", + yaml.safe_load(self.path.read_text())["quirk_note"], + ) + def test_parts_disagreeing_on_the_new_file_do_not_move(self): parts = [ PartResult(RefPart("b.c", 2, 2, "2"), "RENAMED", "src/b.c", 5, 5, []), @@ -4231,6 +4284,40 @@ class TestRealignProse(unittest.TestCase): yaml.safe_load(text)["source_commit"], "pin", ) + def test_two_entries_sharing_a_name_keep_their_own_history(self): + # supermodel declares Games.xml once per build mode. Both notes + # answered to files[Games.xml].note, and the history of the libretro + # note decided where the standalone note was written. + sample = ( + 'emulator: Test\n' + 'source: "https://github.com/o/n"\n' + 'source_commit: "oldpin"\n' + 'files:\n' + ' - name: "Games.xml"\n' + ' mode: standalone\n' + ' note: "Parsed at startup (a.c:10)."\n' + ' - name: "Games.xml"\n' + ' mode: libretro\n' + ' note: "Read through the VFS (b.c:2)."\n' + ) + self._git("init", "-q") + self._git("config", "user.email", "t@t") + self._git("config", "user.name", "t") + self._commit(sample, "profile") + # The pin advances and the libretro note is rewritten, uncommitted. + self.path.write_text( + sample.replace('"oldpin"', '"pin"').replace("(b.c:2)", "(b.c:5)"), + encoding="utf-8", + ) + self.files[("oldpin", "a.c")] = ["x"] * 9 + ["subject"] + self.files[("pin", "a.c")] = ["x"] * 13 + ["subject"] + self.files[("pin", "b.c")] = ["p"] * 4 + ["read"] + messages = profile_sync.realign_prose(self.path, self.tmp.name, dry_run=True) + self.assertEqual( + messages, + ["would recale files[Games.xml@standalone].note: a.c:10 -> a.c:14"], + ) + def test_pin_moved_under_committed_text_is_not_guessed(self): """A committed pin change under the same text may be a correction.