neo-tree.nvim icon indicating copy to clipboard operation
neo-tree.nvim copied to clipboard

refactor(type): add type annotation for defaults.lua

Open pysan3 opened this issue 1 year ago • 6 comments

Hi ;)

I think I can start working on the big rewrite.

I am going to bring a huge diff to the codebase, so I'd like to ask your opinion first.

Add Type Annotations

  • .github/workflows/{.luarc.json,lua_ls-typecheck.yml}
  • lua/neo-tree/defaults.lua: https://github.com/pysan3/neo-tree.nvim/blob/add-type-annotations/lua/neo-tree/defaults.lua
    • diff: https://github.com/pysan3/neo-tree.nvim/commit/014f98a1a7741f83d6c327c8ebfcc85ed1fe8dab

I've refactored defaults.lua to have type annotations for every field in the table. With a recent change to GitHub's desktop UI, you should be able to jump to references to see the description.

lua_ls-typecheck.yml is configured to only check types on defaults.lua for now, and I will gradually make the list longer as I add the correct annotations.

Apply stylua

  • .github/workflows/stylua.yml

This is a github action to format the code automatically.

Changes other than defaults.lua are all made by this action (' -> " etc), and I'd like to add this action as well.

Roadmap

@cseickel I've written my roadmap about what I'm planning to do (no due date tho). May I hear your opinion about the list?

  • https://github.com/pysan3/neo-tree.nvim/issues/1

This PR is not meant to be merged, but just a POC. I'd like to fill in NeotreeTypes.node and NeotreeTypes.state before this gets merged.

Thanks in advance.

pysan3 avatar Feb 02 '24 17:02 pysan3

I think the type annotations are a great idea. One thing I would consider though is putting the config types in a separate file from the defaults.lua just because there is so much to it and we are using the defaults.lua file as an example and part of the documentation.

With the workflow, I normally prefer code formatting to be applied as a pre-commit hook, but I guess that's hard to enforce as an open source project with so many one time contributors. I'd say go ahead with that too.

As far the rest in your Roadmap, there are a lot of good ideas there but I think each one desrves it's own extended discussion. How about starting separate discussions for each one in https://github.com/nvim-neo-tree/neo-tree.nvim/discussions/categories/internal-development?

cseickel avatar Feb 03 '24 03:02 cseickel

Thanks for your feedback @cseickel !

One thing I would consider though is putting the config types in a separate file from the defaults.lua just because there is so much to it and we are using the defaults.lua file as an example and part of the documentation.

Roger, moved type definitions to lua/neo-tree/types/config.lua.

Along with that, I think examples for event_handlers should be moved to repo's wiki as it occupies quite a long chunk of code.

https://github.com/pysan3/neo-tree.nvim/blob/72776bcf371c77d482ce8331ecbba9906aaba05f/lua/neo-tree/defaults.lua#L519-L521

I don't know how to manage the wiki pages so could you create a page for me? As we have many more kinds of events, we could also add more examples there.

I'll gather people's code posted on issues / discussions later.

https://github.com/pysan3/neo-tree.nvim/blob/72776bcf371c77d482ce8331ecbba9906aaba05f/lua/neo-tree/defaults.lua#L596-L610

This code could also be moved to the wiki, or shall we just add it to the default config values?

pysan3 avatar Feb 03 '24 08:02 pysan3

Along with that, I think examples for event_handlers should be moved to repo's wiki as it > > > occupies quite a long chunk of code.

I don't know how to manage the wiki pages so could you create a page for me? As we have many more kinds of events, we could also add more examples there.

Sure.

https://github.com/pysan3/neo-tree.nvim/blob/72776bcf371c77d482ce8331ecbba9906aaba05f/lua/neo-tree/defaults.lua#L596-L610

This code could also be moved to the wiki, or shall we just add it to the default config values?

I'm not entirely clear on that one. I think it's for the document symbols source and I have never had to use those settings. I don't know if it belongs there, the docs, or the wiki.

cseickel avatar Feb 03 '24 13:02 cseickel

Most of the events were either already there or pointless example code. I did add the hide cursor example events though: https://github.com/nvim-neo-tree/neo-tree.nvim/wiki/Recipes#events

cseickel avatar Feb 03 '24 14:02 cseickel

Along with that, I think examples for event_handlers should be moved to repo's wiki as it > > > occupies quite a long chunk of code.

I don't know how to manage the wiki pages so could you create a page for me? As we have many more kinds of events, we could also add more examples there.

Sure.

Thanks <3

pysan3/neo-tree.nvim@72776bc/lua/neo-tree/defaults.lua#L596-L610 This code could also be moved to the wiki, or shall we just add it to the default config values?

I'm not entirely clear on that one. I think it's for the document symbols source and I have never had to use those settings. I don't know if it belongs there, the docs, or the wiki.

I think we can simply delete it.

pysan3 avatar Feb 03 '24 14:02 pysan3

pysan3/neo-tree.nvim@72776bc/lua/neo-tree/defaults.lua#L596-L610 This code could also be moved to the wiki, or shall we just add it to the default config values?

I'm not entirely clear on that one. I think it's for the document symbols source and I have never had to use those settings. I don't know if it belongs there, the docs, or the wiki.

I think we can simply delete it.

I think we should ask @nhat-vo about where the best location for this example is, he's the one that created the document symbols source. I'd say it is probably better either as an addition to the docs or the wiki.

cseickel avatar Feb 03 '24 15:02 cseickel