diff --git a/scripts/exporter/retropie_exporter.py b/scripts/exporter/retropie_exporter.py index 3eeb6b8f..4d6f7b7a 100644 --- a/scripts/exporter/retropie_exporter.py +++ b/scripts/exporter/retropie_exporter.py @@ -48,6 +48,7 @@ _FILENAME = re.compile(r"[A-Za-z0-9][\w.+-]*\.[A-Za-z0-9]{1,5}\b") # than by a sentence template, and the names in it are the list to extend. _BIOS_CLAUSE = re.compile(r"BIOS\b.*?(?=\\n|$)", re.DOTALL) _BIOS_NOUN = re.compile(r"BIOS (files?)\b") +_SEPARATOR = re.compile(r"\s*(?:,\s*(?:and\s+)?|and\s+)") class Exporter(BaseExporter): @@ -141,12 +142,13 @@ class Exporter(BaseExporter): return {match.group(0).lower() for match in cls._names_in(plain)} @classmethod - def _insertion_point(cls, help_text: str) -> int | None: - """Where a name joins the list, or None when there is no list. + def _enumeration(cls, help_text: str) -> tuple[int, int, list[str]] | None: + """The list of names to extend: its span and its names, or None. A list of alternatives ("a or b", "a/b") is not an enumeration: an appended ", c" reads as one more required file and the plural turns - "one of these" into "all of these". Such a list is left alone. + "one of these" into "all of these". Such a list is left alone, and so + is one with words between its names, which a rewrite would lose. """ clause = _BIOS_CLAUSE.search(help_text) if clause is None: @@ -158,7 +160,14 @@ class Exporter(BaseExporter): between = text[names[0].start():names[-1].end()] if re.search(r"\bor\b|/", between): return None - return clause.start() + names[-1].end() + gaps = [text[a.end():b.start()] for a, b in zip(names, names[1:])] + if not all(_SEPARATOR.fullmatch(gap) for gap in gaps): + return None + return ( + clause.start() + names[0].start(), + clause.start() + names[-1].end(), + [match.group(0) for match in names], + ) @staticmethod def _in_search_order(candidates: list[NativeFile]) -> list[str]: @@ -195,6 +204,21 @@ class Exporter(BaseExporter): return names[0] return ", ".join(names[:-1]) + " and " + names[-1] + @classmethod + def _extended( + cls, + help_text: str, + enumeration: tuple[int, int, list[str]], + missing: list[str], + ) -> str: + """The help with the names added to its list. + + The whole list is written again so the "and" moves before the last + name: appending ", c and d" after "a and b" read "a and b, c and d". + """ + start, end, written = enumeration + return help_text[:start] + cls._join([*written, *missing]) + help_text[end:] + def modules(self, originals: dict[str, str]) -> dict[str, tuple[str, str]]: """Module id and help string of every script that mentions BIOS.""" found: dict[str, tuple[str, str]] = {} @@ -270,8 +294,8 @@ class Exporter(BaseExporter): if not missing: continue - insert_at = self._insertion_point(help_text) - if insert_at is None: + enumeration = self._enumeration(help_text) + if enumeration is None: # No enumeration to extend, and a sentence we would have to # write ourselves is a documentation change, not a correction. skip("names no file to extend", module_id) @@ -281,12 +305,7 @@ class Exporter(BaseExporter): skip("more names than the help enumerates", module_id) continue - new_help = ( - help_text[:insert_at] - + ", " - + self._join(missing) - + help_text[insert_at:] - ) + new_help = self._extended(help_text, enumeration, missing) if len(listed) + len(missing) > 1: new_help = _BIOS_NOUN.sub("BIOS files", new_help, count=1) produced[relative] = originals[relative].replace( diff --git a/tests/test_export_counts.py b/tests/test_export_counts.py index 9546325e..eb8ca7f1 100644 --- a/tests/test_export_counts.py +++ b/tests/test_export_counts.py @@ -373,8 +373,35 @@ class RetroPieProposals(unittest.TestCase): def test_a_list_of_alternatives_is_not_extended(self): alternatives = "Copy the required BIOS file a.rom or b.rom to $biosdir" enumeration = "Copy the required BIOS files a.rom and b.rom to $biosdir" - self.assertIsNone(RetroPie._insertion_point(alternatives)) - self.assertIsNotNone(RetroPie._insertion_point(enumeration)) + self.assertIsNone(RetroPie._enumeration(alternatives)) + self.assertIsNotNone(RetroPie._enumeration(enumeration)) + + def test_an_extended_list_keeps_retropies_idiom(self): + """lr-geargrafx read "syscard3.pce and gexpress.pce, pac-n1.bin and + pac-n10.bin to $biosdir".""" + cases = ( + ("Copy the required BIOS files syscard3.pce and gexpress.pce to $biosdir", + ["pac-n1.bin", "pac-n10.bin"], + "Copy the required BIOS files syscard3.pce, gexpress.pce, pac-n1.bin" + " and pac-n10.bin to $biosdir"), + ("Copy the required BIOS file saturn_bios.bin to $biosdir", + ["mpr-17933.bin"], + "Copy the required BIOS file saturn_bios.bin and mpr-17933.bin to $biosdir"), + ("requires the BIOS files a.bin, b.bin copied to $biosdir", + ["c.bin"], + "requires the BIOS files a.bin, b.bin and c.bin copied to $biosdir"), + ) + for help_text, missing, expected in cases: + with self.subTest(help_text=help_text): + enumeration = RetroPie._enumeration(help_text) + self.assertEqual( + RetroPie._extended(help_text, enumeration, missing), expected + ) + + def test_words_between_names_are_not_rewritten(self): + self.assertIsNone( + RetroPie._enumeration("BIOS files a.bin (US) and b.bin to $biosdir") + ) class BizHawkRewritesOnlyWhatItDeclares(unittest.TestCase):