Skip to content

Conversation

g-arjones
Copy link
Contributor

rich.markup is used to escape strings when rendering help texts.

This fixes a regression introduced by #1128

g-arjones and others added 2 commits September 3, 2025 21:19
rich.markup is used to escape strings when rendering help texts.

This fixes a regression introduced by fastapi#1128
@svlandeg svlandeg changed the title Make sure 'rich.markup' is imported when rendering help text 🐛 Make sure rich.markup is imported when rendering help text Sep 5, 2025
@svlandeg svlandeg self-assigned this Sep 5, 2025
@svlandeg svlandeg added the bug Something isn't working label Sep 5, 2025
Copy link
Member

@svlandeg svlandeg left a comment

Choose a reason for hiding this comment

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

So just to clarify some edits I just made: The original content of the PR imported rich.markup before using escape, because due to the lazy imports from #1128 this module wasn't available.

I've rewritten this and moved the escaping functionality to rich_utils. My proposal is to never use rich directly in the core library, to isolate all this functionality within rich_utils and avoid any other future regressions.

There's a few more edits and maybe an additional test I'd like to add to this PR, so I'll put this in draft while I continue to work on it.

And thanks for the report & PR, @g-arjones ! 🙏

@svlandeg svlandeg marked this pull request as draft September 5, 2025 09:44
@svlandeg
Copy link
Member

svlandeg commented Sep 5, 2025

@g-arjones: do you have a minimal example that crashes on master?

@g-arjones
Copy link
Contributor Author

g-arjones commented Sep 5, 2025

@svlandeg Sure, here you go:

from typing import Annotated

import typer


app = typer.Typer(rich_markup_mode=None)


@app.command()
def foo(bar: Annotated[str, typer.Argument(help='foobar')]) -> None:
    """foobar."""
    pass


app()

Then run with --help

@svlandeg
Copy link
Member

svlandeg commented Sep 5, 2025

Thanks! This has been a bit tricky to get into the test suite, because other tests do import rich.markup at some point, which is why we didn't catch this regression earlier. I've now added a test that catches your originally reported regression on master, and that should go green on this PR.

I will add one more edit to this PR to fix a similar issue with accessing rich.traceback directly in main.py.

@g-arjones
Copy link
Contributor Author

Thanks for taking the time!

@svlandeg svlandeg marked this pull request as ready for review September 5, 2025 14:50
Copy link
Member

@tiangolo tiangolo left a comment

Choose a reason for hiding this comment

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

Thanks for the report and fix! 🍰

And thanks @svlandeg for the tweaks, confirmation, test... 🙌

This will be available in Typer 0.17.4 released in the next few minutes. 🎉

@tiangolo tiangolo merged commit 263e427 into fastapi:master Sep 5, 2025
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants