feat: load command definitions at nvim startup

[przepompownia]

This change ensures that commands are defined even if most modules are loaded as late as possible.

[przepompownia]

It depends on #3210.

Initially I wanted to move most of lua/nvim-tree/commands.lua logic into plugin, but the get() method is used in api.

I'm not sure if the commit should be labelled as feat - it only changes the way of defining all the commands.

I didn't check yet if it's "setup-safe" - at the first glance it looks like wrap from api should warn when using nvim-tree without setup.

[alex-courtis]

When we finally get around to tidying the API a similar approach can be used.

[alex-courtis]

This all looks great, it works.

Looking at some other plugins, they all have one shot initialisation guards, likely to handle re-sourcing your config.

This one uses a lua local: https://github.com/MeanderingProgrammer/render-markdown.nvim/blob/main/plugin/render-markdown.lua

A vim.g: https://github.com/neovim/nvim-lspconfig/blob/master/plugin/lspconfig.lua

I'm inclined to go with the the vim.g to match the style of lspconfig which is a very official plugin.

[alex-courtis]

Thinking about other things that can go in here, not necessarily today:

• the vim.fn.has("nvim-0.9") == 0 version check
• API initialisation
• some sort of early logging support
• highlight groups
• default on attach

Don't mind me, I'm just brainstorming.

[alex-courtis]

Don't forget that you're an nvim-tree team member with write permissions - you are able to push branches.

[przepompownia]

I'm still confused about the scope of privileges but, indeed, pushing non-master branch here turns out to work for me, so I can create PR's from this original repo instead of my fork.

The markdown plugin example does not defer loading a few modules (which may contain further requires - I didn't check).

The (still not moved to Nvim) lspconfig command definitions are loaded lazily as expected (and what it might look like here if there were no dependence on api).

[alex-courtis]

The markdown plugin example does not defer loading a few modules (which may contain further requires - I didn't check).

Yeah, that one's not cool. It loads the world.

The (still not moved to Nvim) lspconfig command definitions are loaded lazily as expected (and what it might look like here if there were no dependence on api).

They get away with it as vim's APIs are just _meta shells. Doesn't work for us or other plugins.

[alex-courtis]

What do you think about:

if vim.g.NvimTreeLoaded ~= nil then
  return
end
vim.g.NvimTreeLoaded = 1

to match vim.g.NvimTreeSetup

Alternatively, vim.g.nvim_tree to match lspconfig.

[alex-courtis]

I'm not sure if the commit should be labelled as feat - it only changes the way of defining all the commands.

It's a feature. Maybe "load command definitions at vim startup"

[przepompownia]

What do you think about:

if vim.g.NvimTreeLoaded ~= nil then
  return
end
vim.g.NvimTreeLoaded = 1

to match vim.g.NvimTreeSetup

Alternatively, vim.g.nvim_tree to match lspconfig.

FYI. I kept this guard in mind and looked into plugins used by me before creating this PR. Only part of them uses it. I would need an example to be convinced that we really need this.

[alex-courtis]

FYI. I kept this guard in mind and looked into plugins used by me before creating this PR. Only part of them uses it. I would need an example to be convinced that we really need this.

Yeah, fair enough. commands.setup is idempotent due to the force = true.

We'll have to be mindful of this in the future.

Do you have any other work you'd like to do around startup and loading?

[przepompownia]

Do you have any other work you'd like to do around startup and loading?

In practice I have the user commands available without workarounds and with lazy loading. I don't want to interfere in the decision about whether to keep commands.get() available by API (maybe some users use it) or not (in that case we would move its logic to plugin).

Thank you for merging!

[alex-courtis]

In practice I have the user commands available without workarounds and with lazy loading. I don't want to interfere in the decision about whether to keep commands.get() available by API (maybe some users use it) or not (in that case we would move its logic to plugin).

Please always question decisions, your input is valuable.

The legendary plugin folks were pretty specific about getting keymaps #1858 and less specific about commands #2076

They are used however I'm open to anything that doesn't break existing API.

[przepompownia]

The only side note: Nvim commands cannot be namespaced (like augroups). It seems that on the Nvim side we can only filter the result of vim.api.nvim_get_commands({}) by matching the name. I noticed script_id as the common distinguishing field, but its value is not exposed when created and is not documented anywhere.

[alex-courtis]

script_id is odd. It seems that the vimscript plugins have their own id, with the lua ones having a value 5 for me.
treesitter is an exception.

local commands = vim.api.nvim_get_commands({});
for k, c in vim.spairs(commands) do
  print(k .. "\t" .. c.script_id)
end

Hopefully they build this out; namespace or some other plugin id would be valuable.

AerialClose	5
AerialCloseAll	5
AerialGo	5
AerialInfo	5
AerialNavClose	5
AerialNavOpen	5
AerialNavToggle	5
AerialNext	5
AerialOpen	5
AerialOpenAll	5
AerialPrev	5
AerialToggle	5
BA	38
BB	38
BD	38
BF	38
BUN	38
BUNDO	38
BW	38
CD	5
DoMatchParen	19
EditQuery	-8
FZF	32
G	40
GBrowse	40
GDelete	40
GMove	40
GRemove	40
GRename	40
GUnlink	40
Gbrowse	40
GcLog	40
Gcd	40
Gclog	40
Gdelete	40
Gdiffsplit	40
Gdrop	40
Ge	40
Gedit	40
Ggrep	40
Ghdiffsplit	40
Git	40
Gitsigns	5
GlLog	40
Glcd	40
Glgrep	40
Gllog	40
Gmove	40
Gpedit	40
Gr	40
Gread	40
Gremove	40
Grename	40
Gsplit	40
Gtabedit	40
Gvdiffsplit	40
Gvsplit	40
Gw	40
Gwq	40
Gwrite	40
Hexmode	34
Inspect	-8
InspectTree	-8
LspInfo	42
LspLog	42
LspRestart	42
LspStart	42
LspStop	42
Man	29
MatchDebug	18
MatchDisable	18
MatchEnable	18
NoMatchParen	19
NvimTreeClipboard	5
NvimTreeClose	5
NvimTreeCollapse	5
NvimTreeCollapseKeepBuffers	5
NvimTreeFindFile	5
NvimTreeFindFileToggle	5
NvimTreeFocus	5
NvimTreeHiTest	5
NvimTreeOpen	5
NvimTreeRefresh	5
NvimTreeResize	5
NvimTreeToggle	5
NvimWebDeviconsHiTest	5
Open	-8
PS	5
PackerClean	5
PackerCompile	5
PackerInstall	5
PackerLoad	5
PackerProfile	5
PackerSnapshot	5
PackerSnapshotDelete	5
PackerSnapshotRollback	5
PackerStatus	5
PackerSync	5
PackerUpdate	5
PlenaryBustedDirectory	36
PlenaryBustedFile	36
RenderMarkdown	46
S	5
Spectre	43
TOhtml	31
TSBase	5
TSBufDisable	44
TSBufEnable	44
TSBufToggle	44
TSConfigInfo	44
TSDisable	44
TSEditQuery	44
TSEditQueryUserAfter	44
TSEnable	44
TSInstall	44
TSInstallFromGrammar	44
TSInstallInfo	44
TSInstallSync	44
TSModuleInfo	44
TSToggle	44
TSUninstall	44
TSUpdate	44
TSUpdateSync	44
Telescope	47
Tutor	26
UpdateRemotePlugins	21
WhichKey	5

[przepompownia]

Regardless of that I don't suggest relying on that, you shown that (unlike I said) in your example 5 is assigned not only to NvimTree*.