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

Expose a way to send usage information to stderr #164

Closed
Julian opened this issue Mar 10, 2024 · 9 comments
Closed

Expose a way to send usage information to stderr #164

Julian opened this issue Mar 10, 2024 · 9 comments

Comments

@Julian
Copy link

Julian commented Mar 10, 2024

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
Copy link
Collaborator

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
Copy link
Author

Julian commented Mar 11, 2024

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 added a commit to Julian/rich-click that referenced this issue Mar 11, 2024
@Julian
Copy link
Author

Julian commented Mar 11, 2024

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
Copy link
Collaborator

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

Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 10, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 10, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 10, 2024
@Julian
Copy link
Author

Julian commented Apr 10, 2024

Hey! Thanks for the ping (and efforts!). It looks like I still have some failing tests if I switchover -- https:/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).

Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 10, 2024
@dwreeves
Copy link
Collaborator

dwreeves commented Apr 10, 2024

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
Copy link
Collaborator

@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: #166 (comment)

>>> 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 added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 11, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 11, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 12, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 12, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 12, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 12, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 12, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 13, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 13, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 14, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 14, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 14, 2024
@Julian
Copy link
Author

Julian commented Apr 14, 2024

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

@dwreeves
Copy link
Collaborator

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

Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 15, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 15, 2024
Julian added a commit to bowtie-json-schema/bowtie that referenced this issue Apr 19, 2024
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

2 participants