Skip to content

[TRACKING] issue followup for refactoring, rewrites and features planned. #877

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
10 of 12 tasks
kyazdani42 opened this issue Dec 25, 2021 · 15 comments
Closed
10 of 12 tasks

Comments

@kyazdani42
Copy link
Member

kyazdani42 commented Dec 25, 2021

Hi everyone !
I'm currently doing small refactoring iterations of the plugin. Here is a list of the refactoring and features/rewrite i want to achieve in the next few months. Theses lists will be completed incrementally.

@kyazdani42 kyazdani42 pinned this issue Dec 25, 2021
@im-n1

This comment was marked as off-topic.

@perkfly
Copy link

perkfly commented Jan 3, 2022

I was gonna raise an issue about hanging, but since the code is being refactored, I would just describe my issue here briefly.

It seems that nvimtree always read git info when refreshing the tree even with git.enable = false, which causes vim to hang a very long time or forever if I press R for a few times. Please make git an optional requirement, or better, make it stop hanging when reading git statuses.

Nice plugin, BTW.

@kirillbobyrev
Copy link

Is this intended to be a in-place rewrite (i.e. users won't have to do anything, the code will be in the same repo and the plugin will be updated once the rewrite is done)?

@kyazdani42
Copy link
Member Author

@yifeikong are you sure ? All git calls are protected by a config check. The slowdown might com from the full tree refresh. With the new version it should dramatically reduce the refresh time because of the reload being scoped to a folder only.

@kirillbobyrev I'm not sure yet, and although i've started rewriting it, i'm a bit concerned about this rewrite taking too long. I have a 20% working version, but the main improvements i wanted to do are written already, so i might just port the code to the existing repository.

I'll keep you guys informed when i can, but i've been really tired lately because of work. I hope i'll be better after some holidays :)

@vuki656
Copy link

vuki656 commented Jan 12, 2022

@yifeikong are you sure ? All git calls are protected by a config check. The slowdown might com from the full tree refresh. With the new version it should dramatically reduce the refresh time because of the reload being scoped to a folder only.

@kirillbobyrev I'm not sure yet, and although i've started rewriting it, i'm a bit concerned about this rewrite taking too long. I have a 20% working version, but the main improvements i wanted to do are written already, so i might just port the code to the existing repository.

I'll keep you guys informed when i can, but i've been really tired lately because of work. I hope i'll be better after some holidays :)

Take your time. The current plugin works and does the job good enough.

No need to rush anything.

Thanks for your work.

@zhongjis
Copy link

zhongjis commented Jan 16, 2022

Hey just want to come by and say thank you :) Nice plugin XD and please take your time XD

@SystematicError
Copy link

Eagerly awaiting! This plugin is already very cool, but a refactor will make it much better, thanks for the effort you are putting into this!

@13k
Copy link

13k commented Feb 1, 2022

Just a tip from a bug a found: don't initialize configuration-dependent values in immutable variables like in the renderer.

These immutable values are set automatically when require("nvim-tree") is called and any configuration setup after that is ignored (like setting vim.g.* variables or even the actual setup call).

My use case for plugins is like this: I use pcall(require, ...) to check for a plugin's existence, fallback gracefully if it doesn't, otherwise I setup a bunch of other stuff, that may or may not depend on the plugin's own code, which I need to require to be available. So, require'ing a plugin shouldn't initialize immutable config values.

I'm not sure if I should open an issue for this since the rewrite is a good chance for a fix and fixing it in the current code base would require a big refactoring.

Also, from a programming point of view, this trend in lua plugins to think the common case is require("plugin").setup(...) is very wrong in my opinion. Importing libraries/modules, in general, should be an idempotent operation with very limited side effects (ideally none at all) and it should simply expose the library's API to the client.

@lopi-py
Copy link
Contributor

lopi-py commented Feb 2, 2022

Awesome plugin!, If I can contribute to rewrite, reply me, I really want to help in this rewrite

@kyazdani42
Copy link
Member Author

@13k i realized this issue with global vim variables, that's why i started to work on the setup a while ago....

Also the setup thing is controversial for some people, but given how to plugin managers initialize the plugins, it actually makes a lot of sense and allows for a clean interface between the plugin developers and the users. It also enables the plugin being initialized lazily.
I agree their might be some other better way, but i didn't find one and i'm happy enough with those setup functions, and i think most people are. Sometimes you need to compromise a bit, specially regarding "conventions".

@lopi-py Any help is appreciated, i'll use this issue to track the ongoing refactoring that must be done in a week or two, i still have to maintain the issues and i have a lot of them unread, which takes quite a lot of time ^^

@kraegpoeth
Copy link

I love your plugin, so wanted to first of all say a big thanks and really appreciate your work. I hope the rewrite will come 💪

I was about to post a bug about how :bd with NvimTree showing, crashes entire nvim session. Dunno if there is a workaound or hotfix for this? Let me. know how/where i can find some logs or anythign to send you way.

Again, thanks a lot for this nvim plugin - its sooo cool i really like it! 🙏

@kyazdani42
Copy link
Member Author

@kraegpoeth which buffer do you delete ? It might be because of the auto_close option, set it to false in the setup might fix this.

@kyazdani42 kyazdani42 changed the title [DEPRECATION NOTICE] rewrite 🎄 [REFACTO] issue followup for refactoring, feature freeze meanwhile. Feb 5, 2022
@kyazdani42 kyazdani42 changed the title [REFACTO] issue followup for refactoring, feature freeze meanwhile. [REFACTO] issue followup for refactoring, rewrites and features planned. Feb 5, 2022
@kyazdani42 kyazdani42 changed the title [REFACTO] issue followup for refactoring, rewrites and features planned. [TRACKING] issue followup for refactoring, rewrites and features planned. Feb 5, 2022
@nvim-tree nvim-tree locked and limited conversation to collaborators Feb 5, 2022
@alex-courtis
Copy link
Member

  • Help cleanup & improvements (ongoing, format has been revamped)

@kyazdani42 what further changes are you looking for? #1327 looks pretty good ;)

@kyazdani42
Copy link
Member Author

indeed, i've forgot to update this one :)

@alex-courtis alex-courtis unpinned this issue Oct 15, 2022
@alex-courtis
Copy link
Member

Last task tracked at #457 , closing.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests