Skip to content

fix: harden render against stale window and node inputs#615

Closed
dekaravanhoc wants to merge 1 commit intoMeanderingProgrammer:mainfrom
dekaravanhoc:fix/null_object_errors
Closed

fix: harden render against stale window and node inputs#615
dekaravanhoc wants to merge 1 commit intoMeanderingProgrammer:mainfrom
dekaravanhoc:fix/null_object_errors

Conversation

@dekaravanhoc
Copy link

Add compatibility fallbacks and defensive guards so mixed plugin states or transient windows do not crash scheduled renders, and keep tests runnable on Neovim builds that use --clean instead of --noplugin.

Hi,

I am using https://github.com/sudo-tee/opencode.nvim and had often several errors with the plugin because of prop access of non existing objects. And seldom on larger md files.
Now everything runs smoothly.

Best regards

Add compatibility fallbacks and defensive guards so mixed plugin states or transient windows do not crash scheduled renders, and keep tests runnable on Neovim builds that use --clean instead of --noplugin.
@MeanderingProgrammer
Copy link
Owner

I have no idea what problems this PR is attempting to solve. This whole thing smells of AI slop, sorry if it's not and this is just you trying your best. If you want to give back to open source projects evaluate each change critically and understand why each line makes sense or not. Otherwise I just feel less and less inclined to accept external contributions.

Below is me feedback for each file:

base.lua

My plugin is the only thing that should ever be calling the execute method, and it will always call it with an instance of the node class and never with the raw nodes. I don't want to enable any external code to use it, it is strictly for internal use and has no plans on being backwards compatible.

env.lua

Just provides a wrapper env.buf.windows method which calls the existing env.buf.wins method. Which like I guess you could argue one name over the other, but it's a pointless refactor. And if you're going to go for the refactor then change all the existing usage to the new one, there is no value in keeping both around.

view.lua

Adds unnecessary checks for the existence of methods in a module. In theory you would need to add these everywhere for it to provide any value and it would make the code awful to work with. Assuming that methods we add to a module will exist is the only sane approach.

Adding a validity check on windows might be useful but I'm not sure it is necessary. Creating a reproducible example of when not checking causes a problem should be possible. Definitely not needed in the contains method since valid is checked before calling contains.

justfile

What builds of neovim do not have access to --noplugin but do have access to --clean?

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.

2 participants