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

Add git signs to neovim highlight groups for coloring #31

Closed
theKnightsOfRohan opened this issue Feb 5, 2024 · 7 comments
Closed

Add git signs to neovim highlight groups for coloring #31

theKnightsOfRohan opened this issue Feb 5, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@theKnightsOfRohan
Copy link
Contributor

Describe the problem you're having
As title says.

Proposed solution(s)
I'm currently looking into the actual code of the plugin to potentially handle it myself, but my first impression is that this should be placed in either syntax_highlighting.lua or view.lua.

Additional context
N/A.

@theKnightsOfRohan theKnightsOfRohan added the enhancement New feature or request label Feb 5, 2024
@simonmclean
Copy link
Owner

The relevant code is here. What I'm doing is registering the signs like vim.fn.sign_define(sign_name, { text = text }), where the text value comes from git_signs.signs in the user config.

sign_define has a lot more options than I'm exposing in the config, so this can definitively be improved.

Perhaps I just expose all these options from sign_define?

The optional {dict}
		argument specifies the sign attributes.  The following values
		are supported:
		   icon		full path to the bitmap file for the sign.
		   linehl	highlight group used for the whole line the
				sign is placed in.
		   numhl	highlight group used for the line number where
				the sign is placed.
		   text		text that is displayed when there is no icon
				or the GUI is not being used.
		   texthl	highlight group used for the text item
		   culhl	highlight group used for the text item when
				the cursor is on the same line as the sign and
				['cursorline'](https://neovim.io/doc/user/options.html#'cursorline') is enabled.

And potentially allow the user to specify a pre-existing sign as well?

@theKnightsOfRohan
Copy link
Contributor Author

Currently, I have the below working as intended, with no upfront complications:

  if u.is_empty(vim.fn.sign_getdefined('TriptychGitAdd')) then
    vim.fn.sign_define('TriptychGitAdd', { text = signs_config.add, texthl =  'TriptychSignGitAdd' })
  end
  if u.is_empty(vim.fn.sign_getdefined('TriptychGitModify')) then
    vim.fn.sign_define('TriptychGitModify', { text = signs_config.modify, texthl =  'TriptychSignGitModify' })
  end
  if u.is_empty(vim.fn.sign_getdefined('TriptychGitRename')) then
    vim.fn.sign_define('TriptychGitRename', { text = signs_config.rename, texthl = 'TriptychSignGitRename' })
  end
  if u.is_empty(vim.fn.sign_getdefined('TriptychGitUntracked')) then
    vim.fn.sign_define('TriptychGitUntracked', { text = signs_config.untracked, texthl = 'TriptychSignGitUntracked' })
  end

  vim.cmd("highlight TriptychSignGitAdd ctermfg=114 guifg=#98c379")
  vim.cmd("highlight TriptychSignGitModify ctermfg=180 guifg=#e5c07b")
  vim.cmd("highlight TriptychSignGitRename ctermfg=39 guifg=#61afef")
  vim.cmd("highlight TriptychSignGitUntracked ctermfg=204 guifg=#e06c75")

It's a little janky as is, because I had to unroll the loop over the signs_to_text table. I can probably update the config to include a color as a string, in the form "guifg=#000000" or "ctermfg=210" and concatenate that to the vim.cmd argument, but by default include both. I could also update the signs_to_text table to work nicer, so it can be put back in the loop. What do you think?

@theKnightsOfRohan
Copy link
Contributor Author

I've made the edits on my fork of the project, the new color config looks like this:

    git_signs = {
      enabled = true,
      signs = {
        add = '+',
        modify = '~',
        rename = 'r',
        untracked = '?',
      },
      colors = {
        add = { fg = '#98c379' },
        modify = { fg = '#e5c07b' },
        rename = { fg = '#61afef' },
        untracked = { fg = '#e06c75' },
      },
    },

I chose to specify the colors in a different table than the signs' text because I didn't want to break already existing configs.

I also converted the sign definition back to a loop, the new definition looks like this:

  local signs_config = vim.g.triptych_config.git_signs

  local signs_to_text = {
    ['TriptychGitAdd'] = {
      text = signs_config.signs.add,
      texthl = 'TriptychSignGitAdd',
      hl = signs_config.colors.add,
    },
    ['TriptychGitModify'] = {
      text = signs_config.signs.modify,
      texthl = 'TriptychSignGitModify',
      hl = signs_config.colors.modify,
    },
    ['TriptychGitRename'] = {
      text = signs_config.signs.rename,
      texthl = 'TriptychSignGitRename',
      hl = signs_config.colors.rename,
    },
    ['TriptychGitUntracked'] = {
      text = signs_config.signs.untracked,
      texthl = 'TriptychSignGitUntracked',
      hl = signs_config.colors.untracked,
    },
  }

  -- Register the signs if they're not already
  for sign_name, opts in pairs(signs_to_text) do
    if u.is_empty(vim.fn.sign_getdefined(sign_name)) then
      vim.fn.sign_define(sign_name, { text = opts.text, texthl = opts.texthl })
      vim.api.nvim_set_hl(0, opts.texthl, opts.hl)
    end
  end

I also added the types for the color table and the highlights to types.lua.

@simonmclean
Copy link
Owner

simonmclean commented Feb 7, 2024

I think it would be preferable to update the existing config options so that they're of type string | table where the table is the dictionary described here.

So if type(value) == 'string' then nothing changes code wise

 vim.fn.sign_define(sign_name, { text = value })

And if type(value) == 'table' then it gets passed along to sign_define unchanged, and we don't have to care about the contents

 vim.fn.sign_define(sign_name, value)

The problem with treating texthl as a special property is that someone can come along later and request support for icon or culhl, and I'd need to add yet more config and logic to handle it. This way it's feature complete and won't require changes in future (unless a major nvim version makes breaking changes)

@theKnightsOfRohan
Copy link
Contributor Author

theKnightsOfRohan commented Feb 7, 2024

That makes sense, I can shift it around. But how will the color itself be represented in the config? Because the sign_define method does not allow you to pass a color in as an argument, so will that still be part of the separate "colors" table? If that's the case, then the color options will already match the tabular options that you want.

@simonmclean
Copy link
Owner

simonmclean commented Feb 8, 2024

The idea is that the user would specify the highlight group, rather than a colour directly.

I think this is preferable for a few reasons

  1. Highlight groups are more powerful, as they allow for foreground and background colours, as well as a large array of other styling options (bold, italic etc)
  2. When using an existing highlight group, they promote consistency with the rest of the colorscheme, and potentially removes the need to update your Triptych config when switching between colorschemes
  3. If there isn't an existing highlight group you want to use, you can always create your own and have complete control, e.g. vim.api.nvim_create_hl(0, 'MyGitAdd', { fg = "#98c379" })
  4. It's consistent with how colours are managed elsewhere in the Triptych config. Specifically in options.highlights

I don't mind implementing this myself if it's unclear?

@theKnightsOfRohan
Copy link
Contributor Author

theKnightsOfRohan commented Feb 8, 2024

Oh, I see what you mean. I can try to implement it myself, and get a PR by tonight.

simonmclean pushed a commit that referenced this issue Feb 9, 2024
* feat: Add ability to specify sign colors (#31)

* fix: replace colors config with full control over sign_define opts

* Update README.md

* fix: add formatting command to validate.sh

* fix: update signs_config import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants