-
Notifications
You must be signed in to change notification settings - Fork 787
plumbing: diff, add colored output support #40
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
Conversation
cc @mcuadros |
It looks like the CI failures are unrelated to the changes in this PR. |
Looks promising, just remember that should be optional. |
Yes, it's optional. The API for creating and using a ue := NewUnifiedEncoder(w, ctxtLines) To enable color, you need to explicitly call ue.SetColor(NewColorConfig()) // Use default git colors.
ue.SetColor(nil) // Disable colors. |
05c4a54
to
d2e1790
Compare
This is now ready for review. Key points:
The CI failure seems unrelated to the change in this PR. Many thanks for any review :) |
plumbing/color/color.go
Outdated
@@ -0,0 +1,38 @@ | |||
package color |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can we move this to internal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
d2e1790
to
743920c
Compare
Thanks for the fast review and merge :) |
Thanks for the contribution :D |
Add initial colored diff support
Fixes #33.
This is a draft PR. Please tell me if it more-or-less follows the project's coding style and what changes are likely to be needed to make it more acceptable.
At the moment, it only supports the default git diff colors as defined by git 2.26.2. It should be extensible to reading custom colors from a git config file, if needed.
Before merging, this needs a few tweaks to ensure the output matches git's as far as reasonably possible.