Skip to content
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

Change --text from argument to option #28

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
32 changes: 31 additions & 1 deletion src/labelle/cli/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,16 @@
# this notice are preserved.
# === END LICENSE STATEMENT ===
import logging
import sys
from pathlib import Path
from typing import List, NoReturn, Optional

import typer
from click.exceptions import ClickException
from click.exceptions import UsageError as ClickUsageError
from rich.console import Console
from rich.table import Table
from typer.rich_utils import rich_format_error
from typing_extensions import Annotated

from labelle import __version__
Expand Down Expand Up @@ -554,9 +558,35 @@ def default(
output_bitmap(bitmap, output)


def add_hint_about_text_option(e: ClickException) -> None:
"""Insert a suggestion to use the --text flag when a command is not found.

In dymoprint the --text option was implicit. If labelle is invoked without
--text, it presents as a ClickUsageError with the message "No such command..."
We append to this error message a hint to use the --text flag.
"""
if isinstance(e, ClickUsageError):
# Enhance the error message for dymoprint users who are
# not used to the --text flag being mandatory.
if e.message.startswith("No such command '") and e.message.endswith("'."):
command = e.message[17:-2]
if " " in command:
command = f'"{command}"'
e.message += f""" Did you mean --text {command} ?"""


def main() -> None:
configure_logging()
app()
try:
app(standalone_mode=False)
except ClickException as e:
# Use standalone mode to avoid typer's default error handling here:
# <https://github.com/tiangolo/typer/blob/773927208fbf03d30d50fc39fe2a1a18b7bd93cb/typer/core.py#L207-L216>
# This allows us to add the following hint:
add_hint_about_text_option(e)
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't test it, so I might be wrong here, but I think this function should return True in case it found "No such command" and updated the message.
In case of True, it should do the following two lines:

rich_format_error(e)
sys.exit(e.exit_code)

Otherwise, it should raise.
This way we'll get proper error/exception handling by typer, except for this specific case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The function only runs if there is an exception. In that case, it doesn't handle the exception, it simply mutates the message when necessary. Those two lines run in any case in order to handle the exception, so I don't see how returning a boolean would help. Does that make sense?

The goal is basically to handle the standalone_mode=False branches from the code linked in the comment. Based on my own tests I think it's working well, but it'd be great if you could try if you can break something.


rich_format_error(e)
sys.exit(e.exit_code)


if __name__ == "__main__":
Expand Down