Properly override click.HelpFormatter.getvalue

[ofek]

https://click.palletsprojects.com/en/stable/api/#click.HelpFormatter.getvalue

If the class attribute export_console_as on RichContext (or a subclass) is set to a value that is not None then getting the value of the buffer returns the raw text that was printed to the underlying console rather than returning an empty string from the parent class' method.

I'm a maintainer of https://github.com/mkdocs/mkdocs-click and I noticed that users of rich-click commands were not rendering the usage properly because this line was returning nothing. The choice to return text here is both out of necessity because the current context is inaccessible to that method and also because we happen to desire text so that rendering does not contain any formatting:

[dwreeves]

Thanks. I played around and it shouldn't impact things. So we can do this.

One note: I played around with the Click getvalue(), and it doesn't clear the buffer:

>>> from click.formatting import HelpFormatter
>>> hf = HelpFormatter()
>>> hf.write("foo")
>>> hf.write("bar")
>>> hf.getvalue()
'foobar'
>>> hf.getvalue()
'foobar'

So I am thinking it may make more sense to return self.console.export_text(clear=False). The reason being, I do want the RichHelpFormatter to more closely resemble the HelpFormatter (thank you for this PR, as you help us get to this direction), and this behavior I am suggesting would make it more closely align with that. WDYT?

[ofek]

Good idea, thanks! I just pushed a commit that does that.

[dwreeves]

Thanks.

Upon further thinking (refer to the discussion in the linked issue), there may be some additional work that needs to be thought through.

If we switch to something more compliant with the HelpFormatter API, as per the other discussion, that means getvalue() is called at the very end of the runtime context and prints all the output, meaning it would need to have the styles=True kwarg set as well for a typical runtime. However, this should probably be configurable, which could be done by adding a kwarg to getvalue() (altho that could be weird to do that though as getvalue() is a common duck-typed method name for interacting with IO-like objects and typically accepts no args), or more likely by adding a kwarg to RichHelpFormatter().

We may also want to consider adding a more explicit def export(as_: Literal["html", "svg", "text"] = "text", **export_kwargs: Any) -> str method to the RichHelpFormatter as well, which could then be called at the end of the runtime context where rich-click --output ... is specified (instead of getvalue()).

Sorry, need to sit on this for a bit. I am supportive of where you / we are headed here, but I want to make sure it is good for general purpose usage. I do understand your concerns are a little more narrow and you want to support your users of mkdocs-click. I'm on board with that, users who simultaneously use rich-click and mkdocs-click are also my users as well, I just also have a lot more users than that.

[dwreeves]

OK, so the objective here is to make rich-click more aligned with the base Click API in how it handles the HelpFormatter. Really happy we're taking this on, and you've given me a good starting point for how to think through it.

It will require a decent amount of additional work, and it's not as simple as the PR here.

You can take it on if you'd like. If not, I totally understand, and I can take a stab at it.

Even if you do take this on, I am a little reluctant to release these refactors as a patch version release, as a warning; I would rather this be in a minor release, which I do have plans to do by the middle of this year. So think June or July for when this gets shipped (alongside a bunch of other stuff).

---

If you do not want to take this on, please just add styles=True to the export_text() call, and I'll merge this in and do the rest of it when I get a moment. 😄

---

If you do want to take this on, we need to do the following:

• export_text() also needs styles=True, as mentioned before.
• The default console need to write to open(os.devnull, "w") and has record = True turned on; a user who specifies their own Console() obj should not have these defaults overwritten; they would just stream directly to their stdout and the getvalue() should return an empty string.
• Error handling needs to be updated. When I implement these changes as stated so far, error messages do not print correctly, so the RichCommand.main() method needs some love here as well.
• The rich-click CLI tool needs to reflect these changes as well. I would probably remove the weird exit() method override in RichContext, and make it so getvalue() exports html or svg. This also means the RichHelpFormatter needs to make use of export_console_as.
• I need to think about what to do with the file= kwarg in RichHelpFormatter. I am pretty sure though not 100% sure we just deprecate this, as I don't believe it is externally facing to begin with. However, that needs to be done safely (raise a DeprecationWarning, and also make sure that record = False is set so that we don't double the outputs.)