Skip to content

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

Merged
merged 1 commit into from
Apr 26, 2020

Conversation

twpayne
Copy link
Contributor

@twpayne twpayne commented Apr 24, 2020

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.

@twpayne
Copy link
Contributor Author

twpayne commented Apr 24, 2020

cc @mcuadros

@twpayne
Copy link
Contributor Author

twpayne commented Apr 24, 2020

It looks like the CI failures are unrelated to the changes in this PR.

@mcuadros
Copy link
Member

Looks promising, just remember that should be optional.
Thanks

@twpayne
Copy link
Contributor Author

twpayne commented Apr 24, 2020

Looks promising, just remember that should be optional.

Yes, it's optional.

The API for creating and using a UnifiedEncoder is unchanged:

ue := NewUnifiedEncoder(w, ctxtLines)

To enable color, you need to explicitly call UnifiedEncoder.SetColor to set a ColorConfig:

ue.SetColor(NewColorConfig()) // Use default git colors.

NewColorConfig returns the default colored config. You can also pass nil to disable colors (which is the default):

ue.SetColor(nil) // Disable colors.

@twpayne twpayne force-pushed the feature/diff-color branch 2 times, most recently from 05c4a54 to d2e1790 Compare April 25, 2020 23:53
@twpayne twpayne marked this pull request as ready for review April 26, 2020 00:02
@twpayne
Copy link
Contributor Author

twpayne commented Apr 26, 2020

This is now ready for review. Key points:

  • The API to enable color is optional and off by default. Existing code will continue to behave exactly as before.
  • I added a new plumbing/color package for functionality that exists in git's color.c and color.h files in the git source code.
  • The colorized diffs are sensible and match git's output given go-git's existing diff code.

The CI failure seems unrelated to the change in this PR.

Many thanks for any review :)

@@ -0,0 +1,38 @@
package color
Copy link
Member

@mcuadros mcuadros Apr 26, 2020

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

@twpayne twpayne force-pushed the feature/diff-color branch from d2e1790 to 743920c Compare April 26, 2020 18:53
@mcuadros mcuadros merged commit 2a62387 into go-git:master Apr 26, 2020
@twpayne twpayne deleted the feature/diff-color branch April 26, 2020 19:00
@twpayne
Copy link
Contributor Author

twpayne commented Apr 26, 2020

Thanks for the fast review and merge :)

@mcuadros
Copy link
Member

Thanks for the contribution :D

@mcuadros mcuadros changed the title Add initial colored diff support plumbing: diff, add colored output support May 24, 2020
traidare pushed a commit to traidare/go-git that referenced this pull request Oct 26, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Add initial colored diff support
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: colored diff output
2 participants