diff --git a/.coveragerc b/.coveragerc index b160954c38..09fee0e09a 100644 --- a/.coveragerc +++ b/.coveragerc @@ -2,3 +2,6 @@ branch = True source = codespell_lib omit = */codespell_lib/tests/* +# Some tests run the codespell entry point in a subprocess; without this their +# coverage is invisible (pytest-cov 7 dropped its own subprocess support). +patch = subprocess diff --git a/codespell_lib/_codespell.py b/codespell_lib/_codespell.py index e7d94aa7e5..1ec09fdf8f 100644 --- a/codespell_lib/_codespell.py +++ b/codespell_lib/_codespell.py @@ -587,8 +587,9 @@ def convert_arg_line_to_args(self, arg_line: str) -> list[str]: choices=range(0, 4), help="set interactive mode when writing changes:\n" "- 0: no interactivity.\n" - "- 1: ask for confirmation.\n" - "- 2: ask user to choose one fix when more than one is available.\n" + "- 1: ask for confirmation; 'a'/'s' answer for the rest of the file.\n" + "- 2: ask user to choose one fix when more than one is available;" + " 'Na'/'s' answer for the rest of the file.\n" "- 3: both 1 and 2", metavar="MODE", ) @@ -806,6 +807,14 @@ def is_text_file(filename: str) -> bool: return b"\x00" not in s +def _no_more_input(misspelling: Misspelling) -> tuple[bool, str, bool]: + # An unanswered prompt must not count as a "yes": stdin being at EOF means + # the answers ran out (or never existed), so leave the word alone and stop + # asking about it in this file. + print("\nNo answer: leaving the rest of this file alone") + return False, misspelling.data, True + + def ask_for_word_fix( line: str, match: Match[str], @@ -814,14 +823,21 @@ def ask_for_word_fix( colors: TermColors, filename: str, lineno: int, -) -> tuple[bool, str]: +) -> tuple[bool, str, bool]: + """Ask about one match. + + Returns (fix, data, remember), where data is the replacement in dictionary + form, uncased, for the caller to case per match, and remember says whether + the answer was given for the rest of the file rather than for this match. + + This function must not mutate `misspelling`: the object is shared by every + match of the word in the run, so an answer would leak into every later + match and file (GH-62). + """ cfilename = f"{colors.FILE}{filename}{colors.DISABLE}" cline = f"{colors.FILE}{lineno}{colors.DISABLE}" wrongword = match.group() - if interactivity <= 0: - return misspelling.fix, fix_case(wrongword, misspelling.data) - line_ui = ( f"{line[: match.start()]}" f"{colors.WWORD}{wrongword}{colors.DISABLE}" @@ -833,29 +849,36 @@ def ask_for_word_fix( fixword = fix_case(wrongword, misspelling.data) while not r: print( - f"{cfilename}:{cline}: {line_ui}\t{wrongword} ==> {fixword} (Y/n) ", + f"{cfilename}:{cline}: {line_ui}\t{wrongword} ==> {fixword} (Y/n/a/s) ", end="", flush=True, ) - r = sys.stdin.readline().strip().upper() + answer = sys.stdin.readline() + if not answer: + return _no_more_input(misspelling) + r = answer.strip().upper() if not r: r = "Y" - if r not in ("Y", "N"): - print("Say 'y' or 'n'") + if r not in ("Y", "N", "A", "S"): + print( + "Say 'y' or 'n' for this one, " + "'a' or 's' for all of them in this file" + ) r = "" - if r == "N": - misspelling.fix = False + return r in ("Y", "A"), misspelling.data, r in ("A", "S") - elif (interactivity & 2) and not misspelling.reason: + elif (interactivity & 2) and not misspelling.fix and not misspelling.reason: # if it is not disabled, i.e. it just has more than one possible fix, # we ask the user which word to use r = "" + remember = False opt = [w.strip() for w in misspelling.data.split(",")] while not r: print( - f"{cfilename}:{cline}: {line_ui} Choose an option (blank for none): ", + f"{cfilename}:{cline}: {line_ui} Choose an option " + "(blank for none, Na for whole file, s to skip): ", end="", ) for i, o in enumerate(opt): @@ -863,21 +886,26 @@ def ask_for_word_fix( print(f" {i}) {fixword}", end="") print(": ", end="", flush=True) - n = sys.stdin.readline().strip() + answer = sys.stdin.readline() + if not answer: + return _no_more_input(misspelling) + n = answer.strip().lower() if not n: break + if n == "s": + return False, misspelling.data, True + remember = n.endswith("a") try: - i = int(n) + i = int(n[:-1] if remember else n) r = opt[i] except (ValueError, IndexError): print("Not a valid option\n") if r: - misspelling.fix = True - misspelling.data = r + return True, r, remember - return misspelling.fix, fix_case(wrongword, misspelling.data) + return misspelling.fix, misspelling.data, False def print_context( @@ -977,6 +1005,7 @@ def parse_lines( uri_ignore_words: set[str], context: Optional[tuple[int, int]], options: argparse.Namespace, + asked_for: dict[str, tuple[bool, str]], ) -> tuple[int, bool, list[tuple[int, str, str]]]: bad_count = 0 changed = False @@ -1025,7 +1054,6 @@ def parse_lines( extra_words_to_ignore |= pending_next_line_ignore fixed_words = set() - asked_for = set() # If all URI spelling errors will be ignored, erase any URI before # extracting words. Otherwise, apply ignores after extracting words. @@ -1071,20 +1099,26 @@ def parse_lines( fix = misspellings[lword].fix fixword = fix_case(word, misspellings[lword].data) - if options.interactive and lword not in asked_for: - if context is not None: - context_shown = True - print_context(lines, i, context) - fix, fixword = ask_for_word_fix( - lines[i], - match, - misspellings[lword], - options.interactive, - colors=colors, - filename=filename, - lineno=i + 1, - ) - asked_for.add(lword) + if options.interactive: + if lword in asked_for: + fix, data = asked_for[lword] + else: + if context is not None: + context_shown = True + print_context(lines, i, context) + fix, data, remember = ask_for_word_fix( + lines[i], + match, + misspellings[lword], + options.interactive, + colors=colors, + filename=filename, + lineno=line_number + 1, + ) + if remember: + asked_for[lword] = (fix, data) + # The answer is uncased: case it for this match. + fixword = fix_case(word, data) if summary and fix: summary.update(lword) @@ -1225,6 +1259,9 @@ def parse_file( # Parse lines. changed = False changes_made: list[tuple[int, str, str]] = [] + # Answers given for the whole file ('a'/'s'): lword -> (fix, uncased fix). + # Plain y/n answers are not remembered here: they are about one match. + asked_for: dict[str, tuple[bool, str]] = {} for fragment in fragments: ignore, _, _ = fragment if ignore: @@ -1244,6 +1281,7 @@ def parse_file( uri_ignore_words, context, options, + asked_for, ) bad_count += bad_count_update changed = changed or changed_update diff --git a/codespell_lib/tests/test_basic.py b/codespell_lib/tests/test_basic.py index c82d8477dc..0127f57013 100644 --- a/codespell_lib/tests/test_basic.py +++ b/codespell_lib/tests/test_basic.py @@ -1671,3 +1671,205 @@ def test_args_from_file( print("Testing with direct call to cs_.main()") r = cs_.main(*args[1:]) print(f"{r=}") + + +PROMPT = "(Y/n/a/s)" + + +def run_codespell_interactive( + args: tuple[Any, ...], + answers: str, + cwd: Optional[Path] = None, +) -> "subprocess.CompletedProcess[str]": + """Run codespell feeding interactive answers on stdin.""" + args = tuple(str(arg) for arg in args) + return subprocess.run( # noqa: S603 + ["codespell", *args], # noqa: S607 + cwd=cwd, + input=answers, + capture_output=True, + encoding="utf-8", + check=False, + ) + + +def test_interactive_rejection_is_per_match( + tmp_path: Path, +) -> None: + """A y/n answer is about one match, not the rest of the run (GH-62).""" + f1 = tmp_path / "f1.txt" + f2 = tmp_path / "f2.txt" + f1.write_text("abandonned\nabandonned\n") + f2.write_text("abandonned\n") + proc = run_codespell_interactive(("-w", "-i", "1", f1, f2), answers="n\ny\ny\n") + assert proc.stdout.count(PROMPT) == 3 + assert f1.read_text() == "abandonned\nabandoned\n" + assert f2.read_text() == "abandoned\n" + + +def test_interactive_answer_for_whole_file( + tmp_path: Path, +) -> None: + """'a' and 's' answer for the rest of the file, and stop at its end.""" + f1 = tmp_path / "f1.txt" + f2 = tmp_path / "f2.txt" + f1.write_text("abandonned\nabandonned\n") + f2.write_text("abandonned\nabandonned\n") + proc = run_codespell_interactive(("-w", "-i", "1", f1, f2), answers="s\na\n") + assert proc.stdout.count(PROMPT) == 2 + assert f1.read_text() == "abandonned\nabandonned\n" + assert f2.read_text() == "abandoned\nabandoned\n" + + +def test_interactive_whole_file_answer_spans_fragments( + tmp_path: Path, +) -> None: + """An ignored multiline region splits a file but must not re-ask.""" + f = tmp_path / "f.txt" + f.write_text("abandonned\nSKIPSTART\nfoo\nSKIPEND\nabandonned\n") + proc = run_codespell_interactive( + ("-w", "-i", "1", "--ignore-multiline-regex", r"SKIPSTART[\s\S]*?SKIPEND", f), + answers="s\n", + ) + assert proc.stdout.count(PROMPT) == 1 + assert f.read_text() == "abandonned\nSKIPSTART\nfoo\nSKIPEND\nabandonned\n" + + +def test_interactive_answer_keeps_case( + tmp_path: Path, +) -> None: + """An answer is cased for each match, not for the one that was asked about.""" + f = tmp_path / "f.txt" + f.write_text("abandonned\nAbandonned\nABANDONNED\n") + proc = run_codespell_interactive(("-w", "-i", "1", f), answers="a\n") + assert proc.stdout.count(PROMPT) == 1 + assert f.read_text() == "abandoned\nAbandoned\nABANDONED\n" + + +def test_interactive_context_is_shown_once( + tmp_path: Path, +) -> None: + """-C prints the surrounding lines before asking, and not again after.""" + f = tmp_path / "f.txt" + f.write_text("first line\nabandonned\n") + proc = run_codespell_interactive(("-w", "-i", "1", "-C", "1", f), answers="n\n") + assert proc.stdout.count(PROMPT) == 1 + assert proc.stdout.count("first line") == 1 + assert f.read_text() == "first line\nabandonned\n" + + +def test_interactive_invalid_answer_asks_again( + tmp_path: Path, +) -> None: + """Anything that is not y/n/a/s re-asks about the same match.""" + f = tmp_path / "f.txt" + f.write_text("abandonned\n") + proc = run_codespell_interactive(("-w", "-i", "1", f), answers="x\ny\n") + assert proc.stdout.count(PROMPT) == 2 + assert "Say 'y' or 'n'" in proc.stdout + assert f.read_text() == "abandoned\n" + + +@pytest.mark.parametrize( + ("level", "text"), + [ + ("1", "abandonned\nabandonned\n"), # asked with y/n/a/s + ("2", "aache\naache\n"), # asked with a list of fixes + ("3", "abandonned\naache\n"), # both + ], +) +def test_interactive_no_answer_fixes_nothing( + tmp_path: Path, + level: str, + text: str, +) -> None: + """Running out of answers must not count as accepting the rest.""" + f = tmp_path / "f.txt" + f.write_text(text) + proc = run_codespell_interactive(("-w", "-i", level, f), answers="") + assert "No answer" in proc.stdout + assert f.read_text() == text + + +def test_interactive_level_2_answer_keeps_case( + tmp_path: Path, +) -> None: + """The same, for the answer chosen from a list of fixes. + + The list is asked about once per match, and keeps every candidate: choosing + one used to narrow the list for every later match (GH-62). + """ + f = tmp_path / "f.txt" + f.write_text("aache\nAache\n") + proc = run_codespell_interactive(("-w", "-i", "2", f), answers="0\n0\n") + assert proc.stdout.count("Choose an option") == 2 + assert proc.stdout.count("1) ache") == 1 + assert proc.stdout.count("1) Ache") == 1 + assert f.read_text() == "cache\nCache\n" + + +def test_interactive_level_2_answer_for_whole_file( + tmp_path: Path, +) -> None: + """A number with a trailing 'a' picks that fix for the rest of the file.""" + f = tmp_path / "f.txt" + f.write_text("aache\nAache\n") + proc = run_codespell_interactive(("-w", "-i", "2", f), answers="0a\n") + assert proc.stdout.count("Choose an option") == 1 + assert f.read_text() == "cache\nCache\n" + + +def test_interactive_level_2_skip_whole_file( + tmp_path: Path, +) -> None: + """'s' leaves the word alone for the rest of the file.""" + f1 = tmp_path / "f1.txt" + f2 = tmp_path / "f2.txt" + f1.write_text("aache\naache\n") + f2.write_text("aache\n") + proc = run_codespell_interactive(("-w", "-i", "2", f1, f2), answers="s\n0\n") + assert proc.stdout.count("Choose an option") == 2 + assert f1.read_text() == "aache\naache\n" + assert f2.read_text() == "cache\n" + + +def test_interactive_level_2_invalid_answer_asks_again( + tmp_path: Path, +) -> None: + """A bare 'a' is not a choice: it re-asks rather than picking something.""" + f = tmp_path / "f.txt" + f.write_text("aache\n") + proc = run_codespell_interactive(("-w", "-i", "2", f), answers="a\n9\n1\n") + assert proc.stdout.count("Choose an option") == 3 + assert proc.stdout.count("Not a valid option") == 2 + assert f.read_text() == "ache\n" + + +def test_interactive_level_2_no_prompt_for_single_fix( + tmp_path: Path, +) -> None: + """Level 2 prompts only when more than one fix is available (as --help says). + + A word with a single candidate used to get an option list where the blank + "none" answer still applied the fix (GH-62). + """ + f = tmp_path / "f.txt" + f.write_text("abandonned\n") + proc = run_codespell_interactive(("-w", "-i", "2", f), answers="\n") + assert "Choose an option" not in proc.stdout + assert f.read_text() == "abandoned\n" + + +def test_interactive_level_3_rejection_keeps_yn_prompt( + tmp_path: Path, +) -> None: + """A rejected word must stay a Y/n question, not degrade to an option list.""" + f1 = tmp_path / "f1.txt" + f2 = tmp_path / "f2.txt" + f1.write_text("abandonned\n") + f2.write_text("abandonned\n") + proc = run_codespell_interactive(("-w", "-i", "3", f1, f2), answers="n\nn\n") + assert proc.stdout.count(PROMPT) == 2 + assert "Choose an option" not in proc.stdout + assert f1.read_text() == "abandonned\n" + assert f2.read_text() == "abandonned\n" diff --git a/pyproject.toml b/pyproject.toml index 6c9f452aa4..7cbdb717e5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -43,7 +43,7 @@ dev = [ "chardet", "pre-commit", "pytest", - "pytest-cov", + "pytest-cov>=7", "pytest-dependency", "Pygments", "ruff", @@ -54,7 +54,7 @@ types = [ "chardet>=5.1.0", "mypy", "pytest", - "pytest-cov", + "pytest-cov>=7", "pytest-dependency", ] @@ -177,7 +177,7 @@ max-complexity = 45 [tool.ruff.lint.pylint] allow-magic-value-types = ["bytes", "int", "str",] -max-args = 13 +max-args = 14 max-branches = 48 max-returns = 12 max-statements = 120