Skip to content

refactor(mv): split src/core/mv.rs into responsibility-oriented submodules#11

Merged
StudentWeis merged 3 commits into
mainfrom
refactor/10-split-core-mv-module
Apr 27, 2026
Merged

refactor(mv): split src/core/mv.rs into responsibility-oriented submodules#11
StudentWeis merged 3 commits into
mainfrom
refactor/10-split-core-mv-module

Conversation

@StudentWeis
Copy link
Copy Markdown
Owner

Summary

Split the 1750-line src/core/mv.rs into a module directory organized by
responsibility. The public API (pub fn mv, pub fn preview_move) and every
observable behavior are unchanged — this is a pure structural refactor with
minor naming/comment polish (no logic rewrites, no signature changes).

Linked Issue

Closes #10

New layout

src/core/mv/
├── mod.rs        # public entry points + regular/case_only/directory orchestration + tests
├── validate.rs   # resolve_destination / canonicalize_destination / validate_move_paths
├── case_only.rs  # resolve_case_only_destination + case-only external-replacement planning
├── plan.rs       # ReplacementPlan, all plan_*/build_*_replacement helpers, line cache,
│                 # reference-definition URL span parsing, plan bookkeeping
├── apply.rs      # apply_replacements + line-ending preservation + rename-with-fallback +
│                 # execute_with_rollback
└── preview.rs    # build_move_preview + print_dry_run_report

The MoveTransaction data model already lives in
src/core/model/move_transaction.rs and is reused as-is — no new transaction
module is introduced.

Why this split (and not a different one)

The review brief suggested six candidate seams: validate / plan / apply / case_only / preview / transaction. I kept five of them and merged the sixth in
the most faithful way to the code as it actually stands today:

  • transaction is not a new module. The MoveTransaction type is already
    in src/core/model/move_transaction.rs. The only transactional logic left in
    mv.rs was execute_with_rollback, a thin wrapper that runs a closure and
    rolls back on failure. That wrapper lives with its sole caller category —
    file-writing — so it now sits in apply.rs next to apply_replacements.
  • case_only is its own module, not a branch of plan, because it has its
    own destination-detection logic (resolve_case_only_destination) and its own
    filename-case-preserving relative-path routine, and both need to be usable
    before regular planning kicks in. Keeping it separate makes the "am I a
    case-only rename?" question a one-file read.
  • plan is the largest module because replacement construction, directory
    mapping and ReplacementPlan bookkeeping are all one job: "turn a move into a
    concrete set of per-file rewrites, pure computation, no I/O". Splitting it
    further would just scatter tightly coupled helpers.
  • mod.rs keeps the three orchestrators (mv_regular_file,
    mv_case_only_file, mv_directory + their preview twins) because each one
    is a short top-level recipe that reads top-down and calls into every other
    submodule. They are the natural home for cross-module coordination.

Notes for review

  • All cross-module symbols use pub(super) so nothing new is exposed outside
    the mv module.
  • The inline mod tests is preserved verbatim inside mod.rs. Moving the
    tests into per-submodule files would have been cleaner on paper, but every
    existing test relies on use super::* and a mix of private helpers from
    different seams; keeping them together now avoids subtle coverage regressions
    and minimizes diff noise. Per-module test co-location can be a follow-up when
    we do the "second-stage" local abstractions the review doc mentions.
  • Line count before: 1 file, 1750 lines.
    Line count after: 6 files, 895 / 535 / 193 / 118 / 110 / 80 lines.

Testing

  • scripts/precheck.sh passes locally (cargo test: 275 passed, 1 ignored (9 suites))
  • No new tests added — the refactor is purely mechanical and the existing
    suite (including the 19 tests that exercise apply_replacements,
    build_link_replacement, find_reference_definition_url_span,
    execute_with_rollback, and try_rename_regular_file_with) already
    covers the affected surface.

…dules

Break the 1750-line src/core/mv.rs into a module directory organized
by responsibility, without any behavior or public API change:

- validate.rs: path resolution and existence/collision validation
- case_only.rs: case-only rename detection and planning
- plan.rs: replacement planning (external/internal/directory), link
  rewriting construction, ReplacementPlan bookkeeping
- apply.rs: filesystem mutation (apply_replacements, line-ending
  preservation, rename-with-cross-device-fallback, rollback execution)
- preview.rs: build_move_preview and print_dry_run_report
- mod.rs: pub fn mv / pub fn preview_move and the three orchestration
  routines (regular file / case-only / directory), plus the existing
  tests module (kept intact to preserve coverage and minimize risk)

The MoveTransaction data model already lives in
src/core/model/move_transaction.rs and is reused as-is.

All 275 tests pass; scripts/precheck.sh passes locally.

Refs #10
@StudentWeis StudentWeis merged commit 9428d4c into main Apr 27, 2026
8 checks passed
@StudentWeis StudentWeis deleted the refactor/10-split-core-mv-module branch April 27, 2026 01:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

refactor: split src/core/mv.rs into responsibility-oriented submodules

1 participant