refactor: deepen fix into fixer.py; main() becomes an adapter#21
Merged
Conversation
Records the design choice (made while planning the fixer deepening) to keep constants beside their owning behavior rather than gathering them into a settings/constants module; revisit when config becomes runtime-loaded. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Full-lane bundle 2026-06-26.02-deepen-fixer: extract fix_file + fix_directory into eof_fixer/fixer.py, make main() a thin adapter, and move the test surface off the argv/stdout/cwd harness. Three-task TDD plan with complete code. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
One deviation from the brief: added an empty-read guard in _detect_trailing (after read(1) returns b"") because BytesIO silently clamps seek(-1, SEEK_END) to position 0 on an empty buffer rather than raising OSError as real files do. Added # noqa: PLR0911 for the extra return statement this introduces. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Also adds test_empty_real_file_unchanged to test_fixer.py to cover the OSError branch in _detect_trailing (seek(-1, SEEK_END) on a real empty file), which was previously masked by the deleted harness tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…railing
Swap the final `return ("none", 0) # pragma: no cover` for
`raise AssertionError("unreachable")` so the invariant fails loud instead
of silently returning a wrong value. With only 6 return paths remaining,
the PLR0911 suppression is no longer needed and has been removed.
EM101 is suppressed inline on the unreachable line only.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
A deepening refactor (from the architecture review): the EOF-fixing logic moves out of
main.pyinto a new moduleeof_fixer/fixer.py, andmain()becomes a thin CLI adapter. The goal is testability — the fix behavior previously had no interface, so all 16 tests reached it by swappingsys.argv+sys.stdout+os.chdir. Now the interface is the test surface.No behavior change except one documented, non-semantic detail:
Fixinglines are batch-rendered after the walk instead of streamed during it (same lines, order, exit codes).Shape
eof_fixer/fixer.py:fix_file(file_obj, *, check) -> bool— normalize one open file; binary-skip + the none/append_lf/truncate action model (moved verbatim).fix_directory(root, *, check=False, extra_excludes=()) -> list[pathlib.Path]— drivesdiscovery.iter_text_files, owns the default-skip policyDEFAULT_EXCLUDES = (".cache", ".uv-cache"), returns the fixed relative paths.eof_fixer/main.py: argparse →is_dirguard (exit 2) →fix_directory→ renderFixinglines → exit1/0. No file-level logic, noos/IOimports.Tests, three layers
fix_file(BytesIO(...))— CRLF/CR/BOM/null-byte/empty, no filesystem.fix_directory(tmp_path, ...)— gitignore/binary/symlink/--exclude/check/readonly/cwd.main();_run_main_inshrinks to serve only these.Planning artifacts (Full lane)
planning/changes/2026-06-26.02-deepen-fixer/planning/decisions/2026-06-26-no-central-settings-module.mdarchitecture/eof-normalization.md,architecture/cli.mdVerification
Built via subagent-driven TDD (per-task spec+quality reviews, all PASS) + a final whole-branch review (Ready to merge: Yes, behavior preservation verified against the old code, incl. the empty-file
OSErrorvsBytesIOpaths).just test→ 35 passed, 100% coverage;just lint-ciclean (ruff select=["ALL"], ty).🤖 Generated with Claude Code