Skip to content
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

Option to use negative style for inline change highlights #374

Open
waldyrious opened this issue May 16, 2020 · 25 comments
Open

Option to use negative style for inline change highlights #374

waldyrious opened this issue May 16, 2020 · 25 comments

Comments

@waldyrious
Copy link

I've been using the diff-highlight script that comes with git to improve my diffs' output, and I was looking to switch to diff-so-fancy for the extra improvements it does to the diff output; however, in terms of colors, I think diff-highlight's coloring strategy (of simply using the negative style of the same red/green color used for the non-highlighted parts of the line) works better than what diff-so-fancy does:

diff-highlight diff-so-fancy
diff-highlight diff-so-fancy

I like how all the highlight colors from diff-highlight preserve the existing foreground red/green colors, whereas diff-so-fancy introduces a new shade of green for the highlighted part. I suspect that's also why the lines with highlighted changes become bold/bright (to improve contrast). I'd like if diff-so-fancy had an option to use the simpler strategy that diff-highlight uses.

Now, I realize that the current behavior may be preferable, so IMO a command-line flag, or git config option, to behave like diff-highlight would be perfectly acceptable ways to address this.

@OJFord
Copy link
Member

OJFord commented May 16, 2020

Are you sure you haven't just blindly copied:

git config --global color.diff-highlight.oldNormal    "red bold"
git config --global color.diff-highlight.oldHighlight "red bold 52"
git config --global color.diff-highlight.newNormal    "green bold"
git config --global color.diff-highlight.newHighlight "green bold 22"

from the readme? 🙂

Highlighting in d-s-f is entirely delegated to diff-highlight, which you can leave at its defaults or configure however you like.

@waldyrious
Copy link
Author

I did look at my configs, to be sure (I set this up a while ago), but none of the settings from the README were defined there:

$ git config --list | grep color
color.diff=auto
color.interactive=auto
color.status=auto
color.ui=auto
color.branch.current=yellow reverse
color.branch.local=yellow
color.branch.remote=green
color.diff.meta=yellow bold
color.diff.frag=magenta bold
color.diff.old=red
color.diff.new=green
color.status.added=green
color.status.changed=yellow
color.status.untracked=cyan

@waldyrious
Copy link
Author

Highlighting in d-s-f is entirely delegated to diff-highlight

Note that I set diff-highlight manually, following these instructions. Does DSF bring its own copy of it?

@OJFord
Copy link
Member

OJFord commented May 16, 2020

That's up to the packager (for whatever package manager you used to install it) really - I think the only first-party package (other than the GitHub releases of course) is npm. But unless you're on a platform that distributes git without it, (v. rare) I don't see why they would.

@scottchiefbaker I'm not lying about this, am I? --

Highlighting in d-s-f is entirely delegated to diff-highlight

I tried to read some perl, and briefly thought it actually was just inverting the colour instead of using diff-highlight's, but then realised - as far as I can tell - that's only in the case of the subroutine for empty lines.

@waldyrious
Copy link
Author

waldyrious commented May 16, 2020

Highlighting in d-s-f is entirely delegated to diff-highlight

The script seems to be almost identical to git's copy. It sure looks like those differences are what's causing the discrepancy.

@OJFord
Copy link
Member

OJFord commented May 17, 2020

Oh sorry, I didn't realise that was included. Nor do I know anything about perl to be able to say whether it needs to be, or if it can still be 'imported' and used as a package like this from the copy distributed with git. I'll let Scott answer on whether those differences can be updated, or if we can even just use upstream's directly. 🙂

@james-antill
Copy link

FWIW I ended up here because the default highlighting is unreadable for my poor old eyes (looks smudged). I ended up with:

[color "diff-highlight"]
oldHighlight = reverse red
oldReset = noreverse
newHighlight = ul green
newReset = noul

...and it works great.

@scottchiefbaker
Copy link
Contributor

Can this issue be closed then?

@waldyrious
Copy link
Author

waldyrious commented May 28, 2021

Can this issue be closed then?

I don't follow — why do you say so? The request is for a CLI option to make diff-so-fancy use the same coloring scheme as git's own diff-highlight script, and IIUC that is not yet available.

That said, it seems like it may be worth first addressing the discrepancy between this repo's copy of the diff-highlight script and the upstream equivalent (raised in the exchange above with @OJFord).

@OJFord
Copy link
Member

OJFord commented May 28, 2021

@scottchiefbaker Is diff-highlight just here for use in testing, or is it somehow loaded in perl from that relative directory?

Is there a reason (avoiding shelling out?) not just to use it from PATH, i.e. the one distributed with git?

@scottchiefbaker
Copy link
Contributor

I see... I misunderstood.

Are you setting any diff-hightlight colors in your git config? Or just relying on the colors that are hardcoded in diff-highlight itself?

@scottchiefbaker
Copy link
Contributor

@OJFord we had to make some minor modifications to the library itself. Mainly to decrease startup time. Vanilla diff-highlight shells out to git config four times which really slows down startup.

Also we changed some my variables to our so we can externally change the colors it uses. d-s-f most definitely uses the modified version in lib/ even if the system already has a version in the path.

I have seen very little development on diff-highlight over the years, so I haven't really worried about making it a drop-in replacement. We just use a one-offed version for our needs.

@waldyrious
Copy link
Author

Are you setting any diff-hightlight colors in your git config? Or just relying on the colors that are hardcoded in diff-highlight itself?

AFAIK, no — but just to make sure, is what I wrote above sufficient information, or are you referring to something not covered there?

@scottchiefbaker
Copy link
Contributor

@waldyrious if that is your complete git config then you have not set any diff-highlight specific colors. The defaults for the d-s-f version of diff-highlight are probably just different (we made them to match the screenshot).

If you want the default colors (or any other colors) then simply set them in your git config using something the docs.

@waldyrious
Copy link
Author

I know that I can tweak it manually :) what I am asking for here is a feature (a command-line flag, or a custom git config option, as I mentioned in my opening comment) that would allow anyone to easily emulate the style that comes by default with git's own diff-highlight, without having to fiddle with the config values by hand. This would be helpful to those who are adopting d-s-f for the first time, and to those (like me) who simply prefer the default highlighting style over d-s-f's style.

scottchiefbaker added a commit to scottchiefbaker/diff-so-fancy that referenced this issue May 31, 2021
@scottchiefbaker
Copy link
Contributor

scottchiefbaker commented May 31, 2021

I just reverted the diff-highlight colors to the defaults... check out the latest on next and tell me if that addresses your issue. I probably just never should have tweaked the diff-highlight colors at all :)

@waldyrious
Copy link
Author

First of all, thanks so much for considering making that the default in diff-so-fancy as well! However, I'm not really seeing the same output yet (note: I had to check out the next branch in your fork, because it hasn't been pushed to this repo yet 🙂).

Here's what I did to test:

$ git remote add scott https://github.com/scottchiefbaker/diff-so-fancy
$ git fetch scott
$ git checkout scott/next
$ git -c 'core.pager=diff-highlight' show HEAD~
$ git show HEAD~ | diff-so-fancy
$ git show HEAD~ | ./diff-so-fancy

Screenshot of the output:

Screenshot 2021-05-31 at 22 11 11

Let me know if I'm missing anything.

@scottchiefbaker
Copy link
Contributor

OK try the next branch again... I think I got it respecting the diff-highlight defaults.

@waldyrious
Copy link
Author

Yeah! It's pretty much identical now, with the only difference being the use of bold. Is that intentional?

Screenshot 2021-06-01 at 11 07 35

@scottchiefbaker
Copy link
Contributor

I cannot recreate this... Try clearing ALL the color options from both ~/.gitconfig and .git/config and see if that resolves the problem?

You can view the decoded ANSI codes with something like:

git diff --color | diff-so-fancy | third_party/ansi-reveal/ansi-reveal.pl

In my case I see:

[BOLD][COLOR001][RED][RESET][RED]## History[RESET][RESET]
[BOLD][COLOR002][GREEN][RESET][GREEN]Newline[RESET][RESET]

It bolds, and then resets before the added/removed lines. The resulting output is not bolded from what I see.

@waldyrious
Copy link
Author

Try clearing ALL the color options from both ~/.gitconfig and .git/config and see if that resolves the problem?

I had already done so, and double-checked now.

You can view the decoded ANSI codes

Thanks, I've tried that — here's what I see:

❯ git show HEAD~ | ./diff-so-fancy  | third_party/ansi-reveal/ansi-reveal.pl
5c4da1c - Correct the diff-highlight defaults (22 hours ago) <Scott Baker>

[RESET][COLOR011]───────────────────────────────────────────────[RESET]
[COLOR011]modified: lib/DiffHighlight.pm
[COLOR011]───────────────────────────────────────────────[RESET]
[BOLD][COLOR013]@ lib/DiffHighlight.pm:15 @[BOLD][BOLD][COLOR146] my $NULL = File::Spec->devnull();[RESET]
# Highlight by reversing foreground and background. You could do
# other things like bold or underline if you prefer.
our @OLD_HIGHLIGHT = (
[BOLD][COLOR001]	[UKN: 7]""[UKN: 27],[RESET]
[BOLD][COLOR002]	[UKN: 7]undef[UKN: 27],[RESET]
	"\e[7m",
	"\e[27m",
);

Color-coded, for convenience:

Screenshot 2021-06-01 at 22 53 08

I don't know what could be interfering :( @OJFord any chance you could give it a try so we could compare?

@OJFord
Copy link
Member

OJFord commented Jun 2, 2021

(With none in config)

$git show 5c4da1c | ./diff-so-fancy  | third_party/ansi-reveal/ansi-reveal.pl
[...]
[RESET][COLOR011]──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────[RESET]
[COLOR011]modified: lib/DiffHighlight.pm
[COLOR011]──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────[RESET]
[BOLD][COLOR013]@ lib/DiffHighlight.pm:15 @[BOLD][BOLD][COLOR146] my $NULL = File::Spec->devnull();[RESET]
# Highlight by reversing foreground and background. You could do
# other things like bold or underline if you prefer.
our @OLD_HIGHLIGHT = (
[BOLD][COLOR001]	[REVERSE]""[NOTREV],[RESET]
[BOLD][COLOR002]	[REVERSE]undef[NOTREV],[RESET]
	"\e[7m",
	"\e[27m",
);

@waldyrious
Copy link
Author

Huh, intriguing. So what you see as [REVERSE] and [NOTREV], I see as [UKN: 7] and [UKN: 27], respectively. Maybe it's shell-dependent? I'm using iTerm2 on a macOS, in case that helps.

(still, it's puzzling that diff-highlight is able to show the reverse colors in the same shell...)

@scottchiefbaker
Copy link
Contributor

scottchiefbaker commented Jun 2, 2021

@waldyrious I updated the ansi_reveal.pl yesterday to show REV/NOTREV after your first post

Something is still setting bold but I have no idea what!

@waldyrious
Copy link
Author

@waldyrious I updated the ansi_reveal.pl yesterday to show REV/NOTREV after your first post

I can confirm, I've pulled the latest changes, and now I get exactly the same output as @OJFord.

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

No branches or pull requests

4 participants