docs(iconv): design minimal FilenameConverter trait (#1910)#3517
Merged
Conversation
Audit confirms FilenameConverter already exists at crates/protocol/src/iconv/converter.rs as a concrete struct backed by encoding_rs. Task #1910 reduces to confirm-and-document plus three minor refinement recommendations (rename byte-oriented Cow methods to convert_send/convert_recv, surface unsupported-charset errors instead of swallowing them, and align with #1915 on the no-feature stub). Documents the three application sites (sender flist emit, receiver flist ingest, filter-rule path matching), justifies the struct-not- trait choice, the encoding_rs backing, and the protocol-crate home, and lays out the wiring plan that unblocks #1911-#1919.
oferchen
added a commit
that referenced
this pull request
May 5, 2026
Audit confirms FilenameConverter already exists at crates/protocol/src/iconv/converter.rs as a concrete struct backed by encoding_rs. Task #1910 reduces to confirm-and-document plus three minor refinement recommendations (rename byte-oriented Cow methods to convert_send/convert_recv, surface unsupported-charset errors instead of swallowing them, and align with #1915 on the no-feature stub). Documents the three application sites (sender flist emit, receiver flist ingest, filter-rule path matching), justifies the struct-not- trait choice, the encoding_rs backing, and the protocol-crate home, and lays out the wiring plan that unblocks #1911-#1919.
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.
Summary
Audit confirms a
FilenameConverteralready exists atcrates/protocol/src/iconv/converter.rsas a concrete struct backed byencoding_rs. Task #1910 therefore reduces to a confirm-and-documentexercise, with three minor refinement recommendations rather than
mandates:
Cowmethods (local_to_remote/remote_to_local) to align with the tracker description(
convert_send/convert_recv) and promote&Path/Cow<Path>to the public boundary.
rsync_error!(1, ...)rather than the current silent
tracing::warn!->Nonefallback.when
--iconvis supplied without theiconvCargo feature).The audit also documents the three application sites that need a
converter (sender flist emit, receiver flist ingest, filter-rule path
matching), justifies the struct-not-trait recommendation, defends
encoding_rsover system iconv on cross-platform / unsafe-policygrounds, and confirms
crates/protocol/src/iconv/as the correcthome crate.
This is the design follow-on to PR #3514
(
docs/audits/iconv-parse-deadend.md) and unblocks the wiring tasks#1911-#1919.
Test plan
Related
docs/audits/iconv-parse-deadend.md(PR docs(iconv): audit IconvSetting parse-path dead-end (#1909) #3514)docs/audits/iconv-inert.mddocs/audits/iconv-pipeline.mddocs/audits/iconv-feature-design.md