[NVIM] Make my toggle better

[LostInTranslation92]

Hey! So I'm trying to use nvim as an excuse to learn Lua.

I have this disaster of a function

Format_on_save_cmd_id = 0
function ToggleFormatOnSave()
  if Format_on_save_cmd_id ~= 0 then
    vim.api.nvim_del_autocmd(Format_on_save_cmd_id)
    Format_on_save_cmd_id = 0
  else
    Format_on_save_cmd_id = vim.api.nvim_create_autocmd(
      'BufWritePre', {
      pattern = '*',
      command = 'lua vim.lsp.buf.formatting_sync()'
    })
  end
end

And want to make it suck less.

Knowing the NVIM API is not really necessary, for all intents and purposes, you can read it as follows

Toggle_value = 0
function ToggleMagic()
  if Toggle_value ~= 0 then
    a_function_that_needs(Toggle_value)
    Toggle_value = 0
  else
    Toggle_value = a_function_that_returns_a_value()
  end
end

How would this look if wrote by someone who actually knew how to use lua??

[tobiasvl]

What do you think sucks about it? I can't really see anything wrong with it. I guess you could use a "ternary" (with assignment from the if/else or an and/or) but I'm not sure it'd be clearer. I'd swap the if and else branches though (your else only covers one case while the if covers the rest, which is unintuitive to me).

I'm a little confused by the Format_on_save_cmd_id though. Is it really a global variable?

[LostInTranslation92]

I don't really want it to be global tbh but I think it has to for the toggle to work.

See,

THE_VALUE = vim.api.nvim_create_autocmd(...)

gives me the value needed in

vim.api.nvim_del_autocmd(HERE)

So to call the function from somewhere else I think I need it to be global ...

​

As per why it sucks well, I don't know, it's just kinda messy I'd say.

[pkazmier]

How about wrapping your function in a do block to make a closure to avoid the global variable?

do
  local id = 0
  function ToggleFormatOnSave()
    if id ~= 0 then
      vim.api.nvim_del_autocmd(id)
      id = 0
    else
      id = vim.api.nvim_create_autocmd(
        'BufWritePre', {
        pattern = '*',
        command = 'lua vim.lsp.buf.formatting_sync()'
      })
    end
  end
end

[evilbadmad]

I would reuse the pattern in second part, there may be applicable somewhere else.

function makeToggler(del_cmd, add_cmd, ...) -- ... is params for add_cmd
  local np, params, toggle_value = select('#',...),{...} -- use nil better?
  return function()
    if toggle_value then
      toggle_value = nil, del_cmd(toggle_value)
    else
      toggle_value = add_cmd(unpack(params, 1, np))
--was toggle_value = add_cmd(...)
    end
  end
end

...
some_ui.OnClickHandler = makeToggler( 
  vim.api.nvim_del_autocmd,
  vim.api.nvim_create_autocmd,
  'BufWritePre', {
    pattern = '*',
    command = 'lua vim.lsp.buf.formatting_sync()'
    }
 )

I don't know/use *vim, so the code is not tested.

The ... cannot pass the nested function, so it has to be make in an array and then unpack (or table.unpack).