Repair three mangled rustdoc comments and close the guards that missed them - #50
Merged
Conversation
…ined edit `outcome::perform`, `outcome::perform_handle` and `path::prepare` each had a sentence welded onto the closing fence of their example, and each was missing that sentence from where it belonged: perform "Use this" with no object; the tail on the fence perform_handle an entirely EMPTY `# Errors` section; its body on the fence prepare `# Errors` ending mid-clause at "exceeds `MAX_PATH` before" All three are restored to the sentences the fragments reassemble into, and the fences now close on their own line. What this cost is worth stating precisely, because the obvious guess is wrong. The doctests were **not** disabled: `cargo test --doc -- --list` reports the same three tests before and after, at shifted line numbers, and 36 pass either way. The damage was to the rendered documentation and to the text itself -- rustdoc emitted three `invalid_rust_codeblocks` warnings, the unterminated blocks swallow the prose that follows them, and an empty `# Errors` section documents nothing whatever it renders like. Two guards close the gap, because the existing ones both had a hole this fell through. `cargo doc` in CI denied only the two link lints, so three warnings rendered on every run without failing anything; the docs job now also denies `rustdoc::invalid_rust_codeblocks`, which is the lint that fires on exactly this. Verified the workspace passes under the stricter flags. `tools/check-encoding.ps1` already refused a glued doc-comment marker -- that check exists because one lived in a doc example for nine review rounds -- but its pattern was anchored to end of line, and this variant welds `///` to a fence in the middle of a line. The anchor is removed so `///` after any non-space character is caught. It is zero-hit across the clean repository, and sabotage confirms it fires: re-introducing one glued fence flags the file and exits 1. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The comment on the teardown drain said recv reports None only at end of stream. The code calls recv_timeout, which also reports None on a timeout -- which is precisely why the loop is followed by an is_disconnected assertion. The comment described a weaker mechanism than the one the test actually relies on, and read as though that assertion were belt-and-braces rather than the step that tells the two None cases apart. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes are narrowly scoped doc/guard fixes and the updated rustdoc/guard logic is consistent with the intended failure modes described in the PR.
Pull request overview
Repairs three mangled rustdoc comment blocks in windows-namespace-request-sys and tightens two repository guards (CI rustdoc lints + encoding checker) so the same “glued fence / swallowed prose” failure mode is caught automatically in the future.
Changes:
- Restore the missing prose and re-close the doctest fences for
path::prepare,outcome::perform, andoutcome::perform_handle. - Update CI rustdoc flags to deny
rustdoc::invalid_rust_codeblocks(in addition to intra-doc link lints). - Expand
tools/check-encoding.ps1’s glued-doc-comment detection to catch mid-line welded///cases; clarify a watcher teardown-drain comment to matchrecv_timeoutbehavior.
File summaries
| File | Description |
|---|---|
| tools/check-encoding.ps1 | Broadens the “glued doc comment” guard to catch welded /// (including on closing fences). |
| crates/windows-namespace-request-sys/src/path.rs | Fixes the prepare rustdoc # Errors text and ensures the code fence closes cleanly. |
| crates/windows-namespace-request-sys/src/outcome.rs | Fixes two rustdoc blocks (restores missing sentences and re-closes the fences). |
| crates/windows-file-watcher/src/watcher/tests.rs | Updates the teardown-drain comment to accurately describe the recv_timeout + is_disconnected logic. |
| .github/workflows/ci.yml | Makes rustdoc warnings for invalid codeblocks fail CI by denying rustdoc::invalid_rust_codeblocks. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
…t regex `\S///` is the whole condition -- a doc marker touching a non-space character. The surrounding `^.*` and `.*$` were inherited from the pattern's earlier end-of-line-anchored form and were left in place when that anchor was removed, where they are not merely redundant but the expensive part: `^.*` matches to the end of a line and then backtracks hunting for the marker, on every line of every file. Measured on 1.47 MB of this repository's own Rust, a no-match scan -- the case CI runs on every clean build -- went from 872 ms to 288 ms over 20 passes. The whole check now completes in 0.83 s across 445 files. Equivalent, not merely faster. Both damage shapes still match (`let x = 1;///` and a sentence welded to a closing fence) and both clean shapes still do not, and sabotage confirms each variant is flagged with the right line number: the match index still identifies the line, because the marker and the character it is glued to are on it. Raised in review of #50. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The changes correctly repair the rustdoc damage and close the documented guard gaps without introducing functional or API risk.
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 0 new
- Review effort level: Lite
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
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.
Follow-up to #46, found by its Copilot reviews after the merge. Three doc comments in
windows-namespace-request-syshad been mangled by a mis-joined edit, and both guards that should have caught it had a hole.The damage
Each of three items had a sentence welded onto the closing fence of its example, and was missing that same sentence from where it belonged:
outcome::perform"Use this"-- no objectoutcome::perform_handle# Errorssectionpath::prepare# Errorsending mid-clause atexceeds `MAX_PATH` beforeAll three are restored to the sentences the fragments reassemble into, and the fences now close on their own line.
What it actually cost
Worth stating precisely, because the obvious guess is wrong. The doctests were not disabled.
cargo test --doc -- --listreports the same three tests before and after, at shifted line numbers, and 36 pass either way.The damage was to the rendered documentation and to the text itself: rustdoc emitted three
invalid_rust_codeblockswarnings, the unterminated blocks swallow the prose that follows them, and an empty# Errorssection documents nothing however it renders.Two guards closed
Both existing guards had a hole this fell straight through.
CI denied only the two link lints, so three rustdoc warnings rendered on every run without failing anything. The docs job now also denies
rustdoc::invalid_rust_codeblocks, the lint that fires on exactly this. Verified the workspace passes under the stricter flags.tools/check-encoding.ps1already refused a glued doc-comment marker -- that check exists because one lived in a doc example for nine review rounds -- but its pattern was anchored to end of line, and this variant welds///to a fence mid-line. The anchor is removed so///after any non-space character is caught. It is zero-hit across the clean repository, and sabotage confirms it fires: re-introducing one glued fence flags the file and exits 1.Also
A separate review finding on the teardown-drain comment in
windows-file-watcher: it describedrecvreturningNoneonly at end-of-stream, but the code callsrecv_timeout, which also returnsNoneon a timeout -- which is precisely why the loop is followed by anis_disconnectedassertion. The comment described a weaker mechanism than the test relies on, making that assertion read as belt-and-braces rather than the step that tells the twoNonecases apart.Verification
cargo fmt,cargo clippy --all-targets,cargo doc --workspaceunder the stricter RUSTDOCFLAGS, the fullwindows-namespace-request-syssuite (202 + 20 + 11 unit/integration, 36 + 1 doctests), the touched file-watcher test, and both repo guards -- all clean on top of mergedmain.Not touched: a pre-existing
redundant explicit link targetwarning atcrates/windows-thread-ambient-sys/src/state.rs:333, from commite7bdc7cand unrelated to this work.