Skip to content

feat(formatting) allow empty string in result to imply code format #81

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ianhi
Copy link
Contributor

@ianhi ianhi commented Mar 1, 2025

Sometimes for formatting I found it preferable to have the output wrapped in a code block. In particular for when I had many python print statements. A hacky way to do this is to pass an invalid language name. e.g. result="code" but that is not super intuitive. Passing result="" was the first thing I tried to get the rendering to work properly so this PR offers that as an option. There is likely a better alternative, but I couldn't immediately think of it.

(I didn't use result="python" because that can give confusing syntax highlighting in the print statements.

Copy link
Owner

@pawamoy pawamoy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey again @ianhi, thanks for this other PR 🚀 I'm not entirely sold on the idea, as it's usually better to actually provide a language name. For code blocks without syntax highlighting, you'd typically use text. But given this change is very small, lets merge it.

@@ -92,7 +92,7 @@ def base_format(
md: Markdown,
html: bool = False,
source: str = "",
result: str = "",
result: str = None,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
result: str = None,
result: str | None = None,

result="",
md=md,
)
assert "<code>" in markup
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like it should assert a bit more. Were you able to confirm the test didn't pass without the change?

@ianhi
Copy link
Contributor Author

ianhi commented Mar 2, 2025

For code blocks without syntax highlighting, you'd typically use text

Maybe a better change here is to add something to that effect to the documentation. If using "text" is pretty standard

@pawamoy
Copy link
Owner

pawamoy commented Mar 4, 2025

You're right, lets just document that then 🙂 You can update this PR or update a new one (or neither and I'll take care of it), as you wish 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants