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

Diff highlighing not replaced by text property highlighting when combine false #14966

Open
errael opened this issue Jun 11, 2024 · 9 comments
Open
Labels

Comments

@errael
Copy link
Contributor

errael commented Jun 11, 2024

Steps to reproduce

In help's syntax.txt, there is a section labeled "DIFF". This seems
to imply that the diff highlighing is syntax highlighting. In textprop.txt
it says:

"combine"       when omitted or TRUE the text property highlighting is
                combined with any syntax highlighting; when FALSE the
                text property highlighting replaces the syntax
                highlighting

NOTE: "text property highlighting replaces the syntax highlighting"

To see the problem do

  1. Put the test below in a file.
  2. Start up gvim in diff mode, gvim -d f1 f2 with files that have differences.
  3. Source the test file, :source <test-file>, can do it for both sides of the diff.
  4. Observe: the diff highlighing is still visible
    Note to clear can do: :vim9cmd prop_remove({id: 555})

Test file

vim9script

def DoIt()
    var lnum = 1
    while lnum <= line('$')
        var col = col([lnum, '$']) - 1
        prop_add(lnum, 1, { type: 'prop_hl', length: col, id: 555 })
        lnum += 1
    endwhile
enddef

try
    'prop_hl'->prop_type_add({ highlight: 'WildMenu', priority: 100, combine: false })
catch
endtry

DoIt()

Expected behaviour

I may be reading the docs wrong.

When editing a file with syntax highlighting, sourcing the test-file overrides the highlighting as expected.

How can I set a text property that overrides the diff highlights. (this is for a tool that allows merge conflict editing).

Version of Vim

9.1.474

Environment

linux/gtk3

Logs and stack traces

No response

@errael errael added the bug label Jun 11, 2024
@errael
Copy link
Contributor Author

errael commented Jun 11, 2024

As mentioned, with combine: false, syntax highlighting is overridden. But doing combine: true, the syntax also seems to be overridden. What does combine: true do? How can I see the effect?

@chrisbra
Copy link
Member

I believe diff-highlighting works on a different level than syntax highlighting. I am not sure, if diff highlighting can be overridden by text-properties at all (haven't used it much yet). Can you try your example with some real syntax highlighting, e.g. vim script files?

@errael
Copy link
Contributor Author

errael commented Jun 12, 2024

I didn't understand what combine means. I had a sense that it meant merge/blend like with colors; now I'm guessing that there separate "fields" in a highlight spec and when combine is set that fields not already set are "added" from the new text property.

What does combine mean?
I can't find combine defined anywhere, not in syntax.h not in textprop.h (and might there be some terminal dependencies?). A scan of :he highlight (which I've never really read) is helpful, and a more careful reading is in order. But what/how highlights combine is somewhat of a mystery.

Using a modified test file, see below, with underline and combine: true. Doing gvim -d f1.txt f2.txt and applying the test file. I see the underline combining, except for DiffDelete. That may be because DiffDelete defines a guifg and gui=underline is part of that "field"; who knows?. Note that if I set combine: false, I see the same result; I expect it to replace the existing (but that's the issue, opinions may vary).

diff-combine

Here's the two files in the diff:
f1.txt
f2.txt

The updated test file

vim9script

# to clear: ":vim9cmd prop_remove({id: 555})"
# and there's: 'prop_hl'->prop_type_delete()

def DoIt()
    var lnum = 1
    while lnum <= line('$')
        var col = col([lnum, '$']) - 1
        prop_add(lnum, 1, { type: 'prop_hl', length: col, id: 555 })
        lnum += 1
    endwhile
enddef

highlight clear TestHL
#highlight TestHL guibg=#eeeeee
highlight TestHL gui=underline

try
    'prop_hl'->prop_type_add({ highlight: 'TestHL', priority: 100, combine: true })
catch
endtry

DoIt()

@errael
Copy link
Contributor Author

errael commented Jun 12, 2024

Here's an example with combine: true, underline, guibg and syntax highlighting

doit

which looks a lot like expected. Comparing it with diff above, it's interesting that the underline shows up and doesn't interfere with the foreground. I don't know if this indicates the the diff highlighting has a custom combiner.

@errael
Copy link
Contributor Author

errael commented Jun 12, 2024

Using a file with syntax for the diff is interesting
diff-syn

applying the text-prop, but not to the last few lines, seems to definitely show that the combiner for diff is unique. Notice how the text-prop do not combine with the syntax like they do in the previous example. In particular look at the line

        var col = col([lnum, '$']) - 1

See how it combines in a normal file, or in a diff file without text props. But add the text props and all the syntax highlighting is lost.

comb-diff-syn

@chrisbra
Copy link
Member

chrisbra commented Jun 12, 2024

Yes, when combine is not set, you are loosing the syntax highlighting attributes (because for the part where the text properties are attached, those take priority over the syntax highlighting). If you still want the highlighting, you need to set combine. However please note, diff highlighting is an additional of highlighting and is not directly related to syntax highlighting and therefore it may not apply

@errael
Copy link
Contributor Author

errael commented Jun 12, 2024

Could/should diff highlighting interact is a well defined way with text-props? Yes, it may not apply, but should it?

I vaguely recall having seen a comment about this somewhere some time ago, but couldn't find anything in todo.

One possibility is to have diff behave like syntax when it comes to text properties. Best case (simplest I think) it would only mean adjusting the diff combiner.

Maybe treating the diff highlight like a prop, at least for combining, would work. There could even be an option for what priority for it to use.

@errael
Copy link
Contributor Author

errael commented Jun 12, 2024

If you still want the highlighting, you need to set combine

Right. But as the last examples show, with combine: true, diff still seems misbehaved.

@errael
Copy link
Contributor Author

errael commented Jun 12, 2024

BTW, I haven't looked at any code. Wanted to explore first and see if some new behavior would be acceptable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants