ewels / rich-click

Format click help output nicely with rich.
https://ewels.github.io/rich-click/
MIT License
583 stars 33 forks source link

Expose a way to send usage information to stderr #164

Closed Julian closed 3 months ago

Julian commented 4 months ago

I was quickly evaluating switching/using this on a project of mine, which looks quite nice (originally I found it simply looking for a good command-group plugin for click, but given I already use rich this seems like a nice fit).

I have one main issue though from trying to drop this in -- my tests fail because usage errors seem to go to stdout rather than stderr.

Specifically if I run bowtie non-existing-subcommand after simply changing to import rich_click as click I get usage output on stdout.

I see the README mentions that the rich console I can modify is separate from the one used internally. It seems the internal one is the one that needs modifying to change this behavior. Looking at the code, I see there's a create_console that takes a file where I'd love to pass sys.stderr, but as far as I can quickly tell there's nothing that calls that, and/or no way I can actually pass that argument through via rich.click_config(). Alternatively I don't quite see a way to inject a RichHelpFormatter.

Apologies if this exists and I've missed it. I'm happy to send a PR if need be, though to do so I'd love to know whether the current behavior (sending to stdout) is intentional / whether you want to keep it, and if so but you still would support some way of changing where it goes, whether the intention is to expose create_console somewhere or else to only expose file directly via click_config.

Thanks!

dwreeves commented 4 months ago

Hi @Julian, thank you for opening this issue!

but as far as I can quickly tell there's nothing that calls that

Yep, that's a bug that I noticed and will be fixed in 1.8.0. I am already working on that.

I have one main issue though from trying to drop this in -- my tests fail because usage errors seem to go to stdout rather than stderr.

One of rich-click's objectives is to be a seamless drop-in that mimics click. In this case, there is a disparity between how rich-click emulates click, and is thus a bug.

I think I/we can fix this as part of a patch release to 1.7.x.

If you want to do a PR based on the "1.7.x" branch, I can merge it in and release it!

If you'd rather me work on it, I won't be able to get to this until next weekend.

Julian commented 4 months ago

Thanks for the response! Very glad to hear.

I'll give it a quick look.

One of rich-click's objectives is to be a seamless drop-in that mimics click. In this case, there is a disparity between how rich-click emulates click, and is thus a bug.

Great! OK. Then I'll also mention the other more minor issue, which is that switching also requires me to dot in at least a bunch of calls to textwrap.dedent -- otherwise my help strings end up with strange internal indentation. I assume that also then might be something you'd consider a bug? I haven't looked at which ones end up needing it, but at very least my epilogue.

Julian commented 4 months ago

Sent a PR for this one. Making the epilogue match closer looks a bit trickier it seems, I'm not sure what the idiomatic way in rich is to both reflow the text (so it stretches to whatever the width is) while also dedenting. Specifically, my epilog is having its whitespace messed with a bit (because I see that rich-click does split("\n\n"), but that will remove my intentional extra newline -- I could of course add yet another one, but that makes the source text a bit funny looking). Will wait to hear what you think on that one but at least this specific issue I think should be fixed with the one liner in the PR.

Thanks again!

dwreeves commented 3 months ago

@Julian Can you try rich-click==1.8.0dev2? I think this issue should be fixed in that.

Julian commented 3 months ago

Hey! Thanks for the ping (and efforts!). It looks like I still have some failing tests if I switchover -- https://github.com/bowtie-json-schema/bowtie/actions/runs/8631991053/job/23661594967 -- I'm on vacation so I'm a bit slow to check why but an initial look suggests that at least one reason seemingly is the Usage:line still being sent to stdout. Specifically I see:

~/Development/bowtie is a git repository on rich-click 
⊙  bowtie run --dialect ";;;;;" -i no-such-image 2>/dev/null                                                                                                                                                                                                                                                      julian@Airm ●

 Usage: bowtie run [OPTIONS] [INPUT]                                                                                    

(where that command ends up raising a click.UsageError for the --dialect option being nonsense).

dwreeves commented 3 months ago

No thank you for checking!

Sorry, should've checked more carefully on my end.

This one is tricky to write a clean and elegant abstraction for because of how base Click handles printing of error messages... 😅 I'll take another stab at it shortly! Getting stderr and stdout sorted out is a hard requirement for releasing 1.8.0. (Then at some point down the line I'll rewrite our unit tests to make sure the behavior is asserted.)

dwreeves commented 3 months ago

@Julian Ok, pinky promise this time it should work. I tested and it's great!

pip install rich-click==v1.8.0dev4

Using the code example I described a few weeks ago in my comment: https://github.com/ewels/rich-click/pull/166#issuecomment-1989895934

>>> from subprocess import Popen, PIPE
>>> process = Popen(["python", "foo.py", "--fake-flag"], stdout=PIPE, stderr=PIPE)
>>> stdout, stderr = process.communicate()
>>> print(f"{len(stdout)=}", f"{len(stderr)=}")
len(stdout)=0 len(stderr)=112
>>> print(stderr)
b"Usage: foo.py [OPTIONS] FOO COMMAND [ARGS]...\nTry 'foo.py --help' for help.\n\nError: No such option: --fake-flag\n"

Thanks for opening the issue. I'll close once you've confirmed everything's fine.

The stable release of rich-click is slated to be finalized no later than the first week of May, FWIW.

Julian commented 3 months ago

Fantastic, thanks again! This indeed seems like it works here!

dwreeves commented 3 months ago

Great! I'll close the issue then as completed.