rickhowe / diffchar.vim

Highlight the exact differences, based on characters and words
http://www.vim.org/scripts/script.php?script_id=4932
MIT License
217 stars 10 forks source link

colorscheme a bit messed up #25

Closed ferdinandyb closed 2 years ago

ferdinandyb commented 3 years ago

I was expecting the plugin to use less highlighting than the default, but it actually highlights the entire line that was changed in addition to changes. Is it possible to only put highlights on actual changes? I see there's something about colors in the documentation, but as far as I see changing the default would just make it worse (more colors). It really made the text unreadable on my setup.

rickhowe commented 3 years ago

It sounds like the plugin does not work at all. Could you try to reset colorscheme to default (:colo default) and reset syntax to off (:syn off)? Please show me the screen dump and how to reproduce the problem.

ferdinandyb commented 3 years ago

Thank you for your time looking into this!

This is how vim -d file1.md file2.md looks with my setup without diffchar: image

This is how it looks like with diffchar plugged: image

This is the same but with :colo default and :syn off applied: image

This is with the following minimalrc:

call plug#begin('~/.vim/plugged')
Plug 'rickhowe/diffchar.vim'
call plug#end()

" Set compatibility to Vim only (i.e. no vi)
set nocompatible

image

My expectation was having something like the top picture, but with only quick and lazy highlighted, which would be super useful to say the least, especially with just text instead of code.

rickhowe commented 3 years ago

Thank you. I am wondering why in the 1st dump (without diffchar) the whole line is not highlighted. Vim always highlights the changed text with DiffText (red) and the rest with DiffChange (pink) like this:

aa

The plugin works well in my system like this;

bb

And also wondering why changed text is differently highlighted between 1st and others. The foreground color of 'lazy' and 'quick' is highlighted black in the 1st but white in others. Could you check if you are using the same colorscheme or same color for DiffChange and DiffText?

ferdinandyb commented 3 years ago

This is the relevant part of :highlight without diffchar image

and this is with diffchar image

so it seems that somehow when loading your plugin my original highlight colors are overwritten, to be precise, it seems the "links to DraculaX" parts are not there. Unfortunately, I have no idea how colorscheme setting works :(

On the other hand, now I can provide a real minimal vimrc to reproduce the problem. The order of the plugins don't seem to matter.


call plug#begin('~/.vim/plugged')
" should be coloring changes by char, but messes up colorscheme
Plug 'dracula/vim', { 'as': 'dracula' }
Plug 'rickhowe/diffchar.vim'
call plug#end()

" Set compatibility to Vim only (i.e. no vi)
set nocompatible
colorscheme dracula

I experimented a bit further. Opening vim with the above minimal vimrc (but not in diff mode), has the original "links to draculaX" formulas. Doing :windo diffthis leads to the above already copied overwriting, so I guess your plugin is only loaded for diff views, which makes sense :) Then executing :colorscheme dracula again leads to a third variation, instead of resetting the original colorscheme:

image

image

Any ideas why this is happening and how to get around this?

rickhowe commented 3 years ago

My plugin does not expect that Diff highlights are linked. The plugin overwrites the original (not linked) DIffChange and DiffText when diff mode begins and return them back when it ends. That is why the links are cleared.

In colors/dracula.vim, there are some exceptional highlights which are not linked, such as 'StatusLine' and 'MatchParen'. Diff highlights would also be better not linked.

I made the brief script as below which will remove the links from Diff highlights when diff mode begins and return those links when it ends. Please save and :so saved_file in your vimrc.

Another idea is to automatically switch colorchme between dracula and others in diff and non-diff mode by using autocmd like below.

augroup DiffHLLink
    autocmd!
    autocmd OptionSet diff call s:ToggleDiffHLLink()
augroup END

function! s:ToggleDiffHLLink()
    if v:option_old == v:option_new | return | endif
    let dw = 0
    for ti in gettabinfo()
        for wd in ti.windows
            let dw += getwinvar(wd, '&diff')
        endfor
    endfor
    if v:option_new
        if dw != 1 || exists('s:diff_hl_link') | return | endif
        let s:diff_hl_link = {}
        for hl in ['DiffAdd', 'DiffChange', 'DiffDelete', 'DiffText']
            let id = hlID(hl)
            let idl = synIDtrans(id)
            if id != idl
                let s:diff_hl_link[hl] = synIDattr(idl, 'name')
            endif
        endfor
        let lk = 'NONE'
        for hl in keys(s:diff_hl_link)
            call execute('highlight! link ' . hl . ' ' . lk)
        endfor
    else
        if dw != 0 || !exists('s:diff_hl_link') | return | endif
        for [hl, lk] in items(s:diff_hl_link)
            call execute('highlight! link ' . hl . ' ' . lk)
        endfor
        unlet s:diff_hl_link
    endif
endfunction
ferdinandyb commented 3 years ago

Thanks for the script! I put it in the minimalrc, but it doesn't seem to revert after exiting diffmode. But it would actually not solve my problem of wanting to retain my original colors in diffmode as well. I've been looking at what's happening, and I think I can actually turn this issue into a feature request.

So if I get it right, your plugin changes DiffAdd, DiffChange, DiffDelete, and DiffText and defines three new highlights: image

I played around with changing these 7 after everything was loaded and haven't been able to get exactly what I wanted, but I think in general it would be nice to be able to set the colors explicitly. Maybe an option for leaving the default highlight (DiffX) as they are and options for setting the dcDiffX highlights manually? I might not have understood the purpose of all highlights though ...

I think being able to control the highlights explicitly would benefit many people, since the current method of changing just the diffs leaves me with a basically unreadable coloring due to other non-default colors staying the same and there are probably many other colorschemes out there that are not very compatible with the colors set by your plugin.

rickhowe commented 3 years ago

OK, I will get it as an enhancement request.

As a workaround, I would recommend to modify colors/dracula.vim as below not to link but directly change Diff highlights as same as 'CursorLine'.

--- draculaOrig.vim 2021-01-03 14:06:59.368708800 +0900
+++ dracula.vim 2021-01-03 23:36:57.448786400 +0900
@@ -207,15 +207,16 @@
 call s:h('WildMenu', s:bg, s:purple, [s:attrs.bold])
 call s:h('CursorLine', s:none, s:subtle)

+call s:h('DiffAdd', s:green)
+call s:h('DiffChange', s:orange, s:none)
+call s:h('DiffText', s:bg, s:orange)
+call s:h('DiffDelete', s:red, s:bgdark)
+
 hi! link ColorColumn  DraculaBgDark
 hi! link CursorColumn CursorLine
 hi! link CursorLineNr DraculaYellow
-hi! link DiffAdd      DraculaGreen
 hi! link DiffAdded    DiffAdd
-hi! link DiffChange   DraculaDiffChange
-hi! link DiffDelete   DraculaDiffDelete
 hi! link DiffRemoved  DiffDelete
-hi! link DiffText     DraculaDiffText
 hi! link Directory    DraculaPurpleBold
 hi! link ErrorMsg     DraculaRedInverse
 hi! link FoldColumn   DraculaSubtle

And also modify autoload/diffchar.vim, which is the latest 8.7 version, as below.

--- dcB87.vim   2020-05-02 00:41:30.000000000 +0900
+++ dcA87z.vim  2021-01-03 23:36:26.556510800 +0900
@@ -60,10 +60,8 @@
        let dh.id = id
        " 0: original, 1: for single color, 2: for multi color
        let dh.0 = filter(at, '!empty(v:val)')
-       let dh.1 = (hl == 'DiffChange' || hl == 'DiffText') ?
-                                   \filter(copy(at), 'v:key =~ "bg$"') : dh.0
-       let dh.2 = (hl == 'DiffChange') ? dh.1 :
-                                           \(hl == 'DiffText') ? {} : dh.0
+       let dh.1 = (hl == 'DiffText') ? {} : dh.0
+       let dh.2 = dh.1
        let s:DiffHL[hl] = dh
    endfor
    " adjust id to be equal to diff_hlID() in nvim

Then,

image

will be:

image

ferdinandyb commented 3 years ago

Thank you the patches are perfect! I also appreciate you considering the feature request. Although there's probably a better way to approach it than my outline :)

ferdinandyb commented 2 years ago

I checked because of a followup on https://github.com/dracula/vim/pull/275#issuecomment-1215852522, with the latest from both plugins and this is the current diff situation:

image image

I'm at a bit of a loss as to what a good resolution to this would be. Maybe @benknoble and @rickhowe you see this better?

benknoble commented 2 years ago

Is there text between quick and lazy in both lines?

benknoble commented 2 years ago

My plugin does not expect that Diff highlights are linked. The plugin overwrites the original (not linked) DIffChange and DiffText when diff mode begins and return them back when it ends. That is why the links are cleared.

Perhaps using the new hlget()/hlset() APIs to save/restore would work?

I'm not sure why you recommend Dracula not link Diff* groups; I can see why that would help this plugin work, but is that good general advice (why?) or just to solve this problem? If the former it would be more compelling for us to make the proposed change.

ferdinandyb commented 2 years ago

@benknoble yes, it's "The quick brown fox jumps over the lazy dog" on both lines, with quick and lazy switched (hence the only the words that should be highlighted).

rickhowe commented 2 years ago

(posted again:) I thought I had fixed this problem but I found I made a regression on the latest version, 9.0. Could you try to modify your autoload/diffchar.vim like this and let me know if it is fixed with your colorschme?

--- dcA90.vim   2022-06-03 21:54:13.344445400 +0900
+++ dcA90x.vim  2022-08-23 18:42:00.285420900 +0900
@@ -293,6 +293,7 @@
        let dh.2 = (hs == 'T') ? {} : dh.1
        let s:DiffHL[hs] = dh
    endfor
+   if empty(s:DiffHL.C.1) | let s:DiffHL.T.1 = {} | endif
    let s:DCharHL = {'A': 'DiffAdd', 'D': 'DiffDelete', 'n': 'LineNr'}
    if has('nvim')
        let s:DCharHL.c = 'TermCursor'
ferdinandyb commented 2 years ago

Thanks, it works perfectly with that modification!

rickhowe commented 2 years ago

Thank you. I updated the plugin as 9.01.

ferdinandyb commented 2 years ago

Thanks, upgrade from the repo and confirmed everything is working as expected, so I'm going to close this and the related commit on dracula as well, thank you both!