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

rich markup shows when using sphinx-click #48

Open
ri0t opened this issue Mar 21, 2022 · 16 comments
Open

rich markup shows when using sphinx-click #48

ri0t opened this issue Mar 21, 2022 · 16 comments
Milestone

Comments

@ri0t
Copy link

ri0t commented Mar 21, 2022

Currently, the click-man-page rendering and sphinx html docs get pretty messed up when using rich-click.
Here's an example of a man page output:

ISO(1)                                        iso Manual                                       ISO(1)

NAME
       iso - Main Isomer CLI

SYNOPSIS
       iso [OPTIONS] COMMAND [ARGS]...

DESCRIPTION
       [bold cyan on blue] :diamonds: Isomer[/][white on blue] Management Tool [/]

       This tool supports various operations to manage Isomer instances.

       [yellow]  :warning: Most of the commands are [u]grouped[/u].[/]

       To obtain more information about the groups' available subcommands/groups, try:

       [bright_cyan]  iso [/]

       To display details of a command or its subgroups, try:

       [bright_cyan]  iso   [..]  --help[/]

       To get a map of all available commands, try:

       [bright_cyan]  iso cmdmap[/]

OPTIONS
       -e, --env, --environment [blue|green|current|other]
              Override environment to act on (CAUTION!)

Notice the --env option - it is not immediately obvious if that is weird markup or actual option choices.

I think it would make sense to somehow throw all the markup out in manpages. With sphinx-generated html or similar things that could render them - not sure how to proceed.

@ewels
Copy link
Owner

ewels commented Mar 24, 2022

Hmm, I've not heard of click-man-page before, and I guess it's the sphinx-click plugin? I'd need to look into how they are fetching the click output first - if you're able to do some detective work that would help.

However, hopefully #19 will help. The aim there is to make it easier to return a rich renderable object instead of always printing. Then it would be fairly easy to get a plaintext copy of the help without any markup. If the above plugins are using regular click function calls to fetch the help strings, it might be that we can refactor the rich-click code to better emulate native click. Bit of a long shot, but may be possible.

@ewels ewels changed the title Better handling of meta/tags in simpler outputs rich markup shows when using sphinx-click May 15, 2022
@dwreeves
Copy link
Collaborator

As far as rst is concerned, we have it on our roadmap to support rst docstrings in #172.

And HTML generation is supported via: rich-click --output=html path.to.my:cli --help

Rendering the CLI help text as simple rst is a different story. We can potentially add this to the --output=? options in the rich-click CLI.

I don't know how high of a priority this is. Typer has a markdown rendering feature and I suppose it would be cool to keep up with them in terms of features. I'm marking this as being on the 1.9 roadmap but it's low on the 1.9 wishlist.

@dwreeves dwreeves added this to the 1.9 milestone Apr 10, 2024
@ofek
Copy link
Contributor

ofek commented Apr 6, 2025

I face a similar situation to this and #19. I'm trying to auto generate HTML documentation. What would be a way to get HTML output without printing to stdout?

@ofek
Copy link
Contributor

ofek commented Apr 6, 2025

The solution currently appears to be that an application must manage its own custom subclass of RichContext so that you can set the value of this export_console_as class attribute to html if you detect that you are building documentation (with for example an environment variable), otherwise None. This is because there is currently no way to pass down that configuration. Also, I noticed that this RichHelpFormatter.export_console_as class attribute actually does nothing but perhaps it was a part of an eventual plan to pass down such information.

The last piece to make it work requires modification from what appears to be an oversight so I'm going to open a PR to fix that.

@ofek
Copy link
Contributor

ofek commented Apr 6, 2025

#230

@dwreeves
Copy link
Collaborator

dwreeves commented Apr 7, 2025

Hi, I'll look at this when I get a moment.

For now, I will say, the intended behavior is: you run the command rich-click --output html [your command here] --help, and it will output HTML to stdout. I think this is a pretty reasonable API (I'm personally not a fan of the Typer API where it adds a hidden utils subcommand to your CLI).

The RichContext.export_console_as variable is more for internal usage only, i.e. not part of the public API. I'm not entirely following why it needs to be set for your use case. To be honest, I'm not fully following the issue you are running into, so I'll have to play with mkdocs-click to see what you're talking about firsthand. Looks like a pretty cool project, by the way!

All that said, the PR you've suggested seems pretty reasonable even without me fully understanding the issue; we welcome efforts to do more to make the rich_click.RichHelpFormatter properly aligned with the methods in its subclass click.HelpFormatter. I will however have to do some testing of it and think through the implications, as I'm not sure we have full coverage of everything, in particular the aforementioned rich-click --output ... CLI.

@ofek
Copy link
Contributor

ofek commented Apr 7, 2025

Thanks! I think the missing context is that documentation generators such as the one I maintain and the one described by the opening post of this issue cannot use the CLI as we must go through the actual APIs for introspection in order to construct the desired HTML e.g. segmented into sections, custom table formatting, branded coloring, etc.

@dwreeves
Copy link
Collaborator

dwreeves commented Apr 7, 2025

Thanks, and that makes sense. I've left a comment on your PR.

Note I do not really have plans to make outputting to html or svg any more accessible than RichContext.export_console_as. Unless you can come up with something that's unobtrusive to the core API, I believe having it as a variable in the context is both sufficient and appropriate. Generally my thinking is, anyone who really wants to do stuff like that and is skilled enough, can figure it out, as you have. I also have no plans to change export_console_as's behavior or anything like that, so feel free to rely on it.

@ofek
Copy link
Contributor

ofek commented Apr 7, 2025

I think the downside of what you're saying is the impact on users. In my application I just so happen to have complex requirements where I already had a custom command and context class but the vast majority of users will not. For those users who use rich-click and wish to automatically generate documentation it would be nice if they could just pass down that export_console_as configuration with a simple one-liner based on os.environ. If they don't have that capability then the user has to literally start subclassing and using the subclasses everywhere which I think is an undue burden.

@dwreeves
Copy link
Collaborator

dwreeves commented Apr 7, 2025

If that's the case, I'm not even 100% sure that export_console_as is the right approach for your users.

To be honest, I am reluctant to expand the API here to use environment variables. API scope creep is dangerous for maintainability, and creating niche things that others rely on is not something I want to do when I believe there are other solutions.

Before I continue, can I get a bit of clarification here?: are you actually relying on all the behavior of export_console_as, or is it just a way you use to set console.record = True and console.file = open(os.devnull, "w")?

It seems like the latter may be true in your case. In which case I would recommend setting that directly.

For the current version of rich-click, on your end, you can do something like this:

def _build_command_context(
    prog_name: str, command: click.Command, parent: click.Context | None
) -> click.Context:
    # https://github.com/pallets/click/blob/8.1.8/src/click/core.py#L859-L862
    ctx = command.context_class(
        cast(click.Command, command),
        info_name=prog_name,
        parent=parent,
        **command.context_settings,
    )

    try:
        import rich_click
    except ImportError:
        pass
    else:
        if isinstance(command, rich_click.RichCommand):
            ctx.console.record = True
            ctx.console.file = open(os.devnull, "w")
    return ctx

That said, I do think we can come to some sort of agreement on a set of code changes that would be beneficial for you.

For internal code changes, I would be happy to make it so that, by default, rich-click does not eagerly write to the console, but instead writes to a buffer which getvalue() is then called to retrieve. I do believe this is how Click actually works in practice, e.g. you can see this slight difference in behavior based on where the flask error message occurs:

❯ rich-click flask --help
                                                                                                                                                                
 Usage: flask [OPTIONS] COMMAND [ARGS]...                                                                                                                       
                                                                                                                                                                
 A general utility script for Flask applications.                                                                                                               
 An application to load must be given with the '--app' option, 'FLASK_APP' environment variable, or with a 'wsgi.py' or 'app.py' file in the current directory. 
                                                                                                                                                                
╭─ Options ────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ --env-file          -e  FILE    Load environment variables from this file, taking precedence over those set by '.env' and '.flaskenv'. Variables set         │
│                                 directly in the environment take highest precedence. python-dotenv must be installed.                                        │
│ --app               -A  IMPORT  The Flask application or factory function to load, in the form 'module:name'. Module can be a dotted import or file path.    │
│                                 Name is not required if it is 'app', 'application', 'create_app', or 'make_app', and can be 'name(args)' to pass arguments.  │
│ --debug/--no-debug              Set debug mode.                                                                                                              │
│ --version                       Show the Flask version.                                                                                                      │
│ --help                          Show this message and exit.                                                                                                  │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
Error: Could not locate a Flask application. Use the 'flask --app' option, 'FLASK_APP' environment variable, or a 'wsgi.py' or 'app.py' file in the current directory.

Error: Could not locate a Flask application. Use the 'flask --app' option, 'FLASK_APP' environment variable, or a 'wsgi.py' or 'app.py' file in the current directory.

Error: Could not locate a Flask application. Use the 'flask --app' option, 'FLASK_APP' environment variable, or a 'wsgi.py' or 'app.py' file in the current directory.

Error: Could not locate a Flask application. Use the 'flask --app' option, 'FLASK_APP' environment variable, or a 'wsgi.py' or 'app.py' file in the current directory.

╭─ Commands ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ routes                       Show the routes for the app.                                                                                                    │
│ run                          Run a development server.                                                                                                       │
│ shell                        Run a shell in the app context.                                                                                                 │
╰──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯

versus:

❯ flask --help           
Error: Could not locate a Flask application. Use the 'flask --app' option, 'FLASK_APP' environment variable, or a 'wsgi.py' or 'app.py' file in the current directory.

Usage: flask [OPTIONS] COMMAND [ARGS]...

  A general utility script for Flask applications.

  An application to load must be given with the '--app' option, 'FLASK_APP'
  environment variable, or with a 'wsgi.py' or 'app.py' file in the current
  directory.

Options:
  -e, --env-file FILE   Load environment variables from this file, taking
                        precedence over those set by '.env' and '.flaskenv'.
                        Variables set directly in the environment take highest
                        precedence. python-dotenv must be installed.
  -A, --app IMPORT      The Flask application or factory function to load, in
                        the form 'module:name'. Module can be a dotted import
                        or file path. Name is not required if it is 'app',
                        'application', 'create_app', or 'make_app', and can be
                        'name(args)' to pass arguments.
  --debug / --no-debug  Set debug mode.
  --version             Show the Flask version.
  --help                Show this message and exit.

Commands:
  routes  Show the routes for the app.
  run     Run a development server.
  shell   Run a shell in the app context.

So you can see for rich-click flask, the error message prints halfway through the command, whereas simply flask prints it at the top, which is to say the error message is generated halfway through parsing but Click defers printing the help text until the entire thing is in the buffer (whereas we do not).

If by default, the Console() rich-click provides writes to devnull, has record=True, and has a value retrievable by help_formatter.getvalue(), that should solve your issues, right?

@ofek
Copy link
Contributor

ofek commented Apr 7, 2025

If that's the case, I'm not even 100% sure that export_console_as is the right approach for your users. [...] To be honest, I am reluctant to expand the API here to use environment variables.

No I mean the user would do that like:

@rich_click.rich_config(
    help_config=rich_click.RichHelpConfiguration(
        export_console_as="html" if os.environ.get("BUILDING_DOCS") == "true" else None,
    ),
)

Before I continue, can I get a bit of clarification here?: are you actually relying on all the behavior of export_console_as, or is it just a way you use to set console.record = True and console.file = open(os.devnull, "w")?

A way to set the latter I think because I don't know what else it does 😅

If by default, the Console() rich-click provides writes to devnull, has record=True, and has a value retrievable by help_formatter.getvalue(), that should solve your issues, right?

Following the way click does things like that would indeed solve the issue as long as the value is HTML, or could be configured as such.

edit: Hopefully configurable actually because we would require the help text to be HTML whereas we would prefer the usage string (e.g. command subcommand [OPTIONS]) to have no formatting and just be text.

@dwreeves
Copy link
Collaborator

dwreeves commented Apr 7, 2025

OK, I think I am making sense of what you need and want to do.

I will be honest though, I'm not sure to what extent I would fully support it. I would still think about adding explicit support in your code for rich-click (I can potentially contribute it if you want), which would be an even easier path for users as they would have to do nothing. The usage string will be HTML, there is no way around that; we are just running console.export_html() to generate HTML, and we will not be changing that.

The default output of getvalue() will be text with ansi escape codes, not HTML. I'm also a teeny bit reluctant to add export_console_as to the user facing API, even buried in the config; the config already has some scope issues (it controls styles, names of things, and runtime-specific config, all of which are distinct things) and I want to limit any runtime-specific config if it's not necessary.


I see in your code you print out the command output, which is easier, but can run into some limitations, as is the case here. Forgive me if I am overstepping here in suggesting this, but Typer's markdown parsing may be of interest for your code, and could make things really clean and effective: https://github.com/fastapi/typer/blob/46f4a0f470400aef45fd3d433c5066687f4d61e8/typer/cli.py#L195 Basically, they parse out all the objects inside of a click.Command and return a Markdown string. They have an MIT license, so it's safe to copy paste in your code with attribution. (You can also just import the function directly.) In addition to making things work with rich-click, that would also mean your library works with both rich-click and Typer as well, without requiring anything extra from the user.

If you are time constrained and would want assistance in implementing Typer's implementation of markdown parsing, let me know and I'd be glad to open a PR in mkdocs-click.

@ofek
Copy link
Contributor

ofek commented Apr 7, 2025

Talking this out is very helpful so I appreciate your patience! Upon further thinking, I actually do not need HTML and I was just lucky in my tests that the currently rendered HTML looks nice. What I actually want ubiquitously is the raw text without ANSI codes so that I can dump the user-defined Markdown help text within the generated page. The export_console_as shenanigans I'm doing is just to prevent the printing to stdout apparently.

So final answer is I just need (and the library this issue mentions needs) only the raw text please!

@dwreeves
Copy link
Collaborator

dwreeves commented Apr 7, 2025

Yep, that's what I figured when I was asking for clarification, that the export_console_as stuff wasn't necessary. Good to affirm this!

We can make it so getvalue() works "properly," that would be beneficial, but the ANSI codes will still be there, and I would suggest stripping them out on your end. We would not be the only people with ANSI codes, for what it's worth; click-extra will also add them and your own users using raw Click may add them. So, I would suggest handling those separately.

What do you think about the Typer code I linked to with get_docs_for_click()? I think it could be a good solution that's backwards compatible with older version of rich-click, and provides a lot of extra features on your own end. (I am suggesting this separate of the getvalue() stuff, which I do want to implement irrespective of your situation as it aligns rich-click's internals more closely with base Click.)

@ofek
Copy link
Contributor

ofek commented Apr 8, 2025

  1. Shouldn't be a problem if there are ANSI codes! Do you know if Rich has a utility to strip them, or if you recommend some alternative mechanism to do som
  2. I checked out that function but I'm not sure it would be very useful since the parser we maintain is similar but we just choose different formatting in the end. It would be useful if there was something in click proper that introspected a command by recursively walking and yielding commands and options. Perhaps I should open a feature request over there.

@dwreeves
Copy link
Collaborator

dwreeves commented Apr 8, 2025

Stripping ANSI can be done with regex.

And ahhh, I see. OK. Didn't read the code carefully. 🤦 The issue is specifically the Command().format_usage() call printing to stdout as a side effect instead of writing to the buffer inside the formatter. OK, I think I am fully following now!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants