fix: bold attributes applies to both fore/back #11849
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
RussKie
reviewed
Aug 24, 2024
RussKie
approved these changes
Aug 25, 2024
|
An alternative to use AnsiEscapeUtilities.cs:PrintColors() in GitExtensions is to clone the following repo and view the following file as a diff. https://github.com/gerhardol/tmp_tests/blob/feature/ansi-color-template/AnsiTerminalColors.diff I would like to include this in 5.0 release. |
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.
Proposed changes
Related to #11847
The handling of ANSI escape sequences was probably not completely correct (there is no clear spec...).
All direct attributes applies to all colors specified.
So git colors "black bold red" and "black bold red" are translated to the sequence "1, 30, 41" which will render the same as "brightblack brightred" ("90, 101"), but "black brightred" ("30, 101") is different.
Attributes should not be "persistent" if a "bright" color is set explicitly.
The implementation is slightly simplified too.
In reality, the implementation would not be a major issue as attributes are not often used in combination like this.
Test methodology
Tests are mostly unchanged.
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.