davidgranstrom / scnvim

Neovim frontend for SuperCollider.
GNU General Public License v3.0
208 stars 28 forks source link

Lua rewrite #153

Closed davidgranstrom closed 2 years ago

davidgranstrom commented 3 years ago

This PR is now a rewrite of the scnvim project. It contains some breaking changes, notably the viml API is gone and is now implemented in lua.

New features

Developer features

Breaking changes

See lua/scnvim/config.lua for default configuration values.

View the auto generated API documentation here

Example configuration:

local scnvim = require 'scnvim'
local map = scnvim.map
local map_expr = scnvim.map_expr

scnvim.setup {
  keymaps = {
    ['<M-e>'] = map('editor.send_line', {'i', 'n'}),
    ['<C-e>'] = {
      map('editor.send_block', {'i', 'n'}),
      map('editor.send_selection', 'x'),
    },
    ['<CR>'] = map('postwin.toggle'),
    ['<M-CR>'] = map('postwin.toggle', 'i'),
    ['<M-L>'] = map('postwin.clear', {'n', 'i'}),
    ['<C-k>'] = map('signature.show', {'n', 'i'}),
    ['<F12>'] = map('sclang.hard_stop', {'n', 'x', 'i'}),
    ['<leader>st'] = map('sclang.start'),
    ['<leader>sk'] = map('sclang.recompile'),
    ['<F1>'] = map_expr('s.boot'),
    ['<F2>'] = map_expr('s.meter'),
  },
  editor = {
    highlight = {
      color = 'IncSearch',
      type = 'flash',
    },
  },
  documentation = {
    cmd = '/opt/homebrew/bin/pandoc',
  },
  postwin = {
    keymaps = {
      q = map('postwin.close')
    },
    float = {
      enabled = true,
    },
  },
}

Close #136 Close #25 Close #132 Close #135 Close #136 Close #134 (the generated assets aspect of the feature request) Close #147 Close #122 Close #156 Close #183

davidgranstrom commented 2 years ago

Did an experiment today with the new mapping system. It lets you hook a callback function to be executed before the text is actually sent to SuperCollider. Could be used for text substitution (kind of like a preprocessor), but also to call any other arbitrary function on a specific scnvim key action/mapping.

By default the mapping table is empty, but I will add a example config with all the current default mappings for easy transition to the new lua config.

local scnvim = require'scnvim'
scnvim.setup {
  mapping = {
    ['<M-e>'] = scnvim.map.send_line({'i', 'n'}, function(data)
      local line = data[1]
      line = line:gsub('goodbye', 'hello')
      return {line}
    end),
    ['<C-e>'] = {
      scnvim.map.send_block({'i', 'n'}),
      scnvim.map.send_selection('x'),
    },
    ['<F12>'] = scnvim.map.hard_stop({'n', 'x', 'i'}),
    ['<CR>'] = scnvim.map.postwin_toggle('n'),
    ['<M-CR>'] = scnvim.map.postwin_toggle('i'),
    ['<M-L>'] = scnvim.map.postwin_clear({'n', 'i'}),
    ['<C-k>'] = scnvim.map.show_signature({'n', 'i'}),
  },
}

~edit: Maybe flash should be the last argument..~ it is now!

madskjeldgaard commented 2 years ago

I just tried this and it works great. I really love the floating post window!!! Will it be possible to set the position of the post window in the config as well? That would be cool!

Also seems that FzfSC works without any changes with this. Haven't tested the other plugins that depend on this yet but it's probably fine. Great work!

davidgranstrom commented 2 years ago

@madskjeldgaard Glad to hear! :)

Yes, you can use offset_x and offset_y to position the float in the config, here's all options that you can use: https://github.com/davidgranstrom/scnvim/blob/topic/lua-config/lua/scnvim/config.lua#L37-L50

davidgranstrom commented 2 years ago

..thinking about this now, I think I might add row and col options (as functions) in order to specify the absolute position from the config.

madskjeldgaard commented 2 years ago

I'm wondering something: How does one map something like a custom piece of sc code using the new mapping paradigm? Eg. Server.local.boot ?

Also: Maybe it's worth having recompile as a mappable function in scnvim/editor.lua ? (And maybe also Server.local.boot ? And potentially SCNvimStart)

madskjeldgaard commented 2 years ago

As an example of what I imagine a mappable piece of sc code would like like in the new paradigm:

    ['<F1>'] = scnvim.map.execute_sclang("Server.local.boot", {'n', 'x', 'i'})
davidgranstrom commented 2 years ago

Hi @madskjeldgaard,

It is possible to map any arbitrary function using the map helper, here are some examples:

    ['<leader>st'] = map(scnvim.start),
    ['<leader>sk'] = map(scnvim.recompile),
    ['<leader>sh'] = map(function()
      scnvim.send('"hello!".postln')
    end)

Your example could be written as:

    ['<F1>'] = scnvim.map(function()
      scnvim.send("Server.local.boot")
    end, {'n', 'x', 'i'}),

If you plan to do a lot of these, perhaps this wrapper function could serve as a starting point to make the mappings a bit more clear (maybe I'll add this to the docs too):

local map_expr = function(expr, modes)
  modes = modes or {'n', 'x', 'i'}
  return scnvim.map(function()
    scnvim.send(expr)
  end, modes)
end

-- usage
scnvim.setup {
  mapping = {
    ['<F1>'] = map_expr("Server.local.boot"),
    ['<F2>'] = map_expr("s.meter"),
    }
}
madskjeldgaard commented 2 years ago
, {'n', 'x', 'i'}

fantastic!

madskjeldgaard commented 2 years ago

As far as I can tell the docs don't contain info on changing post window size/position (yet), just FYI

madskjeldgaard commented 2 years ago

Also, this is just a small thing possibly for later - but how could one have these new mappings be used in other filetypes other than SuperCollider? I'm thinking for example of Faust files where having access to post window etc. would be cool.

davidgranstrom commented 2 years ago

Yes, (user) documentation is still lacking in some aspects. There is an overview in :h scnvim, but it's not comprehensive.

I have not decided exactly on how to proceed to create a more detailed user documentation.. the API documentation is auto generated (from the lua comments) so it's always is up-to-date:

https://davidgranstrom.github.io/scnvim/modules/scnvim.config.html#default.postwin

(There is also a page for the config module.)

The default values are set here:

https://github.com/davidgranstrom/scnvim/blob/topic/lua-config/lua/scnvim/config.lua

davidgranstrom commented 2 years ago

Also, this is just a small thing possibly for later - but how could one have these new mappings be used in other filetypes other than SuperCollider? I'm thinking for example of Faust files where having access to post window etc. would be cool.

The mappings are applied for the supercollider filetype only (via an autocmd). But we could expose the local apply_keymaps function in the editor module and then it would be possible to apply keymaps for other filetypes as well, do you think that would be sufficient?

If it's only for a specific function (like toggle the post window), then using lua.keymap.set and calling the scnvim API could also be an alternative:

vim.keymap.set('n', '<leader>st', '<cmd>lua require"scnvim.postwin".toggle()<cr>')

(It is possible to not specify any mappings at all in the config and use any nvim mapping method instead)

madskjeldgaard commented 2 years ago

https://davidgranstrom.github.io/scnvim/modules/scnvim.config.html#default.postwin

this is super nice!!!

madskjeldgaard commented 2 years ago

More random things: postwin width and height must be integers otherwise it results in errors. if Using eg. vim.fn.winwidth(0) / 2 in a function one might easily get a float. It's easily fixed with math.floor(), but maybe that should be done in the function that receives this value?

madskjeldgaard commented 2 years ago

A possible bug: It seems that when choosing Luasnip as snippet engine, the produced snippet is not properly escaped with it's \ characters. Here is an example from the snippets produced on my system:

s( {trig = "FoscStartTextSpan.new", name = "FoscStartTextSpan.new", dscr = "Snippet for FoscStartTextSpan.new, auto generated by SCNvim" }, {t("FoscStartTextSpan.new"),t("("),t("leftText:"),i(1, "nil"),t(", "), t("rightText:"),i(2, "nil"),t(", "), t("style:"),i(3, "nil"),t(", "), t("direction:"),i(4, "nil"),t(", "), t("command:"),i(5, "\startTextSpan"),t(", "), t("concatHspaceLeft:"),i(6, "0.5"),t(", "), t("concatHspaceRight:"),i(7, "0.5"),t(", "), t("leftBrokenText:"),i(8, "nil"),t(", "), t("rightPadding:"),i(9, "nil"),t(", "), t("tweaks:"),i(10, "nil"), t(")"),}),
madskjeldgaard commented 2 years ago

Also it might be worthwile to include the local scnvim = require("scnvim") line in the help file example for setting up scnvim using the new paradigm. This implicit require might confuse new users a bit :)

madskjeldgaard commented 2 years ago

Another thing missing from the docs: The help file should include an example mapping for looking up help file for word under cursor, as I think that one might be a bit complex for some people.

davidgranstrom commented 2 years ago

@madskjeldgaard Great notes about the docs, I'll add your suggestions.

davidgranstrom commented 2 years ago

I think this is ready to be merged now.

madskjeldgaard commented 2 years ago

I think this is ready to be merged now.

I agree. Do it !

madskjeldgaard commented 2 years ago

A possible bug: It seems that when choosing Luasnip as snippet engine, the produced snippet is not properly escaped with it's \ characters. Here is an example from the snippets produced on my system:

s( {trig = "FoscStartTextSpan.new", name = "FoscStartTextSpan.new", dscr = "Snippet for FoscStartTextSpan.new, auto generated by SCNvim" }, {t("FoscStartTextSpan.new"),t("("),t("leftText:"),i(1, "nil"),t(", "), t("rightText:"),i(2, "nil"),t(", "), t("style:"),i(3, "nil"),t(", "), t("direction:"),i(4, "nil"),t(", "), t("command:"),i(5, "\startTextSpan"),t(", "), t("concatHspaceLeft:"),i(6, "0.5"),t(", "), t("concatHspaceRight:"),i(7, "0.5"),t(", "), t("leftBrokenText:"),i(8, "nil"),t(", "), t("rightPadding:"),i(9, "nil"),t(", "), t("tweaks:"),i(10, "nil"), t(")"),}),

This is the only issue at the moment, but I'm not sure if it's related to this. Maybe more relevant in a seperate PR. What do you think David?

davidgranstrom commented 2 years ago

A possible bug: It seems that when choosing Luasnip as snippet engine, the produced snippet is not properly escaped with it's \ characters. Here is an example from the snippets produced on my system:

s( {trig = "FoscStartTextSpan.new", name = "FoscStartTextSpan.new", dscr = "Snippet for FoscStartTextSpan.new, auto generated by SCNvim" }, {t("FoscStartTextSpan.new"),t("("),t("leftText:"),i(1, "nil"),t(", "), t("rightText:"),i(2, "nil"),t(", "), t("style:"),i(3, "nil"),t(", "), t("direction:"),i(4, "nil"),t(", "), t("command:"),i(5, "\startTextSpan"),t(", "), t("concatHspaceLeft:"),i(6, "0.5"),t(", "), t("concatHspaceRight:"),i(7, "0.5"),t(", "), t("leftBrokenText:"),i(8, "nil"),t(", "), t("rightPadding:"),i(9, "nil"),t(", "), t("tweaks:"),i(10, "nil"), t(")"),}),

This is the only issue at the moment, but I'm not sure if it's related to this. Maybe more relevant in a seperate PR. What do you think David?

I'm not so familiar with the luasnip snippet generator code, but I could have a quick look later today. It's probably an escaping issue for the \ character. We could make a follow up PR for that separately otherwise so not to delay merging this any further. Will do some minor fixes for the README tonight and then we should be good to go!

madskjeldgaard commented 2 years ago

One last idea: Add an option to have the key bindings be mapped to the post window as well as the help browser window. I personally use this and perhaps others might find this useful?

davidgranstrom commented 2 years ago

One last idea: Add an option to have the key bindings be mapped to the post window as well as the help browser window. I personally use this and perhaps others might find this useful?

Ah, forgot about the help window, definitely useful to be able to run code examples :) I think I'll add an option in the postwin and documentation tables to easily toggle this on/off.

davidgranstrom commented 2 years ago

One last idea: Add an option to have the key bindings be mapped to the post window as well as the help browser window. I personally use this and perhaps others might find this useful?

Good catch again. This should now work with the latest push, both postwin and documentation. documentation has the same mappings by default, postwin has none.

  postwin = {
    mapping = { -- or set the true to use the same values as top-level `mapping`
      q = map(require('scnvim.postwin').close)
    },
}

I'm having some second thoughts about the mapping system.. I'm not so happy with the code duplication in editor needed for the current mapping approach. I have a solution I would like to try before the merge, better now than later :)

davidgranstrom commented 2 years ago

Here's the idea for the new map helper:

  mappings = {
    ['<M-e>'] = map('editor.send_line', {'i', 'n'}),
    ['<leader>st'] = map(scnvim.start),
    ['<F1>'] = map_expr('s.boot'),
    ['<F2>'] = map_expr('s.meter'),
  },

The map helper object can take a string or a function. If the argument is a string, it will parse the module name and the function name (e.g. editor.send_line) and create a mapping based on that. There is some validation to what modules can be used, mainly for debugging purposes (spelling mistakes etc.)

The map_expr helper is an addition to make it easy to map any valid SuperCollider expression to a key.

davidgranstrom commented 2 years ago

Breaking changes for keymaps: https://github.com/davidgranstrom/scnvim/issues/183#issuecomment-1170939934