-
-
Notifications
You must be signed in to change notification settings - Fork 680
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
🐛 Fix escaping in help text when rich
is installed but not used
#1089
base: master
Are you sure you want to change the base?
Changes from all commits
172a489
9164116
c6a7347
542d6bd
72eb74b
c02f0fd
dd7edfd
c687a85
0a9c253
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,37 @@ | ||
import typer | ||
|
||
sub_app = typer.Typer() | ||
|
||
variable = "Some text" | ||
|
||
|
||
@sub_app.command() | ||
def hello(name: str = "World", age: int = typer.Option(0, help="The age of the user")): | ||
""" | ||
Say Hello | ||
""" | ||
|
||
|
||
@sub_app.command() | ||
def hi(user: str = typer.Argument("World", help="The name of the user to greet")): | ||
""" | ||
Say Hi | ||
""" | ||
|
||
|
||
@sub_app.command() | ||
def bye(): | ||
""" | ||
Say bye | ||
""" | ||
|
||
|
||
app = typer.Typer(help="Demo App", epilog="The end", rich_markup_mode=None) | ||
app.add_typer(sub_app, name="sub") | ||
|
||
|
||
@app.command() | ||
def top(): | ||
""" | ||
Top command | ||
""" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,6 +20,7 @@ def main(arg: str): | |
assert "Hello World" in result.stdout | ||
|
||
result = runner.invoke(app, ["--help"]) | ||
assert "ARG [required]" in result.stdout | ||
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. This fails with |
||
assert all(c not in result.stdout for c in rounded) | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,6 +10,7 @@ | |
from click import Command, Group, Option | ||
|
||
from . import __version__ | ||
from .models import DefaultPlaceholder | ||
|
||
try: | ||
import rich | ||
|
@@ -209,7 +210,7 @@ def get_docs_for_click( | |
title = f"`{command_name}`" if command_name else "CLI" | ||
docs += f" {title}\n\n" | ||
if obj.help: | ||
docs += f"{_parse_html(obj.help)}\n\n" | ||
docs += f"{_parse_html(ctx, obj.help)}\n\n" | ||
usage_pieces = obj.collect_usage_pieces(ctx) | ||
if usage_pieces: | ||
docs += "**Usage**:\n\n" | ||
|
@@ -233,15 +234,15 @@ def get_docs_for_click( | |
for arg_name, arg_help in args: | ||
docs += f"* `{arg_name}`" | ||
if arg_help: | ||
docs += f": {_parse_html(arg_help)}" | ||
docs += f": {_parse_html(ctx, arg_help)}" | ||
docs += "\n" | ||
docs += "\n" | ||
if opts: | ||
docs += "**Options**:\n\n" | ||
for opt_name, opt_help in opts: | ||
docs += f"* `{opt_name}`" | ||
if opt_help: | ||
docs += f": {_parse_html(opt_help)}" | ||
docs += f": {_parse_html(ctx, opt_help)}" | ||
docs += "\n" | ||
docs += "\n" | ||
if obj.epilog: | ||
|
@@ -257,7 +258,7 @@ def get_docs_for_click( | |
docs += f"* `{command_obj.name}`" | ||
command_help = command_obj.get_short_help_str() | ||
if command_help: | ||
docs += f": {_parse_html(command_help)}" | ||
docs += f": {_parse_html(ctx, command_help)}" | ||
docs += "\n" | ||
docs += "\n" | ||
for command in commands: | ||
|
@@ -272,10 +273,13 @@ def get_docs_for_click( | |
return docs | ||
|
||
|
||
def _parse_html(input_text: str) -> str: | ||
if not has_rich: # pragma: no cover | ||
return input_text | ||
return rich_utils.rich_to_html(input_text) | ||
def _parse_html(ctx: click.Context, input_text: str) -> str: | ||
rich_markup_mode = None | ||
if hasattr(ctx, "obj") and isinstance(ctx.obj, dict): | ||
rich_markup_mode = ctx.obj.get("TYPER_RICH_MARKUP_MODE", None) | ||
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. We should also consider pulling this |
||
if has_rich and rich_markup_mode and rich_markup_mode == "rich": # pragma: no cover | ||
return rich_utils.rich_to_html(input_text) | ||
return input_text | ||
Comment on lines
+276
to
+282
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. 🤔 I think we should move the computation of 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. Yea fair enough, I had it there first 😁 Somehow that felt like cluttering the |
||
|
||
|
||
@utils_app.command() | ||
|
@@ -301,6 +305,14 @@ def docs( | |
if not typer_obj: | ||
typer.echo("No Typer app found", err=True) | ||
raise typer.Abort() | ||
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): | ||
if isinstance(typer_obj.rich_markup_mode, DefaultPlaceholder): | ||
ctx.obj["TYPER_RICH_MARKUP_MODE"] = typer_obj.rich_markup_mode.value | ||
else: | ||
ctx.obj["TYPER_RICH_MARKUP_MODE"] = typer_obj.rich_markup_mode | ||
click_obj = typer.main.get_command(typer_obj) | ||
docs = get_docs_for_click(obj=click_obj, ctx=ctx, name=name, title=title) | ||
clean_docs = f"{docs.strip()}\n" | ||
|
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.
The point of this new test is this: checking that
rich_markup_mode=None
, withrich
installed, still produces correct results (showing [required], default values, etc and not accidentally 'removing' them because of incorrect escaping behaviour)