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

feat: add option for bold/italic formatting #3

Merged
merged 11 commits into from
Jan 28, 2023
Merged

feat: add option for bold/italic formatting #3

merged 11 commits into from
Jan 28, 2023

Conversation

itzaeon
Copy link
Contributor

@itzaeon itzaeon commented Jan 22, 2023

I added the option to format line numbers with bold or italics. I changed the name of your function M.set_highlight to M.set_highlight_and_format with a new parameter format.

Note: I haven't seen the syntax ---@param color string before, so you might have to update that for the new format parameter.

@mawkler
Copy link
Owner

mawkler commented Jan 25, 2023

@itzaeon Thank you very much for your PR! I have just a couple of minor things to comment on.

I think I might prefer if the configuration API looked like this:

{
  highlights = {
    modes = {
      ['i']  = {
        color = colors.insert,
        bold = false,
        italic = false,
      },
      ['v']  = {
        color = colors.visual,
        bold = false,
        italic = false,
      },
      ['V']  = {
        color = colors.visual,
        bold = false,
        italic = false,
      },
      [''] = {
        color = colors.visual,
        bold = false,
        italic = false,
      },
      ['s']  = {
        color = colors.select,
        bold = false,
        italic = false,
      },
      ['S']  = {
        color = colors.select,
        bold = false,
        italic = false,
      },
      ['R']  = {
        color = colors.replace,
        bold = false,
        italic = false,
      },
      ['c']  = {
        color = colors.command,
        bold = false,
        italic = false,
      },
    }
  }
}

since in Neovim, the "highlght" includes italic/bold. Or what do you think? Or do you think that would make it more confusing the user to include italic/bold under the highlights table?

lua/modicator/init.lua Outdated Show resolved Hide resolved
lua/modicator/init.lua Outdated Show resolved Hide resolved
@itzaeon
Copy link
Contributor Author

itzaeon commented Jan 26, 2023

A quick summary of my changes:

  • To options:
    • added the key overrides, which one can use to set every mode to bold/italic
    • refactored options.highlights.modes as shown in the above comment
  • To M.setup:
    • new variable has_overrides - if the user has given overrides
  • To M.set_format:
    • rewrote for the new options keys, removed the call to vim.tbl_extend as it wasn't necessary anymore

I also added the updated options keys to config, as well as how to install with lazy.nvim.

@mawkler
Copy link
Owner

mawkler commented Jan 26, 2023

@itzaeon Nice! However, now there's no highlight except for in normal mode for me. If I go to insert/visual/replace/etc. mode the line number indicator gets the foreground color of Nomal (white for me). Do you have the same issue?

@itzaeon
Copy link
Contributor Author

itzaeon commented Jan 26, 2023

I believe I know why you experience this behavior. The two colorschemes I use are zephyr and oxocarbon. This plugin works "as designed" when I use zephyr as my scheme, but not when I use oxocarbon.

Highlights with zephyr:

  • CursorLineNr (normal mode): guifg=#61afef
  • Question (insert mode): guifg=#f7bb3b
  • Type (visual mode): guifg=#1abc9c

And with oxocarbon:

  • CursorLineNr (normal mode): guifg=#d0d0d0
  • Question (insert mode): guifg=#d0d0d0
  • Type (visual mode): guifg=#78a9ff

Note that with oxocarbon, both CursorLineNr and Question have the same value. I see the same behavior that you do. With zephyr, since CursorLineNr/Question/Type are all different, the plugin works as intended.

I see two options to fix this. 1) We can define highlight groups specifically for modicator (ie ModicatorNormalHighlight, ModicatorInsertHighlight) or 2) a new function can be added that finds different highlights for each mode.

@mawkler
Copy link
Owner

mawkler commented Jan 26, 2023

@itzaeon I'm not sure that's the issue I'm seeing since I explicitly set the colors in my config. I get the same problem with the following test config on your branch:

use { 'melkster/modicator.nvim',
  config = function()
    require('modicator').setup({
      highlights = {
        modes = {
          ['i']  = 'red',
          ['v']  = 'green',
          ['V']  = 'blue',
          [''] = 'blue',
          ['s']  = 'yellow',
          ['S']  = 'brown',
          ['R']  = 'pink',
          ['c']  = 'orange',
        }
      },
    })
  end
}

@itzaeon
Copy link
Contributor Author

itzaeon commented Jan 26, 2023

@mawkler That config won't work because it was changed here. I get the same behavior using that config, but because the config options have changed in accordance with your comment, something like this should work:

use { 'melkster/modicator.nvim',
  config = function()
    require('modicator').setup({
      highlights = {
        modes = {
          ['n'] = { color = 'orange' },
          ['i']  = { color = 'red' },
          ['v']  = { color = 'green' },
          ['V']  = { color = 'blue' },
          [''] = { color = 'blue' },
          ['s']  = { color = 'yellow' },
          ['S']  = { color = 'brown' },
          ['R']  = { color = 'pink' },
          ['c']  = { color = 'orange' },
        }
      },
    })
  end
}

@mawkler
Copy link
Owner

mawkler commented Jan 27, 2023

@itzaeon Of course! You're right, that's my bad.

lua/modicator/init.lua Outdated Show resolved Hide resolved
lua/modicator/init.lua Outdated Show resolved Hide resolved
lua/modicator/init.lua Outdated Show resolved Hide resolved
@itzaeon
Copy link
Contributor Author

itzaeon commented Jan 27, 2023

Those should work.

Copy link
Owner

@mawkler mawkler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great! The only thing I want now is for highlights.overrides in the options table passed to setup() to be called highlights.defaults 🙂

README.md Outdated
-- to every mode.
overrides = {
bold = false,
bold = false
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't forget to change this one to italic as well :)

@itzaeon
Copy link
Contributor Author

itzaeon commented Jan 28, 2023

Done!

@mawkler
Copy link
Owner

mawkler commented Jan 28, 2023

Awesome! Thank you very much for your contributing! 😄

@mawkler mawkler merged commit 56fff13 into mawkler:main Jan 28, 2023
@mawkler
Copy link
Owner

mawkler commented Jan 28, 2023

@itzaeon Note that I just changed the name of the option color to foreground in 26338f7 to make it more coherent with Neovim's vim.api.nvim_set_hl(). I also added a background option.

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

Successfully merging this pull request may close these issues.

2 participants