wr-org / webex-rust

Webex Teams for Rust
12 stars 9 forks source link

Makdown message issues #4

Closed My- closed 3 years ago

My- commented 3 years ago

Hi, I wanted the bot to send a code snippet and noticed some issues. Well, here is a good chance I do something wrong if I do I'm sorry. Steps: GIVEN I modified an example auto-replay to send another message as such (after line 56):

                                let mut rep = webex::types::MessageOut::from(&msg);
                                rep.markdown = Some(format!(r###"
                                ```json
                                {:#?}
                            # hi :)
                            `code`

                            "###, activity));
                            println!("{:#?}", &rep);
                            dbg!(webex.send_message(&rep).await.unwrap());
**WHEN** I run it
**AND** send message to the bot (or mention it)
**THEN** Teams client shows:
<img width="552" alt="Screenshot 2020-12-29 at 23 28 46" src="https://user-images.githubusercontent.com/10456471/103320564-b2e0e900-4a2d-11eb-9bae-cf9ac86e1f3f.png">
**AND** Console logs (I believe important one):
```bash
[examples/auto-reply.rs:67] webex.send_message(&rep).await.unwrap() = Message {
    id: Some(
//...
 html: Some(
        "<pre><code>                                Activity {\n    id: <...> vector_counters: Some(\n        VectorCounters {\n            source_dc: \"a-\",\n            counters: {\n                \"a-\": 50,\n            },\n        },\n    ),\n}\n                                ```\n\n                                # hi :)\n                                `code`</code></pre>",
    ),

As you can see client shows as code but do not stop at the last triple-back-tick. Note: the first triple-back-tick is not displayed inside the client. Sorry for a bit messy report.

My- commented 3 years ago

I believe I find why it happens. The reason is the spaces before the triple-back-tic. If I remove indentation (move multiline string close to the age) code looks ugly but all works as expected.

Not sure if it could be fixed (maybe by implementing own Serialize), but this SO post might have an idea how it could be done.

I would like to contribute, well at least try to contribute (I still learning Rust), to this issue fix if you need help.

PS. do you have any plans to make the lib.rs file smaller? eg brake it into couple files?

aknarts commented 3 years ago

This is how Webex Teams does Markdown parsing the triple tick needs to be on the line by itself. I am not sure if forcing the format on the library side would be the expected behavior. It takes the string as you put it in, which is the "problematic" part in your code.

The SO that you posted would be how you would clear this on the side of your code instead of the side of the lib. The lib just sends whatever text you want to sent through Teams.

As for breaking the lib into smaller files, it can probably be done, not sure if either me or Scott have time to do it though. What is your reasoning behind doing so? Also the lib.rs is not the problematic one, I hate the adaptive_card.rs more :smile:.

Anyway feel free to create the PR and we can take a look at it(even if it is just a proposal, issues can be worked out).

My- commented 3 years ago

What is your reasoning behind doing so?

Personal preferences, I don't like big files :) to me, over 200-300 lines is a big file. If I find a proper way to split it Ill rise a PR, but same as you I don't have much free time too.

I plan to build a bot maybe even a couple of them 🤞 if I find any issues/questions/suggestions I will create issue/PR accordingly.

Thanks for your replay :)

aknarts commented 3 years ago

Yes feel free. Most of the changes to this crate come in naturally as we use it in the wild.

I agree that big files might be nicer broken up but if it is one logical piece of code any split might add complexity(not saying this is the case here, I have not tried cleaning the crate up). To be fair what we lack more is more examples and possibly improvements in documentation might be nice.

My- commented 3 years ago

Yesterday I spend some time messing around with adaptive cards. I might add one simple example related to it looks simple enough. Prob I need Open issue for it so the related discussion could be moved there.

I noticed what Webex now supports adaptive card 1.2 version. But what I found is even some 1.1 features are not supported eg Media. Unless I did something wrong...

On that subject I was thinking: would it make more sense to move the adaptive card to separate crate all together? actually yesterday I start working towards it but I believe need mentoring as I faced few issues with serializing few fields. I was trying to add missing fields and upgrade to 1.2 version. I prob should create a separate issue for this discussion too.