refactor: tidy actions submodules and improve API readability
#2593
Conversation
There was a problem hiding this comment.
The move of actions/tree-modifiers to actions/tree/modifiers is just a proposal, I think it makes sense to put it there but I can revert this change if it's better not to do that.
I would also move actions/reloaders/reloaders.lua to actions/reloaders/init.lua or even actions/reloaders.lua, as long as it's not a problem...
lua/nvim-tree/api.lua
Outdated
| local marks_navigation = require "nvim-tree.marks.navigation" | ||
| local marks_bulk_delete = require "nvim-tree.marks.bulk-delete" | ||
| local marks_bulk_trash = require "nvim-tree.marks.bulk-trash" | ||
| local marks_bulk_move = require "nvim-tree.marks.bulk-move" |
There was a problem hiding this comment.
I tried to add the various init.luas in the marks module but dependency loops start to come up, so this is best solution I could find.
There was a problem hiding this comment.
Marks are not quite like the other actions. They need a refactor / tidy but not today.
That's great - it is a hierarchy which makes sense. |
lua/nvim-tree/api.lua
Outdated
|
|
||
| local events = require "nvim-tree.events" | ||
|
|
||
| Api.events.subscribe = wrap(events.subscribe) |
There was a problem hiding this comment.
We can't wrap this one - we alllow users to subscribe before setup is called.
lua/nvim-tree/api.lua
Outdated
|
|
||
| Api.config.mappings.get_keymap = wrap(keymap.get_keymap) | ||
| Api.config.mappings.get_keymap_default = wrap(keymap.get_keymap_default) | ||
| Api.config.mappings.default_on_attach = wrap(keymap.default_on_attach) |
There was a problem hiding this comment.
No wrap for this one either; we let the users call this for before setup, even though it's an odd use case.
lua/nvim-tree/api.lua
Outdated
| Api.marks.navigate.prev = wrap(marks_navigation.prev) | ||
| Api.marks.navigate.select = wrap(marks_navigation.select) | ||
|
|
||
| local keymap = require "nvim-tree.keymap" |
There was a problem hiding this comment.
Could we move these requires to the top, or is order important here?
There was a problem hiding this comment.
I put the definitions just before the references to easily trace back which module each variable refers to, of course we can move them to the top.
lua/nvim-tree/api.lua
Outdated
| local marks_navigation = require "nvim-tree.marks.navigation" | ||
| local marks_bulk_delete = require "nvim-tree.marks.bulk-delete" | ||
| local marks_bulk_trash = require "nvim-tree.marks.bulk-trash" | ||
| local marks_bulk_move = require "nvim-tree.marks.bulk-move" |
There was a problem hiding this comment.
Marks are not quite like the other actions. They need a refactor / tidy but not today.
| @@ -5,7 +5,7 @@ local core = require "nvim-tree.core" | |||
| local events = require "nvim-tree.events" | |||
| local notify = require "nvim-tree.notify" | |||
| local renderer = require "nvim-tree.renderer" | |||
| local reloaders = require "nvim-tree.actions.reloaders.reloaders" | |||
| local reloaders = require "nvim-tree.actions.reloaders" | |||
| M.fs.setup(opts) | ||
| M.node.setup(opts) | ||
| M.root.setup(opts) | ||
| M.tree.setup(opts) |
There was a problem hiding this comment.
This @PostConstruct style is an unfortunate necessity for lua's "dependency injection".
I'll be mindful to add more of this as I touch files with odd instantiation mechanisms.
Discussed in #2583