Fix code blocks in Slack messages#1182
Conversation
Remove the language marker on code blocks. Slack does not support this causing extraneous text to be displayed.
There was a problem hiding this comment.
Pull Request Overview
This PR fixes code blocks in Slack messages by removing language markers that Slack doesn't support, and refactors the message handling logic into a new SlackMessage class for better organization and reusability.
- Introduces a new
transform_code_blockfunction to strip language specifiers from markdown code blocks - Refactors message transformations into a centralized
SlackMessageclass with JSON serialization - Updates existing functions to use the new
SlackMessageclass instead of inline transformations
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| examples/slackbot/src/slackbot/slack.py | Adds SlackMessage class and transform_code_block function, refactors existing functions to use the new class |
| examples/slackbot/tests/test_slack_message_class.py | Comprehensive unit tests for the new SlackMessage class functionality |
| examples/slackbot/.dockerignore | Excludes test files from Docker builds |
| ``` | ||
| """ | ||
| # Remove the language specifier from the code block | ||
| code_block_pattern = r"```(?:\w+)?(.*?)```" |
There was a problem hiding this comment.
The regex pattern doesn't properly handle the optional language specifier. The (?:\w+)? makes the language specifier optional but doesn't account for newlines after it. This will fail to match code blocks with language specifiers followed by newlines. Consider using r"```(?:\w+\n)?(.*?)```" to properly capture language specifiers followed by newlines.
| code_block_pattern = r"```(?:\w+)?(.*?)```" | |
| code_block_pattern = r"```(?:\w+\n)?(.*?)```" |
| "https://slack.com/api/chat.update", | ||
| headers={"Authorization": f"Bearer {settings.slack_api_token}"}, | ||
| json={"channel": channel_id, "ts": thread_ts, "text": updated_text}, | ||
| json=message.json(), |
There was a problem hiding this comment.
The message.json() call will exclude the ts field needed for updating messages. The edit_slack_message function needs to pass the ts parameter to identify which message to update, but the current SlackMessage instance doesn't have the ts set when created with SlackMessage(new_text) in replace mode.
| delimiter = "\n\n" if delimiter is None else delimiter | ||
| updated_text = f"{current_text}{delimiter}{convert_md_links_to_slack(new_text)}" | ||
| message = await SlackMessage.from_message(channel_id, thread_ts) | ||
| message.append(new_text, delimiter=delimiter) |
There was a problem hiding this comment.
The delimiter parameter is used without being defined in the function signature. The function signature shows delimiter as a parameter, but it's not included in the actual parameter list, which will cause a NameError when delimiter is None and needs to default to "\n\n".
SlackMessageclass