Skip to content

feat(pin): add ColorReset to Color type#3

Merged
yarlson merged 1 commit intoyarlson:mainfrom
havardthom:add_color_reset
Feb 13, 2025
Merged

feat(pin): add ColorReset to Color type#3
yarlson merged 1 commit intoyarlson:mainfrom
havardthom:add_color_reset

Conversation

@havardthom
Copy link
Copy Markdown
Contributor

Suggestion to add ColorReset to Color type.

I want to use pin color system in other cli output (non spinner output) to have consistent colors. Therefore it would be nice if it also contains the ANSI reset code so I don't have to declare it myself.

Great package btw!

@havardthom
Copy link
Copy Markdown
Contributor Author

havardthom commented Feb 13, 2025

Ah I see now that Color.getColorCode is private, then I guess I would need to duplicate the colors anyway. It would be great if the color system was a little more flexible, e.g. making getColorCode public and maybe allowing extending with more colors somehow. Or alternatively allowing ANSI color codes directly in functional options so one could use their own color system. Thoughts on this?

@yarlson
Copy link
Copy Markdown
Owner

yarlson commented Feb 13, 2025

Ah I see now that Color.getColorCode is private, then I guess I would need to duplicate the colors anyway. It would be great if the color system was a little more flexible, e.g. making getColorCode public and maybe allowing extending with more colors somehow. Or alternatively allowing ANSI color codes directly in functional options so one could use their own color system. Thoughts on this?

Actually, I was going to refactor it and make it public, for the same reason. It's probably better to rename it to String(). Then we can use Printf functions without bothering to call the function explicitly.

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (5249dae) to head (1c68d80).
Report is 3 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##              main        #3   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines          247       249    +2     
=========================================
+ Hits           247       249    +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yarlson
Copy link
Copy Markdown
Owner

yarlson commented Feb 13, 2025

Suggestion to add ColorReset to Color type.

I want to use pin color system in other cli output (non spinner output) to have consistent colors. Therefore it would be nice if it also contains the ANSI reset code so I don't have to declare it myself.

Great package btw!

Can you move the new constant to the end of the list? Otherwise it will change the values of the other constants due to the nature of iota.

@yarlson yarlson merged commit ca7db3e into yarlson:main Feb 13, 2025
5 checks passed
@yarlson
Copy link
Copy Markdown
Owner

yarlson commented Feb 13, 2025

@havardthom public String function is available in v0.9.0 release

@havardthom
Copy link
Copy Markdown
Contributor Author

Perfect! Thank you

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.

2 participants