search

eclipse / lsp4e

Language Server Protocol support in Eclipse IDE
Eclipse Public License 2.0
62 stars 54 forks source link

Avoid assertion failure #1025

Closed BoykoAlex closed 4 months ago

BoykoAlex commented 4 months ago

Looks like checking for null edit is a proper thing to do as TextChange from LTK has an assert null for the previous edit. Assertion exception appears in e4.32 but looks like the PR above fixes it.

rubenporras commented 4 months ago

Under which conditions is the getEdit() not null?

Is that expected or do we have some programming error? If so, should we log a warning when it is not null with the necessary information to track down the problem?

rubenporras commented 4 months ago

e4.32

Looks like checking for null edit is a proper thing to do as TextChange from LTK has an assert null for the previous edit. Assertion exception appears in e4.32 but looks like the PR above fixes it.

Is this so? I could not find the git repo, but looking at the plugins in my target, the check is already part of eclipse e.4.31.

BoykoAlex commented 4 months ago

Okay, I have verified that the same occurs in 4.31 too.

The changes are grouped by resources. Under resources are leafs - the line changes.

Screenshot 2024-06-05 at 11 22 24

Clicking on different nodes and leafs in the changes tree in the wizard results in acquired doc count going up from 0 and back to 0 when doc is released. The doc release doesn't change the edit - it is set already. However, the code attempts to set it again when acquiredDocument(...) is called due to click on a leaf and acquired doc count being 0. Thus, I'd just guard against setting the edit if it is set already.

Don't think we need a warning. I don't see any obvious programming error. I think we are just missing the fact that acquired doc count can go back to 0 and then go up again.

rubenporras commented 4 months ago

Thanks