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

CC-1249: Colorize first differing bytes in Visualize method #31

Merged
merged 7 commits into from
Jun 4, 2024

Conversation

ryan-gang
Copy link
Contributor

No description provided.

Copy link

linear bot commented May 27, 2024

@ryan-gang ryan-gang self-assigned this May 27, 2024
@ryan-gang ryan-gang requested a review from rohitpaulk May 27, 2024 11:13
@ryan-gang ryan-gang requested a review from rohitpaulk June 3, 2024 14:51
}
}

func formatAsciiWithColorizedByteHelper(value []byte, i int, firstDiffIndex int, end int, chosenColor color.Attribute) string {
Copy link
Member

Choose a reason for hiding this comment

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

@ryan-gang there's still a bunch of complexity here around byte indexes - the main reason for all of this seems to be that we're using formatBytesAsHex instead of formatByteAsHex. Would things be simpler if we were printing out byte-by-byte instead of constructing a whole line at once and then having to mutate it?

This is certainly cleaner than what we were doing to the TabWriter output, but I think it can be far simpler. Good enough to merge, but I'd suggest giving this another go if you're looking to practice code design skills - good opportunity!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Merging this for now, will work on the other implementation separately.
We can actually inline all the hex and ASCII conversion, to make this simpler, that way we can go byte-by-byte as you mentioned.

@ryan-gang ryan-gang merged commit f721587 into master Jun 4, 2024
1 check passed
@ryan-gang ryan-gang deleted the CC-1249 branch June 4, 2024 06:54
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.

None yet

2 participants