Helpers::Text: Add .colorize(color, text, **opts) #2150
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I'm unsure of a lot of things here, one of them is arg order. Does it make more sense to sign That way: config = [color, bold]
# easier
colorize(text, *config)
# versus
colorize(config[0], text, config[1]) |
xjunior
reviewed
Dec 22, 2020
lib/pry/helpers/text.rb
Outdated
| # @return [String] Text | ||
| def colorize(color, text, bold = false) | ||
| text | ||
| .then { |it| bold ? bold(it) : it } |
There was a problem hiding this comment.
Can you add tests to this? I believe this is breaking due to the override of the bold method by the bold argument.
There was a problem hiding this comment.
Yep, done. It's weird because the code was working well on my side.
I've simplified a few things, you can test with puts "\e[1;34mPry\e[0m" (bold blue) if you need
joallard
changed the title
|
I've also added Rebased on top of master for your utmost enjoyment. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Good people of pry-core, hello!
I was in the process of developing some fun console coloring options as part of Pry/StackExplorer and Pry/Rails, and wanting to allow users to customize their colors, I noticed the following:
If the color instructions (coloring, boldness) are offered as data, it is a bit unwieldy to turn that into colored text. For instance, if the config says
{color: :red, bold: true}, it's non-obvious to turn that into the right helper invocation.There's also the caveat that bold needs to be applied before color. (or is it the reverse?) I can never remember.
(FYI, I also have another patch in store that allows for faded text. Stay tuned)
Now this patch is a bit quick and dirty, so have at it. Is there some bandwidth and appetite at maintenance to merge this if the tests and the code are good? Let me know and I would get on that.