Bug 487400 - [classpath] Enable filtering for classpath tab entries
Summary: [classpath] Enable filtering for classpath tab entries
Status: RESOLVED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Debug (show other bugs)
Version: 4.6   Edit
Hardware: All other
: P3 enhancement (vote)
Target Milestone: 4.7 M7   Edit
Assignee: Christian Buck CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2016-02-07 05:48 EST by Christian Buck CLA
Modified: 2017-03-17 04:29 EDT (History)
1 user (show)

See Also:


Attachments
Patch for filtered classpath entries (8.43 KB, patch)
2016-02-07 05:48 EST, Christian Buck CLA
no flags Details | Diff
Revised patch addressing points 1 and 2 from comment #1 (9.42 KB, patch)
2016-02-14 09:26 EST, Christian Buck CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christian Buck CLA 2016-02-07 05:48:18 EST
Created attachment 259617 [details]
Patch for filtered classpath entries

Hi all,

when having projects with very large classpaths the classpath tab entries for run and debug configurations become hard to read.
Common use cases like looking for duplicates or searching for a certain entry are time consuming and error prone.
A possible workaround is opening .classpath in a text editor and using common text search abilities, but this is somehow inconsistent with using an IDE.

After an especially frustrating instance of manual classpath parsing I decided to have a look at the JDT source and suddenly had a working implementation of a filtering classpath tree. The user is able to enter a filter expression using the same well-known UI used in several other places in Eclipse.

I essentially changed RuntimeClasspathViewer from is-a-TreeViewer to has-a-FilteredTree, which also fixes an API misuse (TreeViewer must not be subclassed).
Please note that I have no experience at all in neither using SWT nor Eclipse development, so I kindly ask to review accordingly ;)

The attached patch should apply cleanly to current head.

To follow legal requirements: This contribution complies with http://www.eclipse.org/legal/CoO.php.

Thanks

Christian
Comment 1 Sarika Sinha CLA 2016-02-07 23:15:35 EST
Thanks for contributing. I have not yet reviewed it at code level, but functionality wise couple of points if you could look at :
1. Search does not require * for "Entries" . For other partial search I need to use *. It will be good to be consistent.
2. When I clear my search it collapses the entries. It should bring to the default view (What we get to see the first time)
3. If I give the filter word as Entries, I get both Entries , but I can't expand it. I understand that is the normal Filter Tree behavior. Not must, but will be good to be able to expand them, if it is not much of an effort.
Comment 2 Christian Buck CLA 2016-02-14 09:26:20 EST
Created attachment 259749 [details]
Revised patch addressing points 1 and 2 from comment #1
Comment 3 Christian Buck CLA 2016-02-14 09:26:51 EST
Thanks for looking into this!

Regarding your points:

Ad 1. This is actually the default behaviour of PatternFilter, i.e. matching with an implicit trailing wildcard. I tried to keep defaults as much as possible, but I agree that in this case it's not intuitive.
I changed the PatternFilter to match with an implicit leading wildcard, too. Is this better now?

Ad 2. I fully agree.
Due to the filter tree refresh running asynchronously I saw no obvious trivial solution. See RuntimeClasspathViewer.RuntimeClasspathFilteredTree.doCreateRefreshJob() for what I settled on (re-using existing code from FilteredTree seems to be the cleanest way to me).

Ad 3. I'm really not sure what the expected outcome should be. In my use cases I'd like to see all leaf (i.e. non-collapsible) nodes matching the filter pattern. If there are none except the "*Entries" grouping nodes, what would I expect to see when expanding those?

Please find attached the revised patch with changes for points 1 and 2 plus some additional minor cleanups. In RuntimeClasspathViewer.RuntimeClasspathFilteredTree.textChanged() I added a request for review regarding the several different ways to check for empty filter expressions I found elsewhere in the existing code. Perhaps this is a fitting moment to plan some additional code unifications, but please regard that only as an suggestion and remove the comment as you see fit.

Thanks

Christian
Comment 4 Christian Buck CLA 2016-02-28 09:37:22 EST
I'm sorry, was I supposed to reassign back to you? Did that now.

Thanks

Christian
Comment 5 Sarika Sinha CLA 2016-02-28 21:46:08 EST
(In reply to Christian Buck from comment #4)
> I'm sorry, was I supposed to reassign back to you? Did that now.
> 
> Thanks
> 
> Christian

No, You remain as the Asignee... Sorry for the delay . I will be reviewing it this week.
Comment 6 Sarika Sinha CLA 2016-04-25 04:25:42 EDT
My bad, Couldn't look at it in detail.
Will target it in 4.7 M1
Comment 7 Sarika Sinha CLA 2017-03-15 01:40:34 EDT
@Christian,
Looks good after the changes, can you please sign ECA ?
Comment 8 Christian Buck CLA 2017-03-16 03:18:35 EDT
Sure, signed ECA just now.