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

implement colorization for encoding.TextMarshaler #21

Merged
merged 1 commit into from
Nov 10, 2023
Merged

implement colorization for encoding.TextMarshaler #21

merged 1 commit into from
Nov 10, 2023

Conversation

hermannm
Copy link
Collaborator

@hermannm hermannm commented Nov 8, 2023

Changes (resolves #20)

  • add TextMarshaler field to Colors, defaulting to the same color as String
  • apply TextMarshaler color in encoder.encodeTextMarshaler
  • update null encoding in encodeTextMarshaler to use null color, like other encoder functions
  • update fatihcolor.Colors with new TextMarshaler field to match jsoncolor.Colors
  • add TestEncode_TextMarshaler to confirm that the color is applied properly

I believe I've gone through every use of Colors and made sure that it accounts for the new field. All tests pass on my machine.

One consideration in making TextMarshaler default to the same color as String is that this changes the behavior of the library: such values would previously have no color, but will now be colored as strings. However, I think this is more in line with the expected behavior, and since these are separate fields, users can still customize them separately as they wish.

This was referenced Nov 8, 2023
@neilotoole neilotoole merged commit 09171c9 into neilotoole:master Nov 10, 2023
3 checks passed
@neilotoole
Copy link
Owner

@hermannm I pulled and tested this locally against a private project that uses jsoncolor, and it all checks out 👍

Thank you for the quality of the PR btw! I'm going to use it as an example with my own team...

@hermannm hermannm deleted the textmarshaler-colorization branch November 10, 2023 21:43
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.

TextMarshaler colorization
2 participants