feat: Allow enhanced diff highlight customization

[antosha417]

Hey! I really enjoy using this plugin and like how enhanced_diff_hl looks. The idea that we can apply different highlight groups to different windows blew my mind. So I want to take it even further and allow user customization by allowing enhanced_diff_hl be an object.

What do you think about it?

I'll add this feature to the docs if you would like to merge it.

The config will look like this:

require("diffview").setup({
  enhanced_diff_hl = {
    a = {
      "DiffAdd:DiffviewDiffAddAsDelete",
      "DiffDelete:DiffviewDiffDelete",
      "DiffChange:DiffviewDiffChangeAsDelete",
      "DiffText:DiffviewDiffTextRed",
    },
    b = {
      "DiffDelete:DiffviewDiffDelete",
      "DiffChange:DiffviewDiffChangeAsAdd",
      "DiffText:DiffviewDiffTextGreen",
    }
  }
 })

[sindrets]

Thanks for the PR!

I've been considering allowing more customization for the enhanced_diff_hl, but I'm not sure this is the right approach. Your suggested implementation would kind of lock us into only offering enhanced highlights for the 2-way split layouts. I'm gonna take some time to think about what the best approach would be.

I would like to also point out that you can actually already do this by using the hooks. Take a look at how I'm customizing the highlights in my own dotfiles.

I'll also link an issue where someone asked about something similar: #241

[antosha417]

Thank you! I managed to get it working using hooks. Didn't understand the part view.emitter:on("post_layout", post_layout) though. I think it should be easier to configure the colors without getting into internal parts of the plugin.

In this pr I suggested to accept a table instead of boolean in enhanced_diff_hl setting. Now after I copied this hook from you I think that we can instead accept a function. That function would return an object like.

{
  a = {
    winhl = {
        "DiffAdd:DiffviewDiffAddAsDelete",
        "DiffDelete:DiffviewDiffDelete",
      },
  b = {...}, 
  c = {...},
  ...
}

What do you think about this approach?

[sindrets]

Sorry for the late response! I've concluded that the use-case for this is too specific to warrant its own config option. I've opted to instead provide more context data to the hooks, which are much better suited for this sort of customization, as they allow for dynamic changes and don't rigidly limit the type of customization.

As an example, adjusting the 'winhl' only for diff2* layouts can now be done like this:

require("diffview").setup({
  hooks = {
    diff_buf_win_enter = function(bufnr, winid, ctx)
      if ctx.layout_name:match("^diff2") then
        if ctx.symbol == "a" then
          vim.opt_local.winhl = table.concat({
            "DiffAdd:DiffviewDiffAddAsDelete",
            "DiffDelete:DiffviewDiffDelete",
          }, ",")
        elseif ctx.symbol == "b" then
          vim.opt_local.winhl = table.concat({
            "DiffDelete:DiffviewDiffDelete",
          }, ",")
        end
      end
    end,
  }
})

See the updated docs for more info about the hook callback parameters:

:h diffview-config-hooks

[sindrets]

Regardless, thank you for your work, and for taking an interest in the project!

[antosha417]

Thank you for the new approach! Will give it a try tomorrow

[tummetott]

I really like the approach, how you can style individual windows inside a diffview. One question remains:

When filtering for 3 way diffs (in fact a merge conflict) with the following code, the ctx.symbol is empty.

if ctx.layout_name:match("^diff3") then
    vim.pretty_print(ctx)
end

the output is:

{
  layout_name = "diff3_horizontal"
}
{
  layout_name = "diff3_horizontal"
}
{
  layout_name = "diff3_horizontal"
}

Is this intended behaviour? I'd also love to style my colors for a merge conflict.

I'd expect symbols like a b c or ours local theirs

[sindrets]

@tummetott Thank you for reporting! No, that wasn't intended behavior. Should be fixed now.