Skip to content

Refactor: use paths.rs helpers instead of inline .quilt literals#698

Open
fiskus wants to merge 5 commits into
mainfrom
refactor/dot-quilt-paths
Open

Refactor: use paths.rs helpers instead of inline .quilt literals#698
fiskus wants to merge 5 commits into
mainfrom
refactor/dot-quilt-paths

Conversation

@fiskus
Copy link
Copy Markdown
Member

@fiskus fiskus commented May 25, 2026

Summary

  • Expose DOT_QUILT_DIR and DomainPaths::dot_quilt_dir() in quilt-rs::paths, and replace inline .quilt literals with helper calls in quilt-sync (telemetry::diagnostics, commands::debug_dot_quilt, fswatcher::filter) and the quilt-cli install test.
  • Add test_domain_paths_layout to pin the byte-exact path each DomainPaths accessor returns, so any drift in DOT_QUILT_DIR / LINEAGE_FILE / INSTALLED_DIR / MANIFEST_DIR / OBJECTS_DIR / AUTH_DIR fails one focused test rather than silently round-tripping through every downstream caller.

Context

Item #3 from plans/cli/shared-data-dir/problem-rethinking.md: the .quilt directory structure was duplicated as inline strings across crates despite a shared paths module. This PR consolidates the easy cases. Items #1, #2, #4, #5 from that doc are out of scope.

No user-visible behavior change → no CHANGELOG.md entry, no alpha bump.

Test plan

  • cargo check --workspace --all-targets — clean (the pre-existing clippy warning in commands.rs:3245 reproduces on main)
  • cargo test -p quilt-rs --lib paths — 11 passed including new test_domain_paths_layout
  • cargo test -p quilt-sync telemetry::diagnostics — 3 passed
  • cargo test -p quilt-sync fswatcher::filter — 4 passed
  • quilt-cli install tests (require S3 access) — compile-checked via cargo test -p quilt-cli --no-run

Greptile Summary

This PR consolidates inline ".quilt" string literals scattered across quilt-sync and quilt-cli behind a new DOT_QUILT_DIR constant and DomainPaths::dot_quilt_dir() accessor in quilt-rs::paths, with no user-visible behavior change.

  • Adds pub const DOT_QUILT_DIR and DomainPaths::dot_quilt_dir() to quilt-rs/src/paths.rs, and updates four call-sites in commands.rs, fswatcher/filter.rs, diagnostics.rs, and the quilt-cli install test to use these helpers instead of raw string literals.
  • Adds test_domain_paths_layout to pin the byte-exact output of every DomainPaths accessor, so any future drift in the path constants fails in one focused test rather than silently propagating through downstream callers.

Confidence Score: 5/5

Pure refactor with no behavior changes; all four call-sites produce the same paths as before, and the new layout test guards against future drift.

All changed paths were verified to be functionally equivalent to the literals they replace — pkg::TOP_HASH matches the previously-hardcoded hash in the install test, the OsStr comparison in the file-system watcher is sound, and the DomainPaths temporary in commands.rs has no lifetime issues. The only gap is that the four private constants inside paths.rs still embed a bare ".quilt" prefix instead of referencing DOT_QUILT_DIR, but since they are co-located in the same file the risk of unnoticed drift is minimal.

No files require special attention; the suggestion in quilt-rs/src/paths.rs is optional hardening.

Important Files Changed

Filename Overview
quilt-rs/src/paths.rs Added public DOT_QUILT_DIR constant and dot_quilt_dir() accessor on DomainPaths, plus a comprehensive layout-pinning test. The existing private constants still embed the .quilt prefix as bare string literals rather than referencing DOT_QUILT_DIR, but all constants live in the same file so any rename would be visible immediately.
quilt-sync/src-tauri/src/telemetry/diagnostics.rs Replaces the inline .quilt/data.json path construction with domain_paths.lineage(). The auth_dir path is intentionally left as a direct info.data_dir.join(AUTH_DIR) since auth lives outside .quilt; both paths remain correct and consistent.
quilt-sync/src-tauri/src/fswatcher/filter.rs Replaces the ".quilt" string literal in the component filter with quilt::paths::DOT_QUILT_DIR. Functionally identical; OsStr == &str comparison is sound on all platforms.
quilt-sync/src-tauri/src/commands.rs Replaces local_data_dir.join(".quilt") with a temporary DomainPaths instance whose dot_quilt_dir() is called immediately; semantically identical, no lifetime issues.
quilt-cli/src/cli/install.rs Test paths for installed manifests, cached manifests, and objects now route through DomainPaths helpers. Verified that pkg::TOP_HASH matches the previously-hardcoded hash, so the assertions are functionally equivalent.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[DOT_QUILT_DIR const] --> B[dot_quilt_dir method]
    A --> C[LINEAGE_FILE const]
    A --> D[INSTALLED_DIR const]
    A --> E[MANIFEST_DIR const]
    A --> F[OBJECTS_DIR const]

    B --> G[commands.rs debug_dot_quilt]
    A --> H[fswatcher filter is_ignored]
    C --> I[diagnostics.rs lineage path]
    D --> J[install.rs installed_manifest]
    E --> K[install.rs cached_manifests_dir]
    F --> L[install.rs objects_dir]
Loading

Comments Outside Diff (1)

  1. quilt-rs/src/paths.rs, line 49-56 (link)

    P2 The four private path constants immediately below DOT_QUILT_DIR still embed a bare ".quilt" prefix instead of referencing the new constant. If DOT_QUILT_DIR is ever renamed, these four won't follow and every downstream accessor will silently diverge. All constants live in the same file so the risk is low, but using concat! makes the dependency explicit and machine-checkable.

Reviews (1): Last reviewed commit: "autoformat" | Re-trigger Greptile

fiskus added 3 commits May 25, 2026 18:14
Expose `DOT_QUILT_DIR` and `DomainPaths::dot_quilt_dir()` so callers
don't need to spell `.quilt` (or its subpaths) by hand.

- diagnostics.rs: lineage zip entry uses `DomainPaths::lineage()`.
- commands.rs: debug-open uses `DomainPaths::dot_quilt_dir()`.
- fswatcher/filter.rs: ignore check uses `DOT_QUILT_DIR`.
- install.rs tests: post-install assertions use `installed_manifest`,
  `cached_manifests_dir`, and `objects_dir` instead of joined string
  literals.
After the previous commit, downstream tests use `DomainPaths` helpers
instead of inline `.quilt/...` strings — so a typo in `LINEAGE_FILE` or
`OBJECTS_DIR` would silently round-trip through every consumer.

Add one test that asserts the byte-exact path each accessor returns,
giving paths.rs an independent witness for its own constants.
@fiskus fiskus marked this pull request as ready for review May 25, 2026 16:18
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.

1 participant