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

feat: Support rich markup (instead of Markdown) in HelpFormatter.default_format? #192

Open
pawamoy opened this issue Jan 5, 2025 · 13 comments

Comments

@pawamoy
Copy link
Contributor

pawamoy commented Jan 5, 2025

Just like we can do:

output = cappa.Output(error_format="[bold]insiders[/]: [bold red]error[/]: {message}")

...I was wondering if it would make sense to allow such markup in HelpFormatter.default_format too, for example:

help_formatter = cappa.HelpFormatter(default_format="[dim italic]Default: {default}.[/]")

Note that the default value themselves should still be rendered as Markdown, for example:

cappa.Arg(
    ...,
    show_default=f"`{defaults.DEFAULT_CONF_PATH}`",
)

The reason cappa.Output and cappa.HelpFormatter can use rich markup while show_default should only use Markdown is that the former two are exclusively displayed in the terminal, while the latter can be programmatically rendered into other formats (HTML page describing the CLI for example).

Not sure if it's possible to combine both. I mean it sounds like it should work if default values (show_default) are rendered in isolation as Markdown, then the whole default (formatter) is rendered in isolation as markup, but that might not be the case (both concatenated/formatted as a single string then rendered as Markdown).

@DanCardin
Copy link
Owner

This is somewhat more complicated because the default value string is being templated inside of a larger string. Or i guess not more complicated if you, for example, expected {message} to be markdown.

seems like it'll have to string format some sentinel default value into the string then split on the sentinel, yielding [Text(before), Markdown(default), Text(after)].

Although the same logic would apply to Arg.help, which I could see the case for being either markdown or markup preferentially, no?

@pawamoy
Copy link
Contributor Author

pawamoy commented Jan 6, 2025

I suppose it's reasonable to let the user choose between markup/markdown. That would make the code base more consistent (instead of having some text rendered as markdown and some other as markup). How would you detect one or the other though? Surely `[/]` should not be interpreted as markup in a markdown string.

@pawamoy
Copy link
Contributor Author

pawamoy commented Jan 6, 2025

Relevant: Textualize/rich#1249, Textualize/rich#1272.

@pawamoy
Copy link
Contributor Author

pawamoy commented Jan 6, 2025

markdownitalic

@DanCardin
Copy link
Owner

I dont know that i would detect markdown/markup string-wise. moreso i think it would be fair to accept Text/Markdown objects where strings are currently allowed, and help/show_default would be wrapped in Markdown if a string.

The only way i can think of to compose this with rich's API as-is would be

  • prepare the whole string formatted with stubs "{help} {default}".format(help=uuid1, default=uuid2)
  • figure out a way of splitting the string on all delimiters while keeping track of their order
  • Text each static string segment
  • interpose each concrete value of whichever rich type it ends up being.
  • profit???

I think after all that, the default behavior would remain consistent with how it works today, while still ultimately enabling what you're suggesting, and enabling more custom stuff like your Markdown "hello", style="dim")

@pawamoy
Copy link
Contributor Author

pawamoy commented Jan 6, 2025

Another relevant discussion: Textualize/rich#1951. See how the author manages to combine different renderables into one (Markdown and markup): ewels/rich-click@6309974 and ewels/rich-click@a004ad4 using Columns and Text.from_markup.

format_arg already builds a list of segments, so maybe it could return that list, and add_long_args would wrap it in Columns (something like that).

@pawamoy
Copy link
Contributor Author

pawamoy commented Jan 6, 2025

I managed to get something working with the following:

    help_formatter = cappa.HelpFormatter(
        arg_format=(
            "{help}",
            "{choices}",
            lambda default, **_: default and Markdown(default, style="dim italic"),
        ),
        default_format="Default: {default}.",
    )

In format_arg:

        if callable(format_segment):
            segment = format_segment(**context)
        else:
            segment = format_segment.format(**context)

        if segment:
            if isinstance(segment, str):
                segment = Markdown(segment)
            segments.append(segment)

    return segments

...passing whole context to callable instead of just arg, as well as returning segments directly (without joining them), which are wrapped in Columns in add_long_args:

                table.add_row(
                    Padding(format_arg_name(arg, ", "), help_formatter.left_padding),
                    Columns(format_arg(help_formatter, arg)),
                )

(there might be a parameter on Columns that would prevent new lines between elements, see screenshot below)

default_dim

Not bad 😄

@pawamoy
Copy link
Contributor Author

pawamoy commented Jan 6, 2025

My solution might be too specific to my use-case. The UUID splitting approach might yield a more consistent API.

Anyway, let me know if you want a diff 🙂

@pawamoy
Copy link
Contributor Author

pawamoy commented Jan 6, 2025

Got something working to prevent newlines: Textualize/rich#3603.

@DanCardin
Copy link
Owner

i'm imagining that it needs to format then split because of the possibility of arbitrary text before/after the actual format specifier. Per your original example: show_default="[dim italic]Default: {default}.[/]". It's the only thing that comes to mind without switching the existing API away from using format strings to easily compose the different parts of the arg string (which prior to this seemed like it was working quite well :P)

fwiw, i think it looks nice (maybe even better) for at least your particular set of screenshotted arguments on newlines, lol. Also kinda makes me want to set the default format to be dim by default.

@pawamoy
Copy link
Contributor Author

pawamoy commented Jan 6, 2025

Of course, whatever is best in terms of API and backwards compatibility ☺️

Yeah dim by default is probably not a bad idea. I was finding it hard to read with long default values also using code quotes, hence trying to dim them.

About newlines vs no newlines, yeah, I'm unsure which one I prefer 😄

@DanCardin
Copy link
Owner

I went down somewhat of a rabbit hole on my original take, and i'm not sure it's feasible.

I guess...maybe HelpPart('{default}', cls=Markdown, style='dim light') and arg_format defaults are adjusted to use that format?

so your code would or could look something like

    help_formatter = cappa.HelpFormatter(
        default_part = HelpPart.default(style="dim italic")
    )

and there'd arg_format would maybe be like... (HelpPart.help, HelpPart.choices, HelpPart.default), those as sentinels would invoke code to use the specific "default_part" value, and existing API uses of raw format strings simply wouldn't get custom segment sylability.

this is also very conceptual in my head and i'm not sure if i'm explaining it intelligbly or if it will work either

@pawamoy
Copy link
Contributor Author

pawamoy commented Jan 8, 2025

Yeah I thought this might prove complicated to support in a robust/elegant way. Honestly I'd be fine with leaving out the italic if you just end up dimming the default string by default. Otherwise the HelpPart suggestion seems good too.

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

No branches or pull requests

2 participants