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

Save and restore proof tree view filters upon proof switching #3369

Closed
wants to merge 12 commits into from

Conversation

unp1
Copy link
Member

@unp1 unp1 commented Dec 1, 2023

Related Issue

This pull request addresses #3367

The PR is an addition to the immediate bug fix PR #3368. It adds a new feature.

Intended Change

It removes some information from the GUIProofTreeModel that was kept redundant and only used for restoring the view state after proof switching.

The proof tree view states are now exported and saved explicitly thus allowing to also restore the filter settings on a per proof basis.

Type of pull request

  • Bug fix (non-breaking change which fixes an issue)
  • Refactoring (behaviour should not change or only minimally change)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • There are changes to the (Java) code
  • There are changes to the taclet rule base
  • There are changes to the deployment/CI infrastructure (gradle, github, ...)
  • Other:

Ensuring quality

  • I made sure that introduced/changed code is well documented (javadoc and inline comments).
  • I made sure that new/changed end-user features are well documented (https://github.com/KeYProject/key-docs).
  • I added new test case(s) for new functionality.
  • I have tested the feature as follows: Opened different proofs with different filter settings and switching between them
  • I have checked that runtime performance has not deteriorated.

Additional information and contact(s)

The contributions within this pull request are licensed under GPLv2 (only) for inclusion in KeY.

@unp1 unp1 self-assigned this Dec 1, 2023
@unp1 unp1 added the Feature New feature or request label Dec 1, 2023
@unp1 unp1 added the Review Request Waiting for review label Dec 1, 2023
Copy link

codecov bot commented Dec 1, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7a43f16) 37.96% compared to head (8da89ae) 37.98%.
Report is 10 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main    #3369      +/-   ##
============================================
+ Coverage     37.96%   37.98%   +0.01%     
- Complexity    17015    17024       +9     
============================================
  Files          2059     2059              
  Lines        126012   126029      +17     
  Branches      21281    21282       +1     
============================================
+ Hits          47845    47874      +29     
+ Misses        72282    72272      -10     
+ Partials       5885     5883       -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@unp1
Copy link
Member Author

unp1 commented Dec 1, 2023

The latest push fixes a last bug I found when selecting two node filter in succession and removes some duplicate code. From my side (pending reviewer feedback), this PR is done (in case we want the feature)

@WolframPfeifer
Copy link
Member

I can see pros and cons in having per-proof view filters. Somewhat recently, I had a discussion with @FliegendeWurst about this because he had a similar idea, and back then we decided not to make the filters proof dependent. Personally, I use the filters very often when working with KeY. I frequently switch between hidden/visible intermediate states (which roughly corresponds to the structure of the proof/detailed steps). In my opinion, it would be a bit annoying (in particular after reloading) if I had to set this option for each individual proof.

However, I would like to hear other opinions on this.

@WolframPfeifer WolframPfeifer added the RFC "Request for comments" is the appeal for making and expressing your opinion on a topic. label Dec 1, 2023
@unp1
Copy link
Member Author

unp1 commented Dec 1, 2023

Hi,
I am fine with both semantics. But I would prefer this PR instead of the other:

  • I can change it (easily) to preserve the old behavior (if that is preferred)
  • this PR improves somewhat code quality and also the number of code lines (despite adding more comments)

unp1 added a commit that referenced this pull request Dec 8, 2023
@WolframPfeifer
Copy link
Member

In our last KaKeY meeting, we decided to keep the current (non-local) filters, since (a) making them proof dependent would create a new category of settings (per-proof view settings) and (b) I prefer the filter settings to be global and basically no one else had a clear preference here.

@unp1 Can this PR be closed now?

@unp1 unp1 closed this Dec 14, 2023
@unp1
Copy link
Member Author

unp1 commented Dec 14, 2023

Thanks!

wadoon pushed a commit that referenced this pull request Dec 26, 2023
wadoon added a commit to wadoon/key that referenced this pull request Dec 28, 2023
* origin/main: (25 commits)
  bug fix
  applying spotless
  Configurable enabled keys for JML condition evaluation
  Spotless cleanups
  Fix selection highlight for OSS node child
  Remove unnecessary checks for correct change as listener is now registered correctly only for changes of interest
  Make usage of PropertyChangeListeners working (addendum to previous commit)
  Reduce number of sequentview updates
  Remove duplicate update of sequentview
  USe refactorings from KeYProject#3369 but preserve non-local filter semantics for proof tree view
  MainWindow updates sequent view after settings change for pretty printing
  Minor clean up to slightly simplify complexity of path selection in ProofTreeView
  Fix problem with closed subtree filter
  Fix node filter selection display in popup dialoh and some refactoring
  Restore correct node selection and remove unused fields from GUIProofTreeModel
  Store ProofTreeViewSettings per proof outside model and restore all settings upon switching
  Fix update of proof tree in case of filter changes (fixes KeYProject#3367)
  applying spotless
  Bump com.miglayout:miglayout-swing from 11.2 to 11.3
  Configurable enabled keys for JML condition evaluation
  ...
wadoon added a commit that referenced this pull request Dec 29, 2023
* origin/main: (25 commits)
  bug fix
  applying spotless
  Configurable enabled keys for JML condition evaluation
  Spotless cleanups
  Fix selection highlight for OSS node child
  Remove unnecessary checks for correct change as listener is now registered correctly only for changes of interest
  Make usage of PropertyChangeListeners working (addendum to previous commit)
  Reduce number of sequentview updates
  Remove duplicate update of sequentview
  USe refactorings from #3369 but preserve non-local filter semantics for proof tree view
  MainWindow updates sequent view after settings change for pretty printing
  Minor clean up to slightly simplify complexity of path selection in ProofTreeView
  Fix problem with closed subtree filter
  Fix node filter selection display in popup dialoh and some refactoring
  Restore correct node selection and remove unused fields from GUIProofTreeModel
  Store ProofTreeViewSettings per proof outside model and restore all settings upon switching
  Fix update of proof tree in case of filter changes (fixes #3367)
  applying spotless
  Bump com.miglayout:miglayout-swing from 11.2 to 11.3
  Configurable enabled keys for JML condition evaluation
  ...

# Conflicts:
#	key.ui/build.gradle
wadoon added a commit that referenced this pull request Jan 12, 2024
* origin/main: (2237 commits)
  Bump org.slf4j:slf4j-api from 2.0.10 to 2.0.11
  Bump org.slf4j:slf4j-api from 2.0.9 to 2.0.10
  spotless
  after discussion: all datatypes are free
  ignore files in */antlr4/gen/*
  bug fix
  applying spotless
  Configurable enabled keys for JML condition evaluation
  Spotless cleanups
  Fix selection highlight for OSS node child
  Remove unnecessary checks for correct change as listener is now registered correctly only for changes of interest
  Make usage of PropertyChangeListeners working (addendum to previous commit)
  Reduce number of sequentview updates
  Remove duplicate update of sequentview
  USe refactorings from #3369 but preserve non-local filter semantics for proof tree view
  MainWindow updates sequent view after settings change for pretty printing
  Minor clean up to slightly simplify complexity of path selection in ProofTreeView
  Fix problem with closed subtree filter
  Fix node filter selection display in popup dialoh and some refactoring
  Restore correct node selection and remove unused fields from GUIProofTreeModel
  ...

# Conflicts:
#	key.core/src/main/java/de/uka/ilkd/key/logic/sort/AbstractSort.java
#	key.core/src/main/java/de/uka/ilkd/key/logic/sort/ParametricSort.java
#	key.core/src/main/java/de/uka/ilkd/key/logic/sort/ParametricSortInstance.java
#	key.core/src/main/java/de/uka/ilkd/key/logic/sort/ProgramSVSort.java
#	key.core/src/main/java/de/uka/ilkd/key/logic/sort/Sort.java
#	key.core/src/main/java/de/uka/ilkd/key/nparser/builder/DeclarationBuilder.java
#	key.core/src/main/java/de/uka/ilkd/key/proof/init/ProblemInitializer.java
#	key/key.core/src/main/java/de/uka/ilkd/key/logic/op/SortDependingFunction.java
#	key/key.core/src/main/java/de/uka/ilkd/key/logic/sort/NullSort.java
#	key/key.core/src/main/java/de/uka/ilkd/key/proof/io/IProofFileParser.java
#	key/key.core/src/main/java/de/uka/ilkd/key/rule/inst/GenericSortCondition.java
#	key/key.core/src/main/java/de/uka/ilkd/key/rule/inst/GenericSortInstantiations.java
#	key/key.core/src/test/java/de/uka/ilkd/key/parser/TestDeclParser.java
#	key/key.util/src/main/java/org/key_project/util/collection/ImmutableList.java
#	key/key.util/src/main/java/org/key_project/util/collection/ImmutableSLList.java
#	key/key.util/src/main/java/org/key_project/util/collection/Immutables.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature New feature or request Review Request Waiting for review RFC "Request for comments" is the appeal for making and expressing your opinion on a topic.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proof tree filters not applied correctly
2 participants