-
Notifications
You must be signed in to change notification settings - Fork 38
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
Added echo, echo_via_pager #29
Conversation
Thanks for this! Will review soon but two speed questions:
|
For me, it's to drop in replace for code that's using click.echo/echo_via_pager. click.echo in particular does a number of things to "play nice" with click apps / progress bars / auto-flush, etc. For me, if I'm pulling rich-click in, it would be nice for it to "richify" all the click things including echo, etc. Using these as drop-in replacements lets the developer slowly "richify" an existing code base. Another use case (and the reason I originally wrote these) is that rich + click.testing.CliRunner + pytest doesn't work. The issue is closed in rich repo but the problem still exists. If you use rich print in a click app and use pytest + the click CliRunner to test, the rich output doesn't get captured. This code lets you use click.echo which does work with pytest and the test runner but get rich formatting.
I'm already working on that...didn't realize how you were using those globals before I sent the PR. I'm working on an update that will work with Markdown and honor the globals you're using. EDIT: added another commit which does this. |
Ok. If you don't mind, I'm going to come back to this after I've looked into #25 (setting up unit tests for this repo). My gut feeling is that I don't want to expand the scope of the package any more than I have to and that I would prefer developers to use native rich functions directly (less chance of error, less maintenance here). If you're going to start editing strings to use rich markup codes, it's not much more code to use the rich function calls as well. The difficulty with testing is more pertinent, but I wonder if we can get around that in a different way seeing as rich-click doesn't use Phil |
No worries -- I'll keep using these "rich-ified" versions in my own code. The package name "rich-click" made me think this is an appropriate place for these functions as that implies more than just "rich help text". Cheers! |
Yeah, I'm a bit in two minds as it's not all that much code to add I guess. It's more the fear of getting to the point where rich-click is rewriting the entire click library 😅 Do you have reasons other than the testing thing to continue using these instead of native rich functions? |
No, for me it's primarily driven by the fact that I want testing to work. The ideal solution is to figure out how to get rich fixed so it plays well with click and pytest. I'll poke around the rich code to see if I can figure out a fix there. I came up with the "rich echo" while trying to solve the testing thing and thought it was kind of elegant as it allows existing click code to be slowly migrated to rich output without any big changes. It does to |
Hi @RhetTbull, Apologies, I let this PR sit here a bit too long and it fell behind the main branch. I've just had a stab at resolving the merge conflicts and getting the code up to speed with the new linting tools. I've also moved the new code into its own file and rearranged the examples to match the new directory structure. The only thing I haven't fixed yet is Many thanks, Phil |
OK, I'll take a look. |
I'm in the middle of another project but will take a look at this when I get a chance |
No rush 👍🏻 I think it should be fairly simple hopefully, I would do it myself but I'm not totally sure about how you're using these functions in the wild, which is why I was a little nervous. (Also I'm totally new to Python typing so there's a high chance that I would mess it up 😅 ). Let me know if you'd like me to have a stab at it anyway though. |
came here looking for this. A straight-forward rich capable click.echo() would be very handy. |
Hi @RhetTbull, I just merged in #92 that @BrutalSimplicity says resolves this PR (#89 (comment)). I can't see how it adds rich markup to these commands, but honestly the PR is massive and has gone a bit over my head. If you get a moment, would you mind taking a look? Thanks! Phil |
This is a neat feature and I'm looking to add it in rich-click 1.8, although I have a few notes.
I will get around to this in the next few weeks. Thanks for your work on this and sorry it took a whole year and a half! |
After thinking about this for a bit, I've come to the conclusion this may not be in scope for rich-click. Apologies for reneging. We appreciate your work on this PR, even though it was a long time ago. So I think given that you worked hard on this, we owe it to you to provide a brief explanation as to why we are not going to implement this feature: 1. Overhead
2.
|
Does rich.console play nice with all the click quirks? Maybe this issue would be a good place to refer to some example on how to use it with rich-click. I wouldn't object your decision since you listed very good reasons and I fail to completely grasp all the consequences of that feature for the project, but I know those are often underestimated. But I'd like to mention that I came to this issue since basically the first thing I missed was a way to print rich text "the click way" using markup. And the lack of a way to do it made me stop using rich-click (richness just wasn't important). So I wouldn't agree that there is no demand. So I think if an example would basically look like a bad version of this PR, then (and only then) reconsideration might make sense since I see a rich echo() as vital feature of a rich framework. If @dwreeves anyways, thank you (and all contributors) for your time & effort! |
I guess my question would be: Why not? Or maybe, what do you have in mind when you ask this?
You can make a Click CLI that is just Click does not do anything weird in terms of mutating how Python fundamentally operates. You can add and remove whatever you want. It just creates a thin and slightly interactive layer between your terminal and your code base. The same goes for rich-click. It's possible we can mention this in the docs somewhere, maybe a paragraph or two saying something like:
Users have a few options: they can print rich text the Rich way, or they can print normal text the Click way, or they can print normal text the "print Rich text the Click way" during code execution is to me a bit of a strained idea. Don't take me saying this the wrong way, but there is nothing sacrosanct about Anyway, I'm sorry this is disappointing. I appreciate your thanks to me and the other contributors! |
Good to know. Thank you for elaborating. |
Adds echo() and echo_via_pager() commands that work just like the click variants but support rich markup.