Fix #565 : Move refactor modules to directory, edit lua files for path change

nvim-treesitter / nvim-treesitter-refactor

Refactor module for nvim-treesitter

Apache License 2.0

408 stars 25 forks source link

Fix #565 : Move refactor modules to directory, edit lua files for path change #3

Closed happy-dude closed 4 years ago

happy-dude commented 4 years ago

ref: nvim-treesitter/nvim-treesitter/issues/565

Mirror https://github.com/nvim-treesitter/nvim-treesitter-textobjects directory structure

Move lua modules under lua/nvim-treesitter/refactor (instead of lua/nvim-treesitter-refactor)
Edit lua files for new path

happy-dude commented 4 years ago

Happy #hacktoberfest 😄 ?

theHamsta commented 4 years ago

Won't this module change cause the breakage a again for the short moment when users install the update until they restart vim?

happy-dude commented 4 years ago

Won't this module change cause the breakage a again for the short moment when users install the update until they restart vim?

For me, the breakage only happens when I attempt to use the modules and/or enable them. nvim starts fine with the modules disabled. highlight_current_scope (when enabled) was causing a runtime path issue everytime characters were changed that is being reported at https://github.com/nvim-treesitter/nvim-treesitter/issues/565.

EDIT: re: breakage for users who are caught between an loaded outdated plugin and updated one on disk Yes, will likely error until neovim is restarted.

theHamsta commented 4 years ago

https://github.com/nvim-treesitter/nvim-treesitter-refactor/blob/130d9485a983814364ee2094e6073eb9e96f71d8/lua/nvim-treesitter-refactor/highlight_current_scope.lua#L33-L34

you only have to fix these two lines.

. -> -

happy-dude commented 4 years ago

Reason why I changed more than that was to make it consistent with how https://github.com/nvim-treesitter/nvim-treesitter-textobjects/tree/master/lua laid out the modules.

theHamsta commented 4 years ago

Let's let @steelsojka decide. I kept the file structure to not have to rename anything and be able to have both plugins installed in the transition period.

happy-dude commented 4 years ago

If the desire is to maintain the nvim-treesitter-refactor/lua/nvim-treesitter-refactor/*.lua directory structure, then it would also make sense to change https://github.com/nvim-treesitter/nvim-treesitter-textobjects/tree/master/lua from

lua/nvim-treesitter/textobjects/ -> lua/nvim-treesitter-textobjects/ ?

Suggesting that only as a consistency thing.

The disruption to users seems minor to me considering

treesitter is still under heavy development, so anyone following HEAD can and will experience breakage,
modules were moved out to separate repos as of the past 2 weeks,
number of users is small (judging by the amount of bug reports),
issue with the highlight_current_scope module in HEAD only happens when enable = true is set for it, since it is disabled by default
after the module is updated, the user just needs to restart neovim to fix any errors that pop up from updating the paths.

stsewd commented 4 years ago

@Happy-Dude I think we want to have a separate namespace so it doesn't collide with the main repo.

happy-dude commented 4 years ago

Okay yeah that makes sense; what would you guys like me to do?

I can wait for @steelsojka or close/edit this PR with the 2-lined changes in highlight_current_scope and make the changes for the textobjects repo?

EDIT: wording

stsewd commented 4 years ago

I think is fine changing those two lines so everything starts working again, we can decide if we want to change the structure later :)

happy-dude commented 4 years ago

Sounds good to me @stsewd -- created https://github.com/nvim-treesitter/nvim-treesitter-refactor/pull/4

Can close this PR 👍

steelsojka commented 4 years ago

@Happy-Dude Thanks for this. Sorry I didn't get back in a meaningful time. I will close this PR. Thanks for you contribution! I'm gonna close this in place of your other PR.