Skip to content
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

Avoid assertion failure #1025

Merged
merged 4 commits into from
Jun 6, 2024
Merged

Avoid assertion failure #1025

merged 4 commits into from
Jun 6, 2024

Conversation

BoykoAlex
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

BoykoAlex commented Jun 5, 2024

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 rubenporras merged commit 51e467d into eclipse:main Jun 6, 2024
5 checks passed
@rubenporras
Copy link
Contributor

Thanks

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.

None yet

2 participants