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

Inconsistent highlighting in help section #108

Closed
km2023-4 opened this issue Apr 19, 2023 · 9 comments
Closed

Inconsistent highlighting in help section #108

km2023-4 opened this issue Apr 19, 2023 · 9 comments
Labels
bug Something isn't working

Comments

@km2023-4
Copy link

Dear authors, I recently started using rich-click and have to say it looks amazing.

While playing around with click's command help sections, though, I noticed some inconsistent colouring/highlighting:

image

It seems that the argument related regex in the OptionHighlighter class (rich_help_configuration.py) is not handling things consistenly. Removing the regex seems to be more pleasing on the eyes...

class OptionHighlighter(rich.highlighter.RegexHighlighter):
    """Highlights our special options."""

    highlights = [        
        r"(^|\W)(?P<switch>\-\w+)(?![a-zA-Z0-9])",
        r"(^|\W)(?P<option>\-\-[\w\-]+)(?![a-zA-Z0-9])",
        # r"(^|\W)(?P<argument>[A-Z0-9\_]+)(?![_a-zA-Z0-9])",        
        r"(?P<metavar>\<[^\>]+\>)",
        r"(?P<usage>Usage: )",
    ]

image

Do we really need to auto highlight words in capitals or numbers inside the help section?

@ewels
Copy link
Owner

ewels commented Apr 19, 2023

Good spot! I think that this regex is intended to highlight positional arguments in the first Usage: line. This bit (^^):

$ python examples/06_arguments.py --help

 Usage: 06_arguments.py [OPTIONS] INPUT
                                  ^^^^^

Totally agree that it's not specific enough though, and it shouldn't be highlighting the random capital letters in the rest of the help text 👍🏻

@ewels ewels added the bug Something isn't working label Apr 19, 2023
@km2023-4
Copy link
Author

The following does the job for me. Limits the highlighter to the "Usage" line:

class OptionHighlighter(rich.highlighter.RegexHighlighter):
    """Highlights our special options."""

    highlights = [
        r"(^|\W)(?P<switch>\-\w+)(?![a-zA-Z0-9])",
        r"(^|\W)(?P<option>\-\-[\w\-]+)(?![a-zA-Z0-9])",
        # r"(^|\W)(?P<argument>[A-Z0-9\_]+)(?![_a-zA-Z0-9])", # old
        r"(Usage:.*\])(?P<argument>[\s\w]+)", # new
        r"(Usage:.*\[)(?P<option>OPTIONS)\]", # new
        r"(?P<metavar>\<[^\>]+\>)",
        r"(?P<usage>Usage: )",
    ]

Full example to test would be (python <.py file> test-command --help):

import rich_click as click

click.rich_click.USE_RICH_MARKUP = True


@click.group()
def cli():
    pass


@cli.command()
@click.argument("a")
@click.argument("argument_2")
@click.option("--test-option0", "-t", is_flag=True, help="An option.")
@click.option(
    "--test-option1",
    "-o",
    is_flag=True,
    help="A second option with highlighted single capital 'A'?",
)
@click.option(
    "--test-option2",
    "-S",
    "-s",
    is_flag=True,
    help="Another option but with inconsistent highlighting of switch '-S','-s'.",
)
@click.option(
    "--test-option3",
    help="An option with weird looking highlighting: v2.0, d-A-ta, dataset0.5, v0.5-test-0.5, [blue]something-N-ot-SO-blue[/]",
)
def test_command(a, argument2, test_option0, test_option1, test_option2, test_option3):
    """
    A test command.
    """
    pass


if __name__ == "__main__":
    cli()

@ewels
Copy link
Owner

ewels commented Apr 20, 2023

Nice, thanks! Unfortunately your example script actually flagged that it's not working with subcommands though 😅

Struggled to find a succinct regex that would match all three whilst being limited to that line. So followed your lead and added 3, one for each section. I actually went even more explicit, as if we're only targeting that first line I think that we can explicitly look for the words OPTIONS etc, rather than all uppercase words. Hopefully these are stable.

Tested locally and working as expected now.

@ewels ewels closed this as completed in 3363e15 Apr 20, 2023
ewels added a commit that referenced this issue Apr 20, 2023
@ewels
Copy link
Owner

ewels commented Apr 20, 2023

I think that we can explicitly look for the words OPTIONS etc, rather than all uppercase words. Hopefully these are stable.

Nope, I was wrong. Too simplistic. Arguments are listed there by name, with and without square brackets (depending on whether they're required or not).

@ewels ewels reopened this Apr 20, 2023
@ewels
Copy link
Owner

ewels commented Apr 20, 2023

Updated example 6 to give a more varied usage string:

 Usage: 06_arguments.py [OPTIONS] INPUT OUTPUT {yaml|json} [FLAVOUR]

ewels added a commit that referenced this issue Apr 20, 2023
@ewels
Copy link
Owner

ewels commented Apr 20, 2023

Ok, I think that got it.. 🤞🏻

@ewels ewels closed this as completed Apr 20, 2023
@km2023-4
Copy link
Author

Awesome! Thanks for dealing with this so quickly!

ewels added a commit that referenced this issue Apr 20, 2023
ewels added a commit that referenced this issue Apr 20, 2023
Prevents line-break bug for when using markdown

See #108 (again)
@km2023-4
Copy link
Author

I noticed that the metavar highlighting can still play up a little when using '<' or '>' in the help section...

image

@ewels
Copy link
Owner

ewels commented Apr 21, 2023

That doesn't really bother me so much, as wrapping things in < foo > is generally accepted notation for metavars. In contrast the option thing was much more generic; any capital letters (also I don't think it worked for required enumerated option args).

That being said, I'm not actually sure what that regex is meant to be highlighting..? I'm struggling to find any standard click output that uses <> now.. 🤔

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

No branches or pull requests

2 participants