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

Add functions to style text (colors, weight, etc) #140

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

gustavothecoder
Copy link

@gustavothecoder gustavothecoder commented Dec 2, 2024

Inspired by the issue #121, I added a module with functions to change text style. The doc docsite/source/styling-your-output.html.md demonstrate how it works. I used the thor implementation as a basis, but I changed the API to make combinations shorter:

# Thor API
set_color "Hi!", :red, :on_white, :bold
# Implemented API
red(on_white(bold("Hi!")))

Resolve #121

@gustavothecoder
Copy link
Author

Maybe this API is a better idea because it looks more like Ruby:

stylize("Hi!").red.on_white.bold

In this case, stylize will return an object with methods to style the text.

@cllns
Copy link
Member

cllns commented Dec 5, 2024

Maybe this API is a better idea because it looks more like Ruby:

stylize("Hi!").red.on_white.bold

In this case, stylize will return an object with methods to style the text.

First of all, thanks for doing this!

When I first read this PR, I also thought of this API :)

Want to go for it?

Also, can you extract some constants like BLUE = 34 and refactor from based on that?

@gustavothecoder
Copy link
Author

Maybe this API is a better idea because it looks more like Ruby:

stylize("Hi!").red.on_white.bold

In this case, stylize will return an object with methods to style the text.

First of all, thanks for doing this!

When I first read this PR, I also thought of this API :)

Want to go for it?

Also, can you extract some constants like BLUE = 34 and refactor from based on that?

Yeah, I'll let you know when I'm done refactoring.

@gustavothecoder gustavothecoder force-pushed the add-colors branch 2 times, most recently from c5934e3 to ad75bc1 Compare December 8, 2024 12:58
@gustavothecoder
Copy link
Author

Hey @cllns, I'm done refactoring and I think that the solution is better now, what you think?

image

Copy link
Member

@cllns cllns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for that refactoring! In reviewing it, I found a few more we can do as well

lib/dry/cli/styles.rb Show resolved Hide resolved
lib/dry/cli/styles.rb Outdated Show resolved Hide resolved
lib/dry/cli/styles.rb Outdated Show resolved Hide resolved
- Changed the API to `stylize("string").bold.blue.on_white`
- Moved magic numbers to constants
- Improve code reuse and legibility
@gustavothecoder
Copy link
Author

Thanks for that refactoring! In reviewing it, I found a few more we can do as well

Thanks for the feedback! Can you review it again, please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature request: colorized output
2 participants