nix-community / kickstart-nix.nvim

❄️ A dead simple Nix flake template repository for Neovim derivations [maintainer=@mrcjkb]
GNU General Public License v2.0
231 stars 10 forks source link

Some feedback #4

Closed Doosty closed 1 year ago

Doosty commented 1 year ago

Hi again, ive been tinkering with nvim and this repo for the past couple days and i have some feedback if you want to hear it. First i want to thank you for creating this derivation, i believe this is the best way of managing nvim (nix for packages, lua for config). However ive had some troubles with the configuration that is included alongside the derivation.

Issues
  1. This snippet creates an annoyance - horizontal cursor position on a line is not remembered properly anymore so j/k navigation now puts you at the start of the line most of the time. I decided to comment it out

    -- Automatic management of search highlight
    local auto_hlsearch_namespace = vim.api.nvim_create_namespace('auto_hlsearch')
    vim.on_key(function(char)
    if vim.fn.mode() == 'n' then
    vim.opt.hlsearch = vim.tbl_contains({ '<CR>', 'n', 'N', '*', '#', '?', '/' }, vim.fn.keytrans(char))
    end
    end, auto_hlsearch_namespace)
  2. This keymap has error, some params are missing. After i provided the width, height, bufpos, relative... the popup still did not work as expected so i decided to comment it out.

    keymap.set('n', '<space>e', function()
    local _, winid = diagnostic.open_float(nil, { scope = 'line' })
    vim.api.nvim_win_set_config(winid or 0, { focusable = true })
    end, { noremap = true, silent = true, desc = 'diagnostics floating window' })
  3. This autocommand creates an annoyance - flickering line numbers on each mouse click. I decided to comment it out.

    -- Toggle between relative/absolute line numbers
    -- Show relative line numbers in the current buffer,
    -- absolute line numbers in inactive buffers
    local numbertoggle = api.nvim_create_augroup('numbertoggle', { clear = true })
    api.nvim_create_autocmd({ 'BufEnter', 'FocusGained', 'InsertLeave', 'CmdlineLeave', 'WinEnter' }, {
    pattern = '*',
    group = numbertoggle,
    callback = function()
    if vim.o.nu and vim.api.nvim_get_mode().mode ~= 'i' then
      vim.opt.relativenumber = true
    end
    end,
    })
    api.nvim_create_autocmd({ 'BufLeave', 'FocusLost', 'InsertEnter', 'CmdlineEnter', 'WinLeave' }, {
    pattern = '*',
    group = numbertoggle,
    callback = function()
    if vim.o.nu then
      vim.opt.relativenumber = false
      vim.cmd.redraw()
    end
    end,
    })
  4. I believe i had to add a leader mapping.

    -- Leader key
    g.mapleader = ' '
    g.maplocalleader = ' '
  5. Plugin cmp-cmdline was missing from derivation but it is configured in lua.

  6. The plugin wf.nvim was giving me errors on its next popups if the previous one did not end gracefully - errored keymapping or otherwise. I decided to use which-key instead, i also prefer it being more minimal and taking up less screen space.

  7. In lualine.lua i believe extenstions is a typo, not sure where to check logs if the plugin loads correctly or not.

  8. Next are annoyances or problems of my own making but maybe worth mentioning since other users might encounter these issues

    • Lualine and Navic config was a nightmare when i decided to install Neotree. The top bar was dancing around and bottom elements were showing on the Neotree window thus creating clutter. I decided to remove the top bar, remove most bottom sections, remove Navic, add Barbar (maybe Lualine could be used for tabs, i dont know). Perhaps a more minimal included statusline config would be better, atleast without the top line.
    • I tried to use pylsp (https://github.com/python-lsp/python-lsp-server) with lspconfig (https://github.com/neovim/nvim-lspconfig) and :LspInfo shows everything green however the lsp is not started and not attached. A similar configuration to your lua-language-server and nil managed to start and attach the pylsp, but i am still curious why lspconfig did not work, since that is the plugin most popular for configuring the lsp, and wondering why you decided not to use it?
    • For the avarage user i dont think current_line_blame in gitsigns.lua should be on.
Thoughts

I believe this nix derivation is brilliant, however the configuration could be a bit less opinionated and more minimal, i think. Users would benefit from a minimal & barebones configuration, so their base is stable, error-free and modular. Stuff like 4 different git plugins, 2 statusbar plugins and opinionated snippets maybe belong in a secondary personal repo which you link to in your readme, or into a readme section where you demonstrate and recommend to install those plugins. In the end i know we are free to delete the entire config and make our own from scratch, however most users might just move on after encountering the first annoyance. Anyway, thanks for your work, i like it and will continue using it :+1:

mrcjkb commented 1 year ago

hey :wave:

Thanks for the feedback! I think you're probably right. I'll look into stripping it down some more.

To address some specific points:

'<space>e' keymap:

:thinking: The problem is that it shows an error if there are no diagnostics. I think I can fix that.

had to add a leader mapping

If you don't set a <leader> mapping, \ is the default <leader> (which I find quite comfortable on a US-intl query keyboard). I'd prefer to leave the default, but I can add that snippet commented out with a comment explaining it.

wf.nvim was giving me errors

Yup, I'll probably remove it. I'm going to disable the auto-popup in my personal config too, because it pops up way more often than I want it to.

Perhaps a more minimal included statusline config would be better, atleast without the top line.

Probably a good idea. I don't want to add any other tab/statusline plugins.

but i am still curious why lspconfig did not work, since that is the plugin most popular for configuring the lsp,

Did you call lspconfig.pylsp.setup in ftplugin/python? If so, that's probably why. lspconfig doesn't use the ftplugin mechanism. Instead, it creates filetype autocommands. If you call lspconfig.xyz.setup within an ftplugin file, it won't work, because the autocommand is created after you open the file.

and wondering why you decided not to use it?

Because I believe the setup antipattern must die :sweat_smile:. Aside from that, autocommands and calling require('<module>') in init.lua or plugin/foo.lua adds a bit of overhead to Neovim's startup. The ftplugin mechanism has zero startup impact and offers a neat way to organise configurations.

Anyways, thanks again for the great feedback!

Doosty commented 1 year ago

Did you call lspconfig.pylsp.setup in ftplugin/python? If so, that's probably why.

Seems like i might have, i tried lspconfig again but with the setup call inside init.lua and it worked. However ill keep using the ftplugin way since you say its better.

Also found a couple of unused vars (maybe you meant to include them, or i dont understand their use):

You can go ahead and close this issue when you read this, thanks.

mrcjkb commented 1 year ago

Hmm, those are probably leftovers from my personal config.

Thanks again 😃