Repository navigation
Deepen mdo C4: split lib.rs into render/page/temp_output/launch - #51
Conversation
C4 / Beads mdo-6w5.6 (epic mdo-6w5, PR order 6 of 6 - last so code moves once into the shapes C1/C2/C3/C5/C6 created). lib.rs was the #2 churn file: 1703 lines carrying four implementations plus a 565-line inline test module. Pure relocation, external interface unchanged (zero caller churn: the crate root re-exports every public name at its old path): - src/render.rs (1006) - the render pipeline per GLOSSARY: parsing, highlight splicing, sanitation, title derivation, stragglers sorted by caller at relocation time (dir_to_file_url: only caller is convert; front-matter/strip helpers: derive_title's), and the single deep entry point convert() with its request/outcome/error types and the O_NOFOLLOW output writer. - src/page.rs (379) - page assembly: assets and CSS/JS consts, wrap_html5, escape_style_end_tags, plus the calendar/datetime math stragglers sorted by caller (wrap_html5's timestamp footer) and html_escape (all callers are wrap_html5's). wrap_html5/source_modified_unix_secs become pub(crate) for convert's base-href and timestamp hand-off. - src/temp_output.rs (167) - temp output: per-source private paths, stem sanitation, private dirs (pub(crate) ensure_private_dir/private_temp_ root for the sample lifecycle at the root). - src/launch.rs (98) - browser openers: ShellExecuteW/opener path, macOS `open`, the xdg-open/gio fallback chain and reaper window. - src/lib.rs (76) - crate docs, module decls, root re-exports, and the setup-sample orchestration (open_setup_sample composes render + temp_output + launch through the same root names). The inline tests moved beside the concepts they test: render owns the splice/sanitize/front-matter/placeholder-safety/title/dir_to_file_url groups; page owns wrap_html5, the CSS-escape test, and the datetime tests; temp_output owns the stem tests and their helpers; the root keeps the setup-sample test. Test counts unchanged. Gates: fmt --check, clippy --all-targets --all-features -D warnings, cargo test --all-targets --all-features --locked all green (113 tests); the byte-identity harness against the pre-epic binary passes all 11 scenarios including rendered-HTML cmp - the split changed no behavior. Beads-Ref: mdo-6w5.6 Agent-Signature: kilo-kilo-auto-efficient on behalf of matt
7d3d389 to
44284bb
Compare
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
| assert!(!escaped.to_ascii_lowercase().contains("</style><script>")); | ||
| } | ||
|
|
||
| // Unix-rooted paths like `/home/user/...` are NOT absolute on Windows |
There was a problem hiding this comment.
SUGGESTION: Misplaced comment - describes tests that moved to src/render.rs
This block explains #[cfg(unix)]/#[cfg(windows)] gating for the dir_to_file_url / Url::from_directory_path path tests and says tests "below", but page.rs contains no such tests: dir_to_file_url and its gated test suites live in src/render.rs (lines 932-1023). It was carried out of lib.rs's single shared test module with the wrong half of the split - it sits here only because it preceded the css_override_escapes_style_end_tags test in the old file. Relocate it next to the tests it documents, or delete it.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
|
|
||
| use std::io; | ||
| use std::path::Path; | ||
| use std::time::Duration; |
There was a problem hiding this comment.
SUGGESTION: use std::time::Duration; produces an unused_imports warning on Windows/macOS builds
Duration is only referenced inside launch_via_xdg_open, which is compiled only under #[cfg(all(unix, not(target_os = "macos")))]. The old monolithic lib.rs also used Duration for StageTimings, so the import was exercised on every platform; after the split it is dead on non-freedesktop targets. CI still passes because the clippy -D warnings job runs on ubuntu only, but the warning is new to this relocation. Gate the import the same way as the function below:
| use std::time::Duration; | |
| #[cfg(all(unix, not(target_os = "macos")))] | |
| use std::time::Duration; |
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
| //! Shared mdo rendering, watch-mode, temp-output, browser launch, page | ||
| //! assembly, and integration helpers. | ||
| //! | ||
| //! The crate is organized by its four real concerns (names per GLOSSARY.md): |
There was a problem hiding this comment.
SUGGESTION: Citation references GLOSSARY.md, which is not in this repository
No GLOSSARY.md (and no case-insensitive match for "GLOSSARY") exists anywhere in the repo at this commit, so "names per GLOSSARY.md" points at nothing for a reader of lib.rs. If the file is landing elsewhere in the epic this resolves itself; otherwise the parenthetical should be dropped or replaced with a reference that exists.
Reply with @kilocode-bot fix it to have Kilo Code address this issue.
Code Review SummaryStatus: 3 Issues Found | Recommendation: Address before merge Overview
The PR's core claim holds: this is a faithful, semantics-preserving relocation. Every item deleted from Issue Details (click to expand)SUGGESTION
Files Reviewed (5 files)
Fix these issues in Kilo Cloud Reviewed by efficient · Input: 313.7K · Output: 24.4K · Cached: 1.9M |
1 participant
C4: Split lib.rs by its four real concerns
Beads mdo-6w5.6 · parent epic mdo-6w5 (PR order 6 of 6)
What
lib.rs (1703 lines, the #2 churn file) splits into the four modules named in GLOSSARY.md — pure relocation, zero caller churn (the root re-exports every public name at its old path):
Inline tests moved beside the concepts: render (splice/sanitize/front-matter/placeholder-safety/title/dir_to_file_url), page (wrap_html5, CSS escape, datetime), temp_output (stem tests + helpers), root (setup-sample). Test counts unchanged.
Gates
fmt --check ✓ · clippy -D warnings ✓ · `cargo test --all-targets --all-features --locked` ✓ (113 tests) · byte-identity harness vs the pre-epic binary: all 11 scenarios identical including rendered-HTML cmp — behavior unchanged
Signed: kilo-kilo-auto-efficient on behalf of matt (Beads mdo-6w5.6)