-
Notifications
You must be signed in to change notification settings - Fork 25
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
Fix update of proof tree in case of filter changes (fixes #3367) #3368
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #3368 +/- ##
=========================================
Coverage 37.98% 37.98%
Complexity 17024 17024
=========================================
Files 2059 2059
Lines 126029 126029
Branches 21282 21282
=========================================
Hits 47874 47874
Misses 72272 72272
Partials 5883 5883 ☔ View full report in Codecov by Sentry. |
Thanks for the fast response! This PR fixes issue #3367. However, while testing the changes, I think I found another problem that is related (at the moment already present on main branch, but not before the merge of #3349): |
I fixed the new issue in the other PR (but we can simply cherry-pick that commit if this PR should be used). Previously the KeY's selection changed events were misused to cause updates of the GUI (but creating other problems) even so no other proof or node had been selected. The committed fix is not yet ideal, but the main window now redraws when pretty printer settings change. A proper solution would use the ChangeListener (or something similar like moving these changes to the Config class). |
see remark at #3367 (comment) instead |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tested the GUI for the use cases that I could think of. Everything worked except the following minor bug: The selection highlights of expanded OSS nodes are not painted anymore (this worked in 2.12.0). The nodes are selected and the sequent is updated as expected (a feature implemented by @FliegendeWurst), just the selection highlight is not (it stays at the parent node).
Thanks. The highlighting is fixed. It fixes also an exception, if a filter like "Hide Closed Subtrees" is activated, while an OSS child node is selected. |
…ettings upon switching
…or proof tree view
Each view setting update caused it own redraw even if itself did not change.
…tered correctly only for changes of interest
This commit fixes also exceptions when activating a filter like "Hide Closed Subtrees" when an OSS child node was selected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I tried to come up with the common use cases and tested them. Everything seems to work now.
Related Issue
This pull request addresses #3367
Intended Change
Activating a filter works immediately.
The underlying problem is that the GUIProofTreeModel stores the selection and does not update with the selection model in the ProofTreeView.
This PR does not maintain the consistency. The redundant information was seemingly only thought to be used to store the selected node when switching proofs and restoring it when switching back. The method setFilter should for that reason not query the model for the selection but the view to get an up-to-date path.
Previously it got null (despite a node being selected in the view) and did not activate the filter.
Type of pull request
Ensuring quality
Additional information and contact(s)
The contributions within this pull request are licensed under GPLv2 (only) for inclusion in KeY.