typer icon indicating copy to clipboard operation
typer copied to clipboard

🐛 Fix escaping in help text when `rich` is installed but not used

Open svlandeg opened this issue 1 year ago • 2 comments

Background

With PR #847, we implemented functionality to ensure that help descriptions with Rich tags (e.g. [bold]) would get rendered properly in Markdown (instead of just showing the "raw" brackets). This uses Rich's functionality from console.export_html.

Unfortunately, a problem occurred at the time because meta information like [required] looks like Rich tags, but with no known formatting they would just get stripped off by console.export_html(). That then meant we had to escape those tags at the point in the code where they're being added to the rest of the help string - this happens in get_help_record for both TyperOption and TyperArgument :

if rich is not None:
    # This is needed for when we want to export to HTML
    extra_str = rich.markup.escape(extra_str).strip()

help = f"{help}  {extra_str}" if help else f"{extra_str}"

Current issue

Now, as pointed out in #1058, there's now an issue when the escaping happens if rich is installed (triggering the if clause in the code above) BUT the Typer app actually has rich_markup_mode=None. The help text will in this case show the backslash from the escaping, which is clearly unwanted behaviour.

Fix (proposal)

To remedy, we need to know at the time of escaping what the value is of rich_markup_mode. This is not trivial, as TyperOption and TyperArgument are unaware of this information. And we can't just pass it through from the Typer app either, because get_click_param() creates empty/default OptionInfo/ArgumentInfo objects which will lead to default values downstream. I don't see a way to make things work via this route.

Instead, this PR proposes to store a custom value in ctx.obj. This object is being passed around anyway, and as far as I understood, ctx.obj may be used for these kind of application-specific settings. The code I've written tries to be super careful not to mess up any current usages of this object:

    if hasattr(typer_obj, "rich_markup_mode"):
        if not hasattr(ctx, "obj") or ctx.obj is None:
            ctx.ensure_object(dict)
        if isinstance(ctx.obj, dict):
            ctx.obj['TYPER_RICH_MARKUP_MODE'] = typer_obj.rich_markup_mode

Then, right before the escaping we can check this value and refrain from escaping if Rich is not enabled.

Additional complication

When utils docs is called to create a markdown version of a file, we need to make sure that we only call rich_to_html on text that has been properly escaped, i.e. when rich is installed AND rich_markup_mode == "rich". This requires us to inspect the value of ctx.obj also in the function _parse_html. I've added another unit test to double check this behaviour when rich_markup_mode=None - cf test_doc_no_typer()

svlandeg avatar Dec 12 '24 11:12 svlandeg

This should also fix issues (such as [linktext](link) being rendered to markdown as (link)) when rich_markup_mode="markdown", right?

fornwall avatar Feb 10 '25 11:02 fornwall

This pull request has a merge conflict that needs to be resolved.

github-actions[bot] avatar Sep 20 '25 07:09 github-actions[bot]