Bug 212166 - [patch] C/C++ Search View bugfixes and improvements
Summary: [patch] C/C++ Search View bugfixes and improvements
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-core (show other bugs)
Version: 4.0   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 4.0.3   Edit
Assignee: Ken Ryall CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2007-12-06 12:52 EST by Ed Swartz CLA
Modified: 2008-06-22 02:37 EDT (History)
2 users (show)

See Also:


Attachments
proposed patch and unit tests (36.84 KB, patch)
2007-12-06 12:52 EST, Ed Swartz CLA
bjorn.freeman-benson: iplog+
Details | Diff
another patch for a bug in IndexModelUtil (1.17 KB, patch)
2007-12-06 14:04 EST, Ed Swartz CLA
bjorn.freeman-benson: iplog+
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ed Swartz CLA 2007-12-06 12:52:22 EST
Created attachment 84644 [details]
proposed patch and unit tests

Build ID: 20071130

This patch provides improvements to the C/C++ Search view to address some UI and operational bugs from our customers.  

1) Search results not found, either due to indexing not enabled or indexing in progress (dups bug 158955).

-- To solve this, status messages are injected into the search results indicating that the results may be incomplete or inaccurate if the indexer is running at the time.  Such messages are always sorted to the top to make them immediately available.  (Other UI choices, like dialogs or status line messages or modifying the search results label, were considered and rejected as bad UI.)

-- Also, for any projects included in the search scope which have no indexer enabled, a warning node is added indicating why the search could not provide any results for the project.  

-- Finally, if a closed project is explicitly included in the search scope and has no hits, a warning node is added indicating the results are not known for closed projects.  (Bug 142192 seems related to this, but that seemed like older behavior where search results WERE provided, but were unnavigable.)

2) Lack of organization in search results.  Results are unnavigable when more than a few dozen are present, since they appear in the random order provided by the index.  

-- To fix this, in both the list and tree views, results are sorted by content type and then by label.

3) Lack of organization in external file results.  Results from external files appear at the same level and are mixed in with results from projects.  Searches in large SDKs usually provide many more hits from external files than projects, but users are likely to want results from projects first.  And, when searching through SDK headers, results from the same directory are meaningful.

-- To fix this, the tree view categorizes external files under directory nodes, sorted by path.  Under these directories, file entries are sorted by filename.

4) Search results label did not show total number of matches.

-- Fixed

5) Re-searching would sometimes result in empty results.

-- Fixed some issues in the IContentProvider implementations.
Comment 1 Ed Swartz CLA 2007-12-06 14:04:39 EST
Created attachment 84661 [details]
another patch for a bug in IndexModelUtil

I left this out of the previous patch.  It would result in the sort being broken when enums were present in the results.
Comment 2 Ed Swartz CLA 2008-01-02 15:26:54 EST
Changed from enhancement to normal bug, since it does address a few open bugs as well as the internal bug here which prompted the work.  And also I want some attention for this issue :)

BTW, if accepted, please add to 4.0.x.
Comment 3 Doug Schaefer CLA 2008-01-02 15:34:36 EST
Is someone looking at this?
Comment 4 Ken Ryall CLA 2008-01-18 17:55:31 EST
Applied patch to HEAD and cdt_4_0.
Comment 5 Markus Schorn CLA 2008-01-21 11:20:39 EST
The testcase BasicSearchTest.testExternalPathRenderedCorrectly_79193() is failing. Without actually checking this, I assume the failure is related to the patches applied.
Comment 6 Ken Ryall CLA 2008-01-21 11:32:48 EST
I had some problems applying the patch so I may have gotten something wrong, I'll review it again.
Comment 7 Ed Swartz CLA 2008-01-21 11:34:39 EST
Yes, that test has a patch (and the class has four more tests).  I verified it works against our local CDT 4.0.x sources, so it's probably a merge error.
Comment 8 Ken Ryall CLA 2008-01-21 14:52:26 EST
I ran the tests again and they passed again but finally found an intermittent failure. After reviewing with Ed I've committed a fix to the test so this should work in all cases now.