Skip to content

fix: error opening file when window.width is not lua number #790

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

Conversation

williamhCode
Copy link
Contributor

@williamhCode williamhCode commented Mar 8, 2023

fixes #789. One comment is that if the width is invalid, normally neo-tree still works and doesn't throw an error. However, since I added this fix, errors only appear if you happen to open a file when there's no other windows (which only happens rarely). This is a bit inconsistent and might confuse users, should we bother at all?

@williamhCode williamhCode force-pushed the 789-error-when-window-width-not-number branch from 35c92a1 to 3db3fef Compare March 8, 2023 15:40
@cseickel
Copy link
Contributor

cseickel commented Mar 8, 2023

errors only appear if you happen to open a file when there's no other windows (which only happens rarely). This is a bit inconsistent and might confuse users, should we bother at all?

I would say that in this context, we should just set width to the default value of 40 and not throw any errors. If we are going to validate the value, it should really be done in the setup method.

@williamhCode williamhCode force-pushed the 789-error-when-window-width-not-number branch from 3db3fef to aa8b27d Compare March 8, 2023 22:30
@williamhCode
Copy link
Contributor Author

I updated the commit so it matches nui's size handling more and sets the default value of 40. There's still things that don't quite match, such as if the width is very small and if the width is invalid. Because of this, I'm thinking maybe instead of doing a vsplit, we could just switch the current window to the file, then toggle the tree. Then, we don't have to try to match the width handling to nui.

@cseickel
Copy link
Contributor

cseickel commented Mar 9, 2023

we could just switch the current window to the file, then toggle the tree. Then, we don't have to try to match the width handling to nui.

That won't work. A lot of people get really finicky about seeing the window open and close. There's also the issue of the built-in window stealing prevention and having to keep the cursor and scroll in the same positions.

I think what you have here is good.

@cseickel cseickel merged commit cf2f90f into nvim-neo-tree:main Mar 9, 2023
@williamhCode williamhCode deleted the 789-error-when-window-width-not-number branch March 10, 2023 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

error opening file when window.width is not lua number and neo-tree is only opened window
2 participants