fix: preserve CRLF line endings in 8v write append and find-replace#4
Merged
xsoheilalizadeh merged 4 commits intomainfrom Apr 27, 2026
Merged
Conversation
xsoheilalizadeh
added
bug
xsoheilalizadeh
added a commit
that referenced
this pull request
Apr 27, 2026
Adds 10 tests covering the gaps surfaced by the three review rounds on PR #4: CRLF file without trailing newline, mixed-ending input, boundary file sizes (0/1/2 bytes), find-replace with newline in pattern, and a CRLF concurrent stress test. Tests that exercise currently-broken behavior are #[ignore]'d and will be un-ignored when the underlying bugs are fixed. Refs #3.
Closes #3. Append now detects the file's line ending under the existing exclusive flock by reading the last two bytes (instead of one), so the separator and trailing terminator match CRLF or LF as the file dictates. Content passed via --append is normalized to the file's detected ending if the user passes pure-LF content into a CRLF file. FindReplace now validates the replacement string with the same rules as other content-injecting modes (rejects \r\n and lone \r in the replacement), and converts \n to \r\n in the replacement when the file is CRLF. This closes the silent-corruption gap where a \n in the replacement on a CRLF file produced mixed endings. Adds five regression tests in e2e_write.rs covering Append, FindReplace, Insert, and Delete on CRLF files.
Adds 10 tests covering the gaps surfaced by the three review rounds on PR #4: CRLF file without trailing newline, mixed-ending input, boundary file sizes (0/1/2 bytes), find-replace with newline in pattern, and a CRLF concurrent stress test. Tests that exercise currently-broken behavior are #[ignore]'d and will be un-ignored when the underlying bugs are fixed. Refs #3.
The previous append fix sampled only the last 2 bytes to detect CRLF — insufficient when a CRLF file lacks a trailing \r\n (e.g., written by a non-8v tool, or where the last line was authored without a final newline). Now reads the full file under the existing exclusive flock, scans for any \r\n to detect CRLF, and validates the existing content has no mixed endings before appending. The full read happens inside the flock so it remains race-safe. Inlines a byte-level mixed-ending check in write_guard.rs (option b) — the rule is two comparisons on a Vec<u8> already in hand, so moving it to a separate helper would add surface area without clarity benefit. Un-ignores 4 tests pinned in 98efedc: - append_to_crlf_file_without_trailing_newline_preserves_crlf - append_then_subsequent_write_succeeds_on_crlf_file_without_trailing_newline - append_on_pre_existing_mixed_ending_file_is_rejected - append_concurrent_50_crlf_seed_no_trailing_newline_preserves_crlf Refs #3.
validate_line_endings was inlined in two places — write_guard.rs (byte level, only catching mixed CRLF+LF) and write.rs (str level, catching CR-only files, lone-\r in LF files, and mixed CRLF+LF). The two implementations diverged: the byte-level inline version missed the lone-\r cases. Extract validate_line_endings_bytes into o8v-fs so both call sites share the same logic and produce the same error messages. Also fixes the stale doc on guarded_append_with_separator that still described the replaced "last 2 bytes" approach.
xsoheilalizadeh
force-pushed
the
fix/crlf-line-ending-preservation
branch
from
d04d2c0 to
c86cedf
Compare
xsoheilalizadeh
changed the base branch from
fix/ci-restore-test-fixtures
to
main
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.
Summary
\nin content is normalized to\r\nwhen the file is CRLF.\nto\r\nin both find and replace strings when the file is CRLF, closing the silent-corruption gap where a\nin the replacement on a CRLF file produced mixed endings.e2e_write.rscovering Append, FindReplace, Insert, and Delete on CRLF files.Closes #3.
Stacked on #2 (the race fix added the flock infra this builds on). Will auto-retarget to main when #2 merges.
Test plan
cargo test -p o8v --test e2e_write— 47 pass, 0 failcargo test --workspace— no regressionscargo clippy --workspace -- -D warnings— clean