errbotio / errbot

Errbot is a chatbot, a daemon that connects to your favorite chat service and bring your tools and some fun into the conversation.
http://errbot.io
GNU General Public License v3.0
3.13k stars 615 forks source link

Slack message formatting issues #684

Closed Synforge closed 3 years ago

Synforge commented 8 years ago

It looks like there are a few cases where the markdown module is mangling Slack formatting that isn't accounted for in the test cases. Since the markdown conversion occurs outside of prepare_message_body it isn't picked up by testing.

In a lot of cases when you want to send a message with three backticks denoting a code block it is completely mangled by markdown before being sent via Slack e.g.

return "testing"
. The only way to accurately send this at the moment is by failing to close the backtick block which is then appended during the prepare_message_body call. e.g.

return "```testing"

Is there any actual reason that the markdown module is in use here? It seems like it would in many cases make sense to not push it through markdown?

Happy to make the changes this end and create a PR however I'm at a loss as to a way to fix this since it all occurs in markdown.

zoni commented 8 years ago

It looks like there are a few cases where the markdown module is mangling Slack formatting that isn't accounted for in the test cases. Since the markdown conversion occurs outside of prepare_message_body it isn't picked up by testing.

In all fairness, this problem isn't specific to Slack. :smile: Telegram for example has its own quirks too, as does the semi-official Skype backend.

In a lot of cases when you want to send a message with three backticks denoting a code block it is completely mangled by markdown before being sent via Slack e.g.

return "```testing```"

Could you elaborate on how markdown mangles this? It's possible I'm not doing the right thing to reproduce this, but it seems to render fine for me on Slack.

Is there any actual reason that the markdown module is in use here? It seems like it would in many cases make sense to not push it through markdown?

You should see Markdown as the "markup language" we picked to do message markup/formatting with. Errbot expects Markdown from plugins and processes it into the best possible representation it can do on the chat network it's connected to.

This makes it possible to write plugins in a backend-agnostic way using one common format, instead of having to use different templates for every single backend. So instead of having to supply XHTML-IM for XMPP, Markdown for Gitter, "Slack formatting" (a Markdown inspired but considerably more limited syntax) for Slack and plaintext for most others, you just write markdown once and we do our best to make it look nice on any network.

zoni commented 8 years ago

Also, in reference to markup getting mangled, I do know this can happen when a message is automatically split across multiple messages because it exceeds the size limits.

Synforge commented 8 years ago

Sure that was probably a stupid question regarding markdown!

It seems that if I pass through:

return "```testing```"

Slack renders this as an inline e.g.

`testing`
. Where I would expect it to do this:
```testing```
I did some testing at my end and it looks like it's converted to the single backtick equivalent after it's been run through Markdown. Additionally looking more at the markdown syntax I attempted to create an markdown template to then indent by 4 spaces which unfortunately didn't result in a code block in Slack.
gbin commented 8 years ago

Did you try

```\nsomething\n``` ?
Synforge commented 8 years ago

Yeah I noticed that in the character list for imtext but no, that doesn't work either, just results in plain text.

didiersmith commented 8 years ago

I also have run into this problem. My workaround was to escape the backticks with backslashes. Pretty gross, and also breaks down when the message is chunked.

Synforge commented 8 years ago

I found that if you just don't close the backticks this isn't parsed as anything in markdown then the auto closing portion of the code closes them for me which results in a properly formatted message. It's ugly though and will only work in Slack.

aschokking commented 8 years ago

Is there a way to opt out of markdown rendering of messages for a given plugin/response? Or if there isn't is that something that you guys would be open to having added/plumbed?

zoni commented 8 years ago

Is there a way to opt out of markdown rendering of messages for a given plugin/response?

Not from within our normal request/response cycles with returning/yielding responses from plugins. We've purposefully documented how the underlying API clients are exposed for exactly this purpose though. In the case of slack you could send a message manually using the respective API call, along the lines of:

self._bot.sc.api_call('chat.postMessage', data={
    'channel': "#general",
    'text': "My message text here",
    'unfurl_media': 'true',
    'as_user': 'true',
})

Or if there isn't is that something that you guys would be open to having added/plumbed?

Given that sending custom formatting which bypasses markdown rendering defeats the whole idea of errbot offering a generic, backend-agnostic interface to plugins and the above option is exposed and documented specifically for people who want to do backend-specific things that are non-portable, I would be hesitant to accept such an option.

aschokking commented 8 years ago

Thank you @zoni for the example of calling the API directly, that's enough of a work around for me.

I can understand the motivation behind the backend-agnostic interface and I definitely come to the table with a very slack-centric view and I love a lot of other features that err brings to the table just for that vs other frameworks. With that said, let me give you the scenario I ran into that led me to want to turn off the markdown rendering and maybe you could help me understand if there's a backend-agnostic way I could have gone about it?

I was attempting to reply with a message that tagged a series of people. This was the string that my method would return: "Okay we've got 4! Let's go <@nick1>, <@nick2>, <@nick3>, <@nick4>" However by the time it made it out of the processing pipeline what was sent to slack was: Okay we've got 4! Let's go @nick1 (mailto:@nick1), @nick2 (mailto:@nick2), @nick3 (mailto:@nick3), @nick4 (mailto:@nick4) And it seemed like the <> required for tagging on slack was what was throwing things off in rendering land. Is there a backend agnostic way I can tag users in a message that won't get mangled en route?

Thanks again.

zoni commented 8 years ago

This was the string that my method would return: "Okay we've got 4! Let's go @nick1, @nick2, @nick3, @nick4" However by the time it made it out of the processing pipeline what was sent to slack was: Okay we've got 4! Let's go @nick1 (mailto:@nick1), @nick2 (mailto:@nick2), @nick3 (mailto:@nick3), @nick4 (mailto:@nick4)

I think this is a seperate but related bug. On incoming messages, we receive user IDs and we convert these into readable usernames just as you would see them in your slack client.

When a plugin sends out a message like Hi @aschokking, how are you? errbot should transparently and automatically convert @aschokking into a proper ID.

I've opened a dedicated issue for this so we don't lose track of it: https://github.com/errbotio/errbot/issues/892

aschokking commented 8 years ago

Does that mean if I make the mentions of the form <U123456|zoni> in the outgoing message the right thing would happen?

zoni commented 8 years ago

I'm not entirely certain. If the markdown conversion step that happens in between your plugin returning data in that format and the backend sending the rendered markdown output on to slack doesn't mangle the angled brackets, then yes, it would.

rcoup commented 6 years ago

@zoni would you be open to having a Message.extras flag which opts-out of markdown conversion in Backend.send_message()? Allows any plugin to use carefully/custom-formatted output without having to drop all the other useful stuff that Errbot provides by going direct to the backend APIs.

ronendayan commented 6 years ago
\`\`\`test\`\`\`

That would work

sijis commented 3 years ago

No activity. Closing.