mrsbfh.meta

Issues/PRs archive for MTRNord/mrsbfh
git clone git://archive.git.mtrnord.blog/MTRNord/mrsbfh.meta.git
Log | Files | Refs

13.md (8309B)


      1 # PR #13 Add a way to add custom state and update sdk
      2 
      3 - **Status:** closed
      4 - **Author:** @MTRNord
      5 - **Created:** 2022-03-12T18:54:50Z
      6 - **Branch:** MTRNord/issue-12 → main
      7 - **Closed:** 2025-06-16T22:41:06Z
      8 - **Labels:** enhancement
      9 - **Assignees:** @MTRNord
     10 - **Diff:** [13.diff](./13.diff)
     11 
     12 ---
     13 
     14 This is heavily building upon what http and axum do. In fact there is a lot of reused code.
     15 
     16 This allows for function defined like this:
     17 
     18 ```rust
     19 #[command(help = "`!hello_world` - Prints \"hello world\".")]
     20 pub async fn hello_world<'a>(
     21     Extension(tx): Extension<mrsbfh::Sender>,
     22 ) -> Result<(), Error> {
     23     let content = RoomMessageEventContent::notice_plain("Hello World!");
     24 
     25     tx.lock().await.send(content).await?;
     26     Ok(())
     27 }
     28 ```
     29 
     30 Downsides: 
     31 
     32 - It adds a Mutex to mutable stuff.
     33 - It needs stuff to be strictly async (I guess that was needed before too?)
     34 - A little more verbose?
     35 
     36 Pros:
     37 
     38 - You can pass anything to it as long as it is defined in your sync handler. (the macro auto extracts the arguments from it)
     39 - You don't need to pass all things to every command
     40 
     41 Fixes #12 
     42 
     43 Missing is:
     44 
     45 - [x] Getting user supplied things from the sync handler and passing it on.
     46 
     47 Tests seem to work:
     48 
     49 ![image](https://user-images.githubusercontent.com/1374914/158032715-da14ef16-c5c7-4c2e-9433-32ac62971a5c.png)
     50 
     51 
     52 
     53 ## Review comments
     54 
     55 ### @MTRNord on `mrsbfh-macros/src/lib.rs`:275 — 2022-03-12T18:57:36Z
     56 
     57 ```diff
     58 @@ -267,15 +266,18 @@ pub fn commands(_: TokenStream, input: TokenStream) -> TokenStream {
     59                             tracing::info!("Got command: {}", command);
     60                          }
     61                          // Make sure this is immutable
     62 -                        let args: Vec<&str> = split.collect();
     63 +                        let args_raw: Vec<String> = split.collect();
     64 +                        let args: std::sync::Arc<Vec<String>> = std::sync::Arc::new(args_raw.clone());
     65 +                        let tx = std::sync::Arc::new(std::sync::Mutex::new(tx));
     66 +
     67 +                        let mut msg = mrsbfh::commands::Message::new();
     68 +                        // TODO insert all the things in the function args
     69 +                        msg.extensions_mut().insert(std::sync::Arc::clone(&args));
     70 ```
     71 
     72 @donicrosby This is basically where now your wish comes in. I basically need to do this dynamically for all the things that the function provides when you use the macro. This is currently still hardcoded to only do the config. But the example bot should give you an idea how the api will look like. Which means it should be fairly similiar as before with not too many changes needed
     73 
     74 ### @MTRNord on `mrsbfh-macros/src/lib.rs`:275 — 2022-03-12T19:28:20Z
     75 
     76 ```diff
     77 @@ -267,15 +266,18 @@ pub fn commands(_: TokenStream, input: TokenStream) -> TokenStream {
     78                             tracing::info!("Got command: {}", command);
     79                          }
     80                          // Make sure this is immutable
     81 -                        let args: Vec<&str> = split.collect();
     82 +                        let args_raw: Vec<String> = split.collect();
     83 +                        let args: std::sync::Arc<Vec<String>> = std::sync::Arc::new(args_raw.clone());
     84 +                        let tx = std::sync::Arc::new(std::sync::Mutex::new(tx));
     85 +
     86 +                        let mut msg = mrsbfh::commands::Message::new();
     87 +                        // TODO insert all the things in the function args
     88 +                        msg.extensions_mut().insert(std::sync::Arc::clone(&args));
     89 ```
     90 
     91 Pushed the rest now. So in theory this PR should work. It compiles but I didnt actually test if it works in practice
     92 
     93 ### @MTRNord on `mrsbfh-macros/src/lib.rs`:275 — 2022-03-12T19:46:34Z
     94 
     95 ```diff
     96 @@ -267,15 +266,18 @@ pub fn commands(_: TokenStream, input: TokenStream) -> TokenStream {
     97                             tracing::info!("Got command: {}", command);
     98                          }
     99                          // Make sure this is immutable
    100 -                        let args: Vec<&str> = split.collect();
    101 +                        let args_raw: Vec<String> = split.collect();
    102 +                        let args: std::sync::Arc<Vec<String>> = std::sync::Arc::new(args_raw.clone());
    103 +                        let tx = std::sync::Arc::new(std::sync::Mutex::new(tx));
    104 +
    105 +                        let mut msg = mrsbfh::commands::Message::new();
    106 +                        // TODO insert all the things in the function args
    107 +                        msg.extensions_mut().insert(std::sync::Arc::clone(&args));
    108 ```
    109 
    110 One issue is that mutex stuff needs to already be `Arc<Mutex<>>` to work. It should however then clone the arc as required
    111 
    112 ### @donicrosby on `mrsbfh-macros/src/lib.rs`:275 — 2022-03-12T20:24:01Z
    113 
    114 ```diff
    115 @@ -267,15 +266,18 @@ pub fn commands(_: TokenStream, input: TokenStream) -> TokenStream {
    116                             tracing::info!("Got command: {}", command);
    117                          }
    118                          // Make sure this is immutable
    119 -                        let args: Vec<&str> = split.collect();
    120 +                        let args_raw: Vec<String> = split.collect();
    121 +                        let args: std::sync::Arc<Vec<String>> = std::sync::Arc::new(args_raw.clone());
    122 +                        let tx = std::sync::Arc::new(std::sync::Mutex::new(tx));
    123 +
    124 +                        let mut msg = mrsbfh::commands::Message::new();
    125 +                        // TODO insert all the things in the function args
    126 +                        msg.extensions_mut().insert(std::sync::Arc::clone(&args));
    127 ```
    128 
    129 That shouldn't be too difficult, I think that's fairly standard for something like this.
    130 
    131 Thanks for the quick fix! I'll test it out later tonight! 
    132 
    133 ### @MTRNord on `mrsbfh-macros/src/lib.rs`:275 — 2022-03-12T20:25:00Z
    134 
    135 ```diff
    136 @@ -267,15 +266,18 @@ pub fn commands(_: TokenStream, input: TokenStream) -> TokenStream {
    137                             tracing::info!("Got command: {}", command);
    138                          }
    139                          // Make sure this is immutable
    140 -                        let args: Vec<&str> = split.collect();
    141 +                        let args_raw: Vec<String> = split.collect();
    142 +                        let args: std::sync::Arc<Vec<String>> = std::sync::Arc::new(args_raw.clone());
    143 +                        let tx = std::sync::Arc::new(std::sync::Mutex::new(tx));
    144 +
    145 +                        let mut msg = mrsbfh::commands::Message::new();
    146 +                        // TODO insert all the things in the function args
    147 +                        msg.extensions_mut().insert(std::sync::Arc::clone(&args));
    148 ```
    149 
    150 Sure no problem :)
    151 
    152 ### @MTRNord on `mrsbfh-macros/src/lib.rs`:275 — 2022-03-12T20:25:34Z
    153 
    154 ```diff
    155 @@ -267,15 +266,18 @@ pub fn commands(_: TokenStream, input: TokenStream) -> TokenStream {
    156                             tracing::info!("Got command: {}", command);
    157                          }
    158                          // Make sure this is immutable
    159 -                        let args: Vec<&str> = split.collect();
    160 +                        let args_raw: Vec<String> = split.collect();
    161 +                        let args: std::sync::Arc<Vec<String>> = std::sync::Arc::new(args_raw.clone());
    162 +                        let tx = std::sync::Arc::new(std::sync::Mutex::new(tx));
    163 +
    164 +                        let mut msg = mrsbfh::commands::Message::new();
    165 +                        // TODO insert all the things in the function args
    166 +                        msg.extensions_mut().insert(std::sync::Arc::clone(&args));
    167 ```
    168 
    169 If you have any bugs feel free to mention them in this PR :) I only ran the example bot as I have no other to test with currently. So there may be stuff i missed
    170 
    171 
    172 ## Comments
    173 
    174 ### @MTRNord — 2022-03-12T19:54:27Z
    175 
    176 Followup issue: #14 
    177 
    178 ### @MTRNord — 2022-03-13T20:13:23Z
    179 
    180 Some feedback from jplatte:
    181 
    182 > One thing I noticed is that you seem to have an automatic 'extension' of Arc<Mutex<mrsbfh::Sender>>. I would recomment instead making the Arc<Mutex<_>> part internal so that the sender is Clone + Send + Sync and you make it an extractor by itself (no Extension wrapping needed)
    183 
    184 > Extension can still make sense if you want users to be able to add their own context
    185 
    186 > In the SDK the Ctx type and register_event_handler_context fills the same role
    187 
    188 ### @MTRNord — 2024-05-24T23:10:22Z
    189 
    190 Main missing TODO is now fixing the comments I think
    191