Skip to content

fix: Force black foreground colors for bold in moved diff #11847

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Aug 25, 2024
Merged

fix: Force black foreground colors for bold in moved diff #11847

merged 1 commit into from
Aug 25, 2024

Conversation

gerhardol
Copy link
Member

Proposed changes

Always use black as foreground colors for moved lines. GE may calculate the color to be white, this is a an override to avoid the low contrast without risking breaking other scenarios.

This can be seen in #11843 (comment)
even if that issue is about something related.
I cannot reproduce, this is a good enough default configuration as theming is limited anyway.
(and colors can be overridden with git-config)

Note that colors set as "black brightyellow" instead of "black yellow bold" as that has a separate meaning - a separate PR will clarify.

Screenshots

white foreground is configured in debugger, cannot reproduce.

image
image

image
image

image
image

Test methodology

manual

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.

@gerhardol
fix: Force black foreground colors for bold in moved diff
fd1be01 
Always use black as foreground colors for moved lines.
GE may calculate the color to be white, this is a an override to avoid
the low contrast without risking breaking other scenarios.
@RussKie RussKie added this to the 5.0.0 milestone Aug 23, 2024
@gerhardol gerhardol mentioned this pull request Aug 23, 2024
Merged
mstv
mstv approved these changes Aug 24, 2024
View reviewed changes
Copy link
Member

@mstv mstv left a comment

Choose a reason for hiding this comment

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

LGTM

@RussKie RussKie merged commit 493fef4 into gitextensions:master Aug 25, 2024
4 checks passed
@snik0
Copy link

snik0 commented Sep 12, 2024

A note to anyone updating, after installing 5.0.0, all of my preview diffs for file modifications show the following error:

error: invalid color value: black brightmagenta
fatal: unable to parse 'color.diff.oldmoved' from command-line config

Git command (exit code: 128): -c color.ui=never -c diff.submodule=short -c diff.noprefix=false -c diff.mnemonicprefix=false -c diff.ignoreSubmodules=none -c core.safecrlf=false -c diff.colorMovedWS=no -c diff.wordRegex="[a-z0-9_]+|." -c diff.colorMoved=dimmed-zebra -c color.diff.old="red reverse" -c color.diff.new="green reverse" -c color.diff.oldMoved="black brightmagenta" -c color.diff.newMoved="black brightblue" -c color.diff.oldMovedAlternative="black brightcyan" -c color.diff.newMovedAlternative="black brightyellow" -c color.diff.oldMovedDimmed="magenta dim reverse" -c color.diff.newMovedDimmed="blue dim reverse" -c color.diff.oldMovedAlternativeDimmed="cyan dim reverse" -c color.diff.newMovedAlternativeDimmed="yellow dim reverse" diff --find-renames --find-copies --color=always --unified=3 "f194b395792e5e8536f7e14fecb0b93f2fd5a8ea" "3d2daab40d46e8a4d891415b54b2489e83d7132b" -- "tools/build_utils.py"

File additions previewed correctly however.

This was due to my git version being too old (2.25.1-windows)

Updating to 2.46.0 resolved the issue.

@gerhardol gerhardol deleted the feature/ansi-black-foreground branch September 12, 2024 12:33
@gerhardol
Copy link
Member Author

The oldest Git version supported is 2.25.0 but 2.46 is recommended.
2.25 is the version in Ubuntu 20.04, it may be restricted there.

Will have to investigate in which version the handling was changed.
Then to decide if this can be handled without compatibility issues or to raise the minimum version.

@gerhardol
Copy link
Member Author

Moved lines were added in Git 2.15, so that is supported.
The problem is "bright", supported first in Git 2.26.

@gerhardol gerhardol mentioned this pull request Sep 13, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RussKie RussKie RussKie approved these changes

@mstv mstv mstv approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
5.0.0
Development

Successfully merging this pull request may close these issues.
4 participants
@gerhardol @snik0 @RussKie @mstv