-
Notifications
You must be signed in to change notification settings - Fork 205
text and markdown formats #1949
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
d1471a8
34bf4f5
db1dcc9
a1ff7c7
4bbb55d
f04f0d7
a7c8f2c
9da38c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| tests/unit/messages/formats/markdown/fixtures/* |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| import json | ||
| import re | ||
| from enum import Enum | ||
|
|
||
| from tabulate import tabulate | ||
|
|
||
| from elementary.messages.blocks import ( | ||
| ActionsBlock, | ||
| CodeBlock, | ||
| DividerBlock, | ||
| ExpandableBlock, | ||
| FactListBlock, | ||
| HeaderBlock, | ||
| Icon, | ||
| IconBlock, | ||
| InlineBlock, | ||
| InlineCodeBlock, | ||
| LineBlock, | ||
| LinesBlock, | ||
| LinkBlock, | ||
| MentionBlock, | ||
| TableBlock, | ||
| TextBlock, | ||
| TextStyle, | ||
| WhitespaceBlock, | ||
| ) | ||
| from elementary.messages.formats.unicode import ICON_TO_UNICODE | ||
| from elementary.messages.message_body import MessageBlock, MessageBody | ||
|
|
||
|
|
||
| class TableStyle(Enum): | ||
| TABULATE = "tabulate" | ||
| JSON = "json" | ||
|
|
||
|
|
||
| class MarkdownFormatter: | ||
| def __init__(self, table_style: TableStyle): | ||
| self._table_style = table_style | ||
|
|
||
| def format_icon(self, icon: Icon) -> str: | ||
| return ICON_TO_UNICODE[icon] | ||
|
|
||
| def format_text_block(self, block: TextBlock) -> str: | ||
| if block.style == TextStyle.BOLD: | ||
| return f"**{block.text}**" | ||
| elif block.style == TextStyle.ITALIC: | ||
| return f"_{block.text}_" | ||
| else: | ||
| return block.text | ||
|
|
||
| def format_inline_block(self, block: InlineBlock) -> str: | ||
| if isinstance(block, IconBlock): | ||
| return self.format_icon(block.icon) | ||
| elif isinstance(block, TextBlock): | ||
| return self.format_text_block(block) | ||
| elif isinstance(block, LinkBlock): | ||
| return f"[{block.text}]({block.url})" | ||
| elif isinstance(block, InlineCodeBlock): | ||
| return f"`{block.code}`" | ||
| elif isinstance(block, MentionBlock): | ||
| return block.user | ||
| elif isinstance(block, LineBlock): | ||
| return self.format_line_block(block) | ||
| elif isinstance(block, WhitespaceBlock): | ||
| return " " | ||
| else: | ||
| raise ValueError(f"Unsupported inline block type: {type(block)}") | ||
|
|
||
| def format_line_block(self, block: LineBlock) -> str: | ||
| return block.sep.join( | ||
| [self.format_inline_block(inline) for inline in block.inlines] | ||
| ) | ||
|
|
||
| def format_lines_block(self, block: LinesBlock) -> str: | ||
| formatted_parts = [] | ||
| for index, line_block in enumerate(block.lines): | ||
| formatted_line = self.format_line_block(line_block) | ||
| formatted_parts.append(formatted_line) | ||
| is_bullet = re.match(r"^\s*[*-]", formatted_line) | ||
| is_last = index == len(block.lines) - 1 | ||
| if not is_bullet and not is_last: | ||
| # in markdown, single line breaks are not rendered as new lines, except for bullet lists | ||
| # so we need to add a backslash to force a new line | ||
| formatted_parts.append("\\") | ||
| if not is_last: | ||
| formatted_parts.append("\n") | ||
| return "".join(formatted_parts) | ||
|
Comment on lines
+74
to
+87
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify the line break handling logic. The complex logic for handling line breaks and bullet lists appears correct for Markdown rendering, but this should be tested to ensure proper output formatting. 🏁 Script executed: #!/bin/bash
# Description: Search for existing tests of the MarkdownFormatter to verify line break handling
# Look for markdown formatter tests
fd -t f -e py test | xargs rg -l "MarkdownFormatter|format_lines_block" -A 5 -B 5Length of output: 83 🏁 Script executed: #!/bin/bash
# Search for the MarkdownFormatter class definition and usage across all Python files
rg -n "class .*MarkdownFormatter" -g "*.py" || echo "No class definition found"
rg -n "MarkdownFormatter" -g "*.py" || echo "No MarkdownFormatter references found"
rg -n "format_lines_block" -g "*.py" || echo "No format_lines_block references found"Length of output: 938 Add unit tests for MarkdownFormatter.format_lines_block There are currently no tests covering Please add tests that cover at least:
File to test:
🤖 Prompt for AI Agents |
||
|
|
||
| def format_fact_list_block(self, block: FactListBlock) -> str: | ||
| facts = [ | ||
| f"{self.format_line_block(fact.title)}: {self.format_line_block(fact.value)}" | ||
| for fact in block.facts | ||
| ] | ||
| return " | ".join(facts) | ||
|
|
||
| def format_table_block(self, block: TableBlock) -> str: | ||
| if self._table_style == TableStyle.TABULATE: | ||
| table = tabulate(block.rows, headers=block.headers, tablefmt="simple") | ||
| return f"```\n{table}\n```" | ||
| elif self._table_style == TableStyle.JSON: | ||
| dicts = [ | ||
| {header: cell for header, cell in zip(block.headers, row)} | ||
| for row in block.rows | ||
| ] | ||
| return f"```\n{json.dumps(dicts, indent=2)}\n```" | ||
| else: | ||
| raise ValueError(f"Invalid table style: {self._table_style}") | ||
|
|
||
| def format_expandable_block(self, block: ExpandableBlock) -> str: | ||
| body = self.format_message_blocks(block.body) | ||
| quoted_body = "\n> ".join(body.split("\n")) | ||
| return f"> **{block.title}**\\\n> {quoted_body}" | ||
|
|
||
| def format_message_block(self, block: MessageBlock) -> str: | ||
| if isinstance(block, HeaderBlock): | ||
| return f"# {block.text}" | ||
| elif isinstance(block, CodeBlock): | ||
| return f"```\n{block.text}\n```" | ||
| elif isinstance(block, LinesBlock): | ||
| return self.format_lines_block(block) | ||
| elif isinstance(block, FactListBlock): | ||
| return self.format_fact_list_block(block) | ||
| elif isinstance(block, ExpandableBlock): | ||
| return self.format_expandable_block(block) | ||
| elif isinstance(block, TableBlock): | ||
| return self.format_table_block(block) | ||
| elif isinstance(block, DividerBlock): | ||
| return "---" | ||
| elif isinstance(block, ActionsBlock): | ||
| # Actions not supported for text | ||
| return "" | ||
| else: | ||
| raise ValueError(f"Unsupported message block type: {type(block)}") | ||
|
|
||
| def format_message_blocks(self, blocks: list[MessageBlock]) -> str: | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainConsider Python version compatibility for type annotations. The 🏁 Script executed: #!/bin/bash
# Description: Check Python version requirements and existing type annotation patterns
# Check for Python version requirements
fd -t f pyproject.toml setup.py setup.cfg requirements.txt | xargs cat 2>/dev/null | rg -i "python.*version|python_requires"
# Check existing type annotation patterns in the codebase
rg "List\[|list\[" --type py -A 1 -B 1 | head -20Length of output: 1950 🏁 Script executed: #!/bin/bash
set -euxo pipefail
echo "Searching for project metadata files..."
# Find potential Python version specifiers
fd -HI pyproject.toml setup.py Pipfile | sed -e 's/^/FOUND: /'
# Display contents if found
for f in $(fd -HI pyproject.toml setup.py Pipfile); do
echo "=== $f ==="
head -n50 "$f"
done
echo
echo "Checking GitHub Actions for python-version..."
rg -n "python-version" -t yaml .github/workflows || echo "No python-version keys found in workflows"
echo
echo "Checking for Travis CI configuration..."
if [ -f .travis.yml ]; then
echo "=== .travis.yml ==="
head -n30 .travis.yml
else
echo "No .travis.yml"
fi
echo
echo "Checking for runtime.txt..."
if [ -f runtime.txt ]; then
echo "=== runtime.txt ==="
head -n10 runtime.txt
else
echo "No runtime.txt"
fi
echo
echo "Looking for Dockerfiles with Python base images..."
rg -n "FROM python" -t docker || echo "No Dockerfiles using Python base image"
echo
echo "Searching for Poetry configuration..."
rg -R "tool.poetry" -t toml || echo "No Poetry metadata found"Length of output: 324 Switch to typing.List for consistency and Python <3.9 compatibility It looks like the rest of the codebase (and your tests) still use • File elementary/messages/formats/markdown.py 🤖 Prompt for AI Agents |
||
| if not blocks: | ||
| return "" | ||
| return "\n\n".join([self.format_message_block(block) for block in blocks]) | ||
|
|
||
| def format(self, message: MessageBody) -> str: | ||
| return self.format_message_blocks(message.blocks) | ||
|
|
||
|
|
||
| def format_markdown( | ||
| message: MessageBody, table_style: TableStyle = TableStyle.TABULATE | ||
| ) -> str: | ||
| formatter = MarkdownFormatter(table_style) | ||
| return formatter.format(message) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,140 @@ | ||
| import json | ||
| from enum import Enum | ||
| from typing import List | ||
|
|
||
| from tabulate import tabulate | ||
|
|
||
| from elementary.messages.blocks import ( | ||
| ActionsBlock, | ||
| CodeBlock, | ||
| DividerBlock, | ||
| ExpandableBlock, | ||
| FactListBlock, | ||
| HeaderBlock, | ||
| Icon, | ||
| IconBlock, | ||
| InlineBlock, | ||
| InlineCodeBlock, | ||
| LineBlock, | ||
| LinesBlock, | ||
| LinkBlock, | ||
| MentionBlock, | ||
| TableBlock, | ||
| TextBlock, | ||
| WhitespaceBlock, | ||
| ) | ||
| from elementary.messages.formats.unicode import ICON_TO_UNICODE | ||
| from elementary.messages.message_body import MessageBlock, MessageBody | ||
|
|
||
|
|
||
| class IconStyle(Enum): | ||
| UNICODE = "unicode" | ||
| NAME = "name" | ||
| OMIT = "omit" | ||
|
|
||
|
|
||
| class TableStyle(Enum): | ||
| TABULATE = "tabulate" | ||
| JSON = "json" | ||
|
|
||
|
|
||
| class TextFormatter: | ||
| def __init__(self, icon_style: IconStyle, table_style: TableStyle): | ||
| self._icon_style = icon_style | ||
| self._table_style = table_style | ||
|
|
||
| def format_icon(self, icon: Icon) -> str: | ||
| if self._icon_style == IconStyle.OMIT: | ||
| return "" | ||
| elif self._icon_style == IconStyle.UNICODE: | ||
| return ICON_TO_UNICODE[icon] | ||
| elif self._icon_style == IconStyle.NAME: | ||
| return f":{icon.value}:" | ||
| else: | ||
| raise ValueError(f"Invalid icon style: {self._icon_style}") | ||
|
|
||
| def format_inline_block(self, block: InlineBlock) -> str: | ||
| if isinstance(block, IconBlock): | ||
| return self.format_icon(block.icon) | ||
| elif isinstance(block, TextBlock): | ||
| return block.text | ||
| elif isinstance(block, LinkBlock): | ||
| return f"{block.text} ({block.url})" | ||
| elif isinstance(block, InlineCodeBlock): | ||
| return block.code | ||
| elif isinstance(block, MentionBlock): | ||
| return block.user | ||
| elif isinstance(block, LineBlock): | ||
| return self.format_line_block(block) | ||
| elif isinstance(block, WhitespaceBlock): | ||
| return " " | ||
| else: | ||
| raise ValueError(f"Unsupported inline block type: {type(block)}") | ||
|
|
||
| def format_line_block(self, block: LineBlock) -> str: | ||
| return block.sep.join( | ||
| [self.format_inline_block(inline) for inline in block.inlines] | ||
| ) | ||
|
|
||
| def format_lines_block(self, block: LinesBlock) -> str: | ||
| return "\n".join( | ||
| [self.format_line_block(line_block) for line_block in block.lines] | ||
| ) | ||
|
|
||
| def format_fact_list_block(self, block: FactListBlock) -> str: | ||
| facts = [ | ||
| f"{self.format_line_block(fact.title)}: {self.format_line_block(fact.value)}" | ||
| for fact in block.facts | ||
| ] | ||
| return " | ".join(facts) | ||
|
|
||
| def format_table_block(self, block: TableBlock) -> str: | ||
| if self._table_style == TableStyle.TABULATE: | ||
| return tabulate(block.rows, headers=block.headers, tablefmt="simple") | ||
| elif self._table_style == TableStyle.JSON: | ||
| dicts = [ | ||
| {header: cell for header, cell in zip(block.headers, row)} | ||
| for row in block.rows | ||
| ] | ||
| return json.dumps(dicts, indent=2) | ||
| else: | ||
| raise ValueError(f"Invalid table style: {self._table_style}") | ||
|
|
||
| def format_expandable_block(self, block: ExpandableBlock) -> str: | ||
| return f"{block.title}\n{self.format_message_blocks(block.body)}" | ||
|
|
||
| def format_message_block(self, block: MessageBlock) -> str: | ||
| if isinstance(block, (HeaderBlock, CodeBlock)): | ||
| return block.text | ||
| elif isinstance(block, LinesBlock): | ||
| return self.format_lines_block(block) | ||
| elif isinstance(block, FactListBlock): | ||
| return self.format_fact_list_block(block) | ||
| elif isinstance(block, ExpandableBlock): | ||
| return self.format_expandable_block(block) | ||
| elif isinstance(block, TableBlock): | ||
| return self.format_table_block(block) | ||
| elif isinstance(block, ActionsBlock): | ||
| # Actions not supported for text | ||
| return "" | ||
| elif isinstance(block, DividerBlock): | ||
| return "--------------------------------" | ||
| else: | ||
| raise ValueError(f"Unsupported message block type: {type(block)}") | ||
|
|
||
| def format_message_blocks(self, blocks: List[MessageBlock]) -> str: | ||
| if not blocks: | ||
| return "" | ||
| return "\n".join([self.format_message_block(block) for block in blocks]) | ||
|
Comment on lines
+125
to
+128
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify block separation behavior for text format. The method joins message blocks with single newlines, which differs from the markdown formatter that uses double newlines ( 🏁 Script executed: #!/bin/bash
# Check if there are any tests that verify the block separation behavior
ast-grep --pattern 'format_message_blocks($$$)' | head -10
rg -A 5 -B 5 'format_message_blocks.*join'Length of output: 325 Add unit tests to verify single-newline separation in TextFormatter No existing tests cover
🤖 Prompt for AI Agents |
||
|
|
||
| def format(self, message: MessageBody) -> str: | ||
| return self.format_message_blocks(message.blocks) | ||
|
|
||
|
|
||
| def format_text( | ||
| message: MessageBody, | ||
| icon_style: IconStyle = IconStyle.UNICODE, | ||
| table_style: TableStyle = TableStyle.TABULATE, | ||
| ) -> str: | ||
| formatter = TextFormatter(icon_style, table_style) | ||
| return formatter.format(message) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Replace HTML entity with Markdown-appropriate whitespace.
Using
is HTML syntax, not standard Markdown. Consider using regular spaces or Markdown-specific non-breaking space alternatives.For non-breaking space in Markdown, you could also use multiple spaces or consider if whitespace blocks need special handling at all.
📝 Committable suggestion
🤖 Prompt for AI Agents