michaelb / sniprun

A neovim plugin to run lines/blocs of code (independently of the rest of the file), supporting multiples languages
MIT License
1.43k stars 46 forks source link

Improvement: Update Lua code to use Neovim API functions #278

Closed pwnalone closed 5 months ago

pwnalone commented 5 months ago

Is your feature request related to a problem? Please describe.

I was trying to link the Sniprun* highlight groups to some builtin Neovim ones with the following configuration options, but it was not working.

snipruncolors = {                                                  
  SniprunVirtualTextOk  = { link = "DiagnosticVirtualTextInfo" },  
  SniprunVirtualTextErr = { link = "DiagnosticVirtualTextError" }, 
  SniprunFloatingWinOk  = { link = "NotifyINFOTitle" },            
  SniprunFloatingWinErr = { link = "NotifyERRORTitle" },           
},                                                                 

I decided to take a look at where the snipruncolors option is used in the source code and discovered that the issue stems from the fact that Sniprun manually constructs VimScript highlight commands and then runs them with vim.cmd(); however, the code only expects certain arguments such as bg, fg, sp, etc., but not the link argument (see [:help nvim_set_hl()](https://neovim.io/doc/user/api.html#nvim_set_hl())).

local highlight = function(group, styles)
  local gui = styles.gui and 'gui='..styles.gui or 'gui=NONE'
  local sp = styles.sp and 'guisp='..styles.sp or 'guisp=NONE'
  local fg = styles.fg and 'guifg='..styles.fg or 'guifg=NONE'
  local bg = styles.bg and 'guibg='..styles.bg or 'guibg=NONE'
  local ctermbg = styles.ctermbg and 'ctermbg='..styles.ctermbg or 'ctermbg=NONE'
  local ctermfg = styles.ctermfg and 'ctermfg='..styles.ctermfg or 'ctermfg=NONE'
  -- This somehow works for default highlighting. with or even without cterm colors
  -- hacky way tho.Still I think better than !hlexists
  vim.cmd('highlight '..group..' '..gui..' '..sp..' '..fg..' '..bg..' '..ctermbg..' '..ctermfg)
  vim.api.nvim_command('autocmd ColorScheme * highlight '..group..' '..gui..' '..sp..' '..fg..' '..bg..' '..ctermbg..' '..ctermfg)
end

NOTE: The above code snippet can be found here.

It then occurred to me that there are several places in the Lua portions of the source code where Sniprun essentially just executes VimScript wrapped in calls to vim.cmd(). While this works, it is less elegant and prevents Sniprun from being able to transparently benefit from new Neovim features. For example, what happens if Neovim core adds another highlight option? Does Sniprun update the above highlight function to include it? Does it choose not to support it?

Describe the solution you'd like

My suggestion is to update the Lua code to use Neovim's API functions wherever possible instead of calling out to embedded VimScript. For example, the above highlight function could be replaced with the something like this.

vim.api.nvim_set_hl(0, group, styles)

Additional context

I'd be happy to take on this task, myself, but I wanted to create this issue first in case there is a specific reason why Sniprun is currently using embedded VimScript. For example, is it to maintain compatibility with older version of Neovim that do not have certain API functions available? Or is it something else?

michaelb commented 5 months ago

specific reason why Sniprun is currently using embedded VimScript.

Mostly because the use of some features predates the Lua migration (sniprun existed before neovim 0.5!)

For many parts (such as highlights) I did not consider worth it at the time to translate these part of the code, because I'm lazy (aren't we all?). If you propose changes that are easy enough for me to understand, make sense / improve in a way the related feature, and doesn't break existing user configurations, I'll be more than happy to include your code (and your name) into the project :-)

Some other parts, however (ex: operator mode, or snipinfo tab completion) there was no way of achieving what I wanted in Lua, or there were some minor quirks. Maybe things have improved since then, but just in case, refrain from rewriting everything in Lua.

pwnalone commented 5 months ago

Okay, I'll fork the repo then and work on some changes. I'll reference this issue in any related PRs that I make.

pwnalone commented 5 months ago

@michaelb How would you feel about me adding a .stylua.toml file, formatting the Lua code, and adding a note in CONTRIBUTING.md to request that future contributors use Stylua to format their code before opening a PR? And if you're all for it, do you have a preference with respect to the Stylua formatting options? Here are the defaults.

column_width = 120
line_endings = "Unix"
indent_type = "Tabs"
indent_width = 4
quote_style = "AutoPreferDouble"
call_parentheses = "Always"
collapse_simple_statement = "Never"

[sort_requires]
enabled = false

I personally prefer spaces and an indent width of 2, but it's up to you.

michaelb commented 5 months ago

I feel like it's overkill. The code is currently no too ugly (according to my - and your - preferences). You're also overestimating the number of future contributions, which I my experience, seldom reaches 1/year (for the Lua code).

Keep it simple ;-)