Introduce macro for ThingListBuilder, and use for AuthorityListBuilder
This is the first part of #447 (closed) which is part of #371 (closed) which is part of #285 (closed)
Merge request reports
Activity
assigned to @Diziet
added 1 commit
- 4452e101 - fixup! Introduce AuthorityListBuilder in NetworkConfigBuilder
marked this merge request as draft from Diziet/arti@4452e101
added 1 commit
- 4ad9e5e4 - Introduce AuthorityListBuilder in NetworkConfigBuilder
requested review from @nickm
Extended this to cover the predicted ports list, too, which means this MR would close #447 (closed)
marked this merge request as draft from Diziet/arti@ec005661
crates/tor-config/src/list_builder.rs 0 → 10064441 /// default = vec![]; 42 /// } 43 /// 44 /// let mut thinglist = ThingListBuilder::default(); 45 /// thinglist.append(ThingBuilder::default().value(42).clone()); 46 /// assert_eq!{ thinglist.build().unwrap().things, &[Thing { value: 42 }] } 47 /// 48 /// thinglist.set(vec![ThingBuilder::default().value(38).clone()]); 49 /// assert_eq!{ thinglist.build().unwrap().things, &[Thing { value: 38 }] } 50 /// ``` 51 #[macro_export] 52 macro_rules! define_list_config_builder { 53 { 54 [ 55 $($docs_and_attrs:tt)* 56 ] -
Um. Well, I had a bit set that that doesn't work right for doc comments. I was going to paste the error message. But I find that it does work. Maybe I was working on a project with an earlier MSRV.
I have added a commit to change this. LMK if you want me to break it up and fold it into the various places where this was introduced.
changed this line in version 8 of the diff
- crates/tor-config/src/list_builder.rs 0 → 100644
59 } 60 built: $Built:ty = $built:expr; 61 default = $default:expr; 62 } => { 63 $($docs_and_attrs)* 64 #[derive(Default, Clone, Deserialize)] 65 #[serde(transparent)] 66 /// 67 /// This is a builder pattern struct which will be resolved 68 /// during configuration resolution, via the `build` method. 69 pub struct $ListBuilder { 70 /// The list, as overridden 71 $field_vis $things: Option<Vec<$EntryBuilder>>, 72 } 73 impl $ListBuilder { 74 /// Add one changed this line in version 8 of the diff
- crates/tor-config/src/list_builder.rs 0 → 100644
60 built: $Built:ty = $built:expr; 61 default = $default:expr; 62 } => { 63 $($docs_and_attrs)* 64 #[derive(Default, Clone, Deserialize)] 65 #[serde(transparent)] 66 /// 67 /// This is a builder pattern struct which will be resolved 68 /// during configuration resolution, via the `build` method. 69 pub struct $ListBuilder { 70 /// The list, as overridden 71 $field_vis $things: Option<Vec<$EntryBuilder>>, 72 } 73 impl $ListBuilder { 74 /// Add one 75 pub fn append(&mut self, item: $EntryBuilder) -> &mut Self { -
I discussed the name with @eta. My conclusion was that
appendwas better; IIRC @eta didn't have a strong opinion.. We might want to add aprependat some point, and thenpushbecomes ambiguous (which is whyVecDequehaspush_frontandpush_back). Rust stdlib currently usesappendmostly for things that are basically theExtendtrait and I think this is anomalous. changed this line in version 8 of the diff
-
Hm; personally I'd expect to find
pushhere. My own thought is that if there's a method thatVecalso has, we should probably give it the same name asVecuses, since every Rust programmer will be familiar with theVecAPI.On the other hand, all the current use-cases for this kind of type are order-irrelevant: maybe
insertwould be a better name, to matchHashSet.I'm not going to insist either way here, since I hope that people will usually not need these APIs at all, so feel free to ignore this if you still disagree.
- crates/tor-config/src/list_builder.rs 0 → 100644
68 /// during configuration resolution, via the `build` method. 69 pub struct $ListBuilder { 70 /// The list, as overridden 71 $field_vis $things: Option<Vec<$EntryBuilder>>, 72 } 73 impl $ListBuilder { 74 /// Add one 75 pub fn append(&mut self, item: $EntryBuilder) -> &mut Self { 76 self.$things 77 .get_or_insert_with(|| $default) 78 .push(item); 79 self 80 } 81 82 /// Set the list to the supplied one, discarding any previous settings 83 pub fn set(&mut self, list: impl IntoIterator<Item = $EntryBuilder>) -> &mut Self { changed this line in version 8 of the diff
- crates/tor-config/src/list_builder.rs 0 → 100644
94 pub fn build(&self) -> Result<$Built, ConfigBuildError> { 95 let default_buffer; 96 let $things = match &self.$things { 97 Some($things) => $things, 98 None => { 99 default_buffer = $default; 100 &default_buffer 101 } 102 }; 103 let $things = $things 104 .iter() 105 .map(|item| item.build()) 106 .collect::<Result<_, _>>()?; 107 Ok($built) 108 } 109 } -
Yes. (After discussion with @eta) I felt it best to leave this for now.
clearcan be done withreplace(vec![]). If we want to addextendwe shouldimpl Extend.(Also, the semantics of
cleararen't super-obvious: does it reset it to "use the default list", or does it mean "definitely use an empty list plus anything I subsequently add". The former can be done withx = default().)
140 140 } 141 141 142 142 // ---------------------------------------------------------------------- 143 144 /// Helper for assisting with macro "argument" defaulting 145 /// 146 /// ```ignore 147 /// macro_coalesce_args!{ [ something ] ... } // => something 17 17 /// nontrivial, you should put the actual defaulting functionality in a (probably-private) 18 18 /// function, as the macro will expand it twice. 19 19 /// 20 /// The `item_build` clause, if supplied, provides a closure with type 21 /// `FnMut(&ThingBuilder) -> Result<Thing, ConfigBuildErro>`; the default is to call 22 /// `thing_builder.build()`.