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

Add Filter Mechanism for Target in Move Refactoring#18 #1246

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ShahzaibIbrahim
Copy link

@ShahzaibIbrahim ShahzaibIbrahim commented Mar 9, 2024

To let user filter the projects or packages from a search bar in a move refactor dialog.

Fixes #1337

What it does
User can now search from the large tree of projects and packages to move the file to.

How to test
Go to any file, right click --> Refactor --> Move --> Dialog Box will open with a treeview of Project and Packages --> Type in the search box and see if the filters are working fine

[x] I have thoroughly tested my changes
[x] The change is following the coding conventions
[x] I have signed the Eclipse Contributor Agreement (ECA)

Copy link
Contributor

@HeikoKlare HeikoKlare left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested the provided functionality and found it to be, as expected, almost equivalent to the one proposed in #981. But since it reuses the existing FilteredTree, it is much cleaner and avoids duplication.

This implementation does still not perform well on our workspace. Once I stop typing, it takes several seconds to update the viewer. This seems to be because the generic filter logic of FilteredTree (and the used getChildren() implementation of the refactoring's content provider) is quite expensive to calculate. But since that is not an issue of newly added functionality but given by the implementation of FilteredTree, we can address that independently via improvements to FilteredTree afterwards.
image

I see one option for slightly improving the behavior when having a large workspace in which the filter update takes a long time: the debounce delay might be increased from FilterTree's default setting of 200ms, as that value is quite low and can easily be exceeded between two character inputs. The value could be increased to something like 500ms by overwriting FilteredTree.getRefreshJobDelay() to return an according value.

I would even be in favor of the changes without that modification, as this is a quick win for having a filter mechanism in the move refactoring. And no one is even forced to use it.

@ShahzaibIbrahim
Copy link
Author

I see one option for slightly improving the behavior when having a large workspace in which the filter update takes a long time: the debounce delay might be increased from FilterTree's default setting of 200ms, as that value is quite low and can easily be exceeded between two character inputs. The value could be increased to something like 500ms by overwriting FilteredTree.getRefreshJobDelay() to return an according value.

@HeikoKlare I have applied your suggestion which gives us slightly more time before debouncing. The value for it is 500ms now. There is a pull request in platform.ui which needs to be merged before this PR now. eclipse-platform/eclipse.platform.ui#1824

Text Search added to search withing the tree while moving (refactoring) the file to different package

eclipse-jdt#1337
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.

Add Filter Mechanism for Target in Move Refactoring
2 participants