feat: Configure ANSI background colors separately #11830
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Enable the routine to print the ANSI terminal colors also in release builds. The colors are used to display e.g. git-diff. The colors are printed by setting the environment variable GIT_EXTENSIONS_CONSOLE_COLORS, seen in "Search files in commit" (git-grep) results.
c9dbaa5 to
8d7d47b
Compare
I'm finding the coloured background much easier on eyes. So, thank you. |
mstv
reviewed
Aug 8, 2024
|
FYI: git inconsistently highlights (leading whitespace of) removed and added moved lines: |
Specify foreground/background colors for the terminal separately, to enable themes that work better with GE colors. Apply theme colors for difftastic: Do not change background for bold text, to improve readability. Only reverse video for normal red/green.
Adjust the ANSI terminal colors to align with the GE theme. The background/foreground colors are separate to improve the display for the extremely bright GE colors. The theme is somehow based on mintty Helmhotz, but heavily modified especially for background (reverse video) to align with the existing GE theme colors.
If GE theme colors are applied, set moved lines as reverse video (colored background) to match the appearances
fc4a6a3 to
21e6752
Compare
gerhardol
added a commit
to gerhardol/gitextensions
that referenced
this pull request
Aug 9, 2024
…textensions#11830) Try to keep the current linenumber when switching to a new commit in file tree, instead of always resetting to first line.
Use the AnsiTerminal colors directly
21e6752 to
e850eeb
Compare
|
There are still some potential palette issues. GetForeColorForBackColor() tries to get a color from some pairs, using HSL perceived luminance, that is obviously not always the best. The pairs checked looks like this: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Supersedes #11824
Fixes #11821
Proposed changes
This PR has four serial parts, the last commit could be dropped.
Enable the routine to print the ANSI terminal colors also in release builds.
The colors are used to display e.g. git-diff.
Specify foreground/background colors for the terminal separately,
to enable themes that work better with GE colors.
Apply theme colors for difftastic: Do not change background
for bold text, to improve readability.
Only reverse video for normal red/green.
Adjust the ANSI terminal colors to align with the GE theme.
The background/foreground colors are separate to improve the
display for the extremely bright GE colors.
The theme is somehow based on mintty Helmhotz, but heavily modified
especially for background (reverse video) to align with the
existing GE theme colors.
If GE theme colors are applied, set moved lines as
reverse video (colored background) to match the appearances
Screenshots
Yellow foreground is still not great, but not much used by git-diff
The background (reverse) are to most used with the last commit
Before
no reverse video
reverse video
Not convinced that this is better, but maybe more predictable
Difftastic
Visibility is still not great, that need to be handled in difftastic itself
Test methodology
Tests are updated
Merge strategy
I agree that the maintainer squash merge this PR (if the commit message is clear).
✒️ I contribute this code under The Developer Certificate of Origin.