CfgPath overhaul
- Make CfgPath Serialize (as per #371 (closed))
- Fix deserialisation to be unambiguous (fixes #449 (closed))
- Overhaul the Rust API
Part of #285 (closed)
Merge request reports
Activity
assigned to @Diziet
requested review from @eta
37 37 enum PathInner { 38 /// A path that should be used literally, with no expansion. 39 Literal(LiteralPath), 38 40 /// A path that should be expanded from a string using ShellExpand. 39 41 Shell(String), 40 /// A path that should be used literally, with no expansion. 41 Literal(PathBuf), 42 } 43 44 #[derive(Clone, Debug, Deserialize, Eq, PartialEq)] 45 /// Inner implementation of PathInner:Literal 46 /// 47 /// `LiteralPath` exists to arrange that `PathInner::Literal`'s (de)serialization 48 /// does not overlap with `PathInner::Shell`'s. 49 struct LiteralPath { 50 /// 
Indeed, it is. Let me quote the struct:
#[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)] /// Inner implementation of PathInner:Literal /// /// `LiteralPath` exists to arrange that `PathInner::Literal`'s (de)serialization /// does not overlap with `PathInner::Shell`'s. struct LiteralPath { /// literal: PathBuf, }I felt that adding "the literal path" to the literal path in the literal path would be otiose. Indeed I'm getting semantic bleaching writing this. In general, do you dislike empty doc comments in situations where the only thing one would write would be to recapitulate the field name and type?
Or maybe you just wanted to ask if I had done this deliberately. In which case, "yes" :-). I write "XXX" for "I need to write something here but haven't yet".
added 21 commits
-
1888c17f...43266ee3 - 16 commits from branch
tpo/core:main - a150d53f - config: Enable "toml" feature
- ae776392 - CfgPath: Change deserialisaation of Literal variant
- ed970310 - CfgPath: Overhaul API
- 1e6c6169 - CfgPath: Make it Serialize
- 2da84857 - CfgPath: Test serialisation round-trip with a binary format
Toggle commit list-
1888c17f...43266ee3 - 16 commits from branch
-
Thanks for the review. I have rebased this onto main, to resolve the conflict, and while I was there I squashed the minimal-versions fixup.
I had one question about your empty doc comment observation. Since you've "approved", I will go ahead and set this to merge. If your comment was an objection please let me know...
Thanks, Ian.
enabled an automatic merge when the pipeline for 2da84857 succeeds
mentioned in commit fbf5e8dc
mentioned in merge request !482 (merged)