Bug 119818 - [60cat] search results sort buttons gone
[60cat] search results sort buttons gone
Status: VERIFIED FIXED
Product: utilities
Classification: Unclassified
Component: Search
6.x
All All
: P3 with 2 votes (vote)
: 6.x
Assigned To: Victor Vasilyev
issues@utilities
: NETFIX, REGRESSION, UI
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-10-23 17:20 UTC by gholmer
Modified: 2010-03-23 13:19 UTC (History)
3 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
binary patch for NB 6.0 (67.28 KB, application/octet-stream)
2007-11-14 16:30 UTC, Marian Petras
Details
updated binary patch for NB 6.0 (67.08 KB, application/octet-stream)
2007-11-14 22:18 UTC, Marian Petras
Details
First patch... (11.03 KB, patch)
2009-07-23 04:32 UTC, Michel Graciano
Details | Diff
Second patch... (4.54 KB, patch)
2010-01-25 18:46 UTC, Michel Graciano
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description gholmer 2007-10-23 17:20:31 UTC
[ BUILD # : beta 2 ]
[ JDK VERSION : 1.5.* ]

The radio buttons that used to appear in search results are no longer
present.  This is a regression and should be repaired.

If you search across a large number of files for all occurrences of a
string, the results are very difficult to read if the file names are
not sorted.
Comment 1 Marian Petras 2007-10-30 17:06:44 UTC
Confirmed. Postponed for an update release (e.g. an updated module published on the update centre) or to the next release.
Comment 2 tfrysinger 2007-11-01 06:35:13 UTC
This is a *really* inconvenient regression. I use Find quite a bit in larger projects, particularly as a way to locate
classes that have I know belong to a particular named 'family', and without a sort capability it severely hampers being
able to zero in on which result is the one I want to open.
Comment 3 Marian Petras 2007-11-14 16:26:29 UTC
I have created an initial implementation of sorting by name. I committed it to CVS branch "results_sorting". I only
tested it very briefly and made only a very simple UI which is far from perfect (just two radio-buttons at the bottom of
search results). Feel free to polish it.

Modified files:
    utilities/src/org/netbeans/modules/search/:
                                          ResultView.java   (1.44.2.1)
                                          Bundle.properties   (1.46.4.1)
                                          ResultTreeModel.java   (1.6.6.1)
                                          ResultModel.java   (1.60.6.1)

Diffs:
http://deadlock.netbeans.org/fisheye/browse/netbeans/utilities/src/org/netbeans/modules/search/ResultView.java?r1=1.44&r2=1.44.2.1
http://deadlock.netbeans.org/fisheye/browse/netbeans/utilities/src/org/netbeans/modules/search/Bundle.properties?r1=1.46&r2=1.46.4.1
http://deadlock.netbeans.org/fisheye/browse/netbeans/utilities/src/org/netbeans/modules/search/ResultTreeModel.java?r1=1.6&r2=1.6.6.1
http://deadlock.netbeans.org/fisheye/browse/netbeans/utilities/src/org/netbeans/modules/search/ResultModel.java?r1=1.60&r2=1.60.6.1
Comment 4 Marian Petras 2007-11-14 16:30:30 UTC
Created attachment 52985 [details]
binary patch for NB 6.0
Comment 5 Marian Petras 2007-11-14 16:33:46 UTC
I have attached a binary patch for NB 6.0, providing the mentioned initial implementation of sorting search results.
To apply it, put the attached file ("binary-patch-bug119818-NB6.0.jar") to subdirectory

     ide8/modules/patches/org-netbeans-modules-utilities

of your NetBeans installation directory. (Not tested, feedback welcome.)
Comment 6 gholmer 2007-11-14 17:32:06 UTC
Thanks very much for working on this!

The contents of the search results window disappear when scrolled, and this exception is reported:
SEVERE [global]
java.lang.IndexOutOfBoundsException: Index: 25, Size: 10
	at java.util.ArrayList.RangeCheck(ArrayList.java:546)
	at java.util.ArrayList.get(ArrayList.java:321)
	at org.netbeans.modules.search.ResultTreeModel.getChild(ResultTreeModel.java:118)...

Comment 7 Marian Petras 2007-11-14 17:57:17 UTC
How many matching files were reported in the root node of the search results window in that case? was it closer to 10 or
to 25 (see the numbers reported in the exception)?
Comment 8 gholmer 2007-11-14 19:30:42 UTC
I tried to repeat the same test.  There are no errors and the results scroll normally until I click the "Sorted by Name"
button, then the red exception indicator in the lower right lights up even before I try to scroll.  The search reported
46 matches in 26 files.  The exception had the same text: java.lang.IndexOutOfBoundsException: Index: 25, Size: 10.
Comment 9 Marian Petras 2007-11-14 22:04:34 UTC
I see. This happens if there are multiple files of the same name (in different directories).
Comment 10 Marian Petras 2007-11-14 22:15:26 UTC
I fixed the bug with the IndexOutOfBoundsException.

Modified file:
    utilities/src/org/netbeans/modules/search/ResultModel.java   (1.60.6.2)

Diff:
http://deadlock.netbeans.org/fisheye/browse/netbeans/utilities/src/org/netbeans/modules/search/ResultModel.java?r1=1.60.6.1&r2=1.60.6.2
Comment 11 Marian Petras 2007-11-14 22:18:06 UTC
Created attachment 53012 [details]
updated binary patch for NB 6.0
Comment 12 Marian Petras 2007-11-14 22:18:45 UTC
I have updated an updated binary patch - it should solve the issue with the IndexOutOfBoundsException.
Comment 13 gholmer 2007-11-15 12:57:18 UTC
Works great now, thanks again!
Comment 14 gholmer 2008-01-04 18:12:12 UTC
Will this go into 6.1? 6.0.1?
Comment 15 Marian Petras 2008-01-04 18:29:03 UTC
I do not know the rules for integrating bugfixes into patch releases of NB 6.0. So I do not know at the moment.

I added the "REGRESSION" keyword (as this actually is a regression) which might increase the chance. 
Comment 16 Marian Petras 2008-01-23 23:58:20 UTC
This is a regression but it requires a UI change. The button was removed when search&replace was implemented - and the
Replace button added. If the Replace button is displayed, it is at the same position as the sorting radio buttons were -
so this must be resolved first. I will not make such a UI change in NB 6.1 - I have more important fixes to do.
Comment 17 Tomas Stupka 2008-10-03 09:48:27 UTC
this is very annoying with a large hitlist ...
Comment 18 Andrey Yamkovoy 2008-12-18 10:52:13 UTC
Since this functionality was removed by Marian Petras while rewriting the results tree for replace functionality it
looks for me as a new feature implementation. And this is out of scope for the 7.0 now. Of course if I have time I would
fix it for 7.0.
Comment 19 Jiri Kovalsky 2009-03-27 14:56:58 UTC
Andrey Yamkovoy agreed that he would review and integrate a patch for this issue contributed by the NetFIX [1] team.

[1] http://wiki.netbeans.org/NetFIX
Comment 20 Michel Graciano 2009-07-22 05:38:50 UTC
I started it and will attach the patch soon. More tests needed.
Comment 21 Michel Graciano 2009-07-23 04:32:06 UTC
Created attachment 85100 [details]
First patch...
Comment 22 Michel Graciano 2009-07-23 04:33:32 UTC
I attached an proposed patch. Comments and review are welcome.

Regards
Comment 23 ulfzibis 2009-07-23 12:01:40 UTC
Michel,
1. What you think, "Unsort" could be good for? Likely you could save footprint + GUI space on having only 1 toggle button.
Instead of "Unsort", IMO there were more interesting options: Sort by [simple name | full package name | source root |
project dependency]

2. Am I right, that you want to perform partly sort, before search is entirely completed?
- be aware, if setSorted() is thread-safe against concurrently searching thread. (I guess it should be interrupted while
copying from matchingObjects).
- sortedMatchingObjects.containsAll(matchingObjects) could become expensive operation on ArrayList.
-- maybe comparing size of both list's would suffice, as size of matchingObjects likely never decreases.
-- containsAll() would be faster on SortedSet for sortedMatchingObjects.
- you could save clear() and sort() if using SortedSet for sortedMatchingObjects, and can do partly addAll(), if saving
last used size of matchingObjects for start index of addAll().
- I you waive from "Unsort", you could save copying to sortedMatchingObjects and directly sort matchingObjects, even
partly, if search thread is interrupted while sorting.
Comment 24 ulfzibis 2009-07-23 12:23:38 UTC
> - I you waive from "Unsort", you could save copying to sortedMatchingObjects and directly sort matchingObjects, even
> partly, if search thread is interrupted while sorting.
In this case Collections.sort() may be faster than collecting results in SortedSet. Should be tested.
If partly sort is performed often, I guess collecting results in SortedSet should be faster than repeated full 
Collections.sort(), as method setSorted() only should run UPDATE_SORTED_CHILDREN_TASK.
Comment 25 Andrey Yamkovoy 2009-08-04 11:32:28 UTC
From the user point of view:
- The "Unsort" function does not make sense. Agree with ulfzibis. In my opinion the list always should be sorted by name
but grouping (simple name, phisical view, logical view etc) should be based on the user choice. 
- The grouping contols should be placed on the toolbar in the left side of output window to have more space for search
results. Existing buttons in the bottom should be moved to it too (see issue 169129).

As for the code the comments from ulfzibis looks reasonable and should be investigated and probably "integrated" into
the patch.
Comment 26 Jiri Kovalsky 2009-10-14 13:39:27 UTC
Michel, can you please improve the patch in accordance with recent comments from Ulf? Thanks a lot!
Comment 27 Michel Graciano 2009-10-14 14:34:46 UTC
Okey, I will make some changes and post the new patch as soon as possible. BTW, I just used the same UI in previous 
version as users could know. The unsort button just show the results as original, just after search is finished.
In my original tests, the sort is possible just when search is finished. The button should be disabled during search.

Thanks for the comments.
Comment 28 Victor Vasilyev 2009-12-09 05:40:01 UTC
This issue has very clear explanation: "the results are very difficult to read if the file names are not sorted".
I don't see any benefits if the search results will have an unsorted representation. 
I guess, if the list of files will always be sorted according to their names then this issue will be completely resolved.

Note, such solution doesn't require any UI changes, and the display space will be used for results, but not for controls that will be used rarely (or even never!).

Any objections?
Comment 29 Michel Graciano 2010-01-25 17:21:57 UTC
Well, I agree with Victor, we don't need any UI change if the behaviour was to order by file names. If you agree about it, I can provide an patch soon.
Comment 30 Michel Graciano 2010-01-25 18:46:41 UTC
Created attachment 93540 [details]
Second patch...

Well, I changed my patch to doesn't change anything in UI, just reorder the view when any new object is found. I need to make more performance tests but looks good for now. I would like to ask you to review it, any comment and improvement are welcome.
Comment 31 Jiri Kovalsky 2010-01-26 04:51:10 UTC
Andrey, can you please review the patch and integrate it if you don't find any problems? Thanks!
Comment 32 Alexei Mokeev 2010-01-26 05:00:51 UTC
Hi Jirka, Michel,

Victor is going to care about this issue. However I could not promise that this will happen prior to M1 :)

-A.
Comment 33 Jiri Kovalsky 2010-01-26 05:08:41 UTC
No problem Alexei. Just don't forget to integrate the patch to trunk before 6.9 is branched, please. Thanks!
Comment 34 Victor Vasilyev 2010-02-01 19:05:20 UTC
Fixed
http://hg.netbeans.org/main/rev/64b5cdf19f2b

The proposed patch (id=93540) has been applied.
Comment 35 Jiri Kovalsky 2010-02-02 04:36:45 UTC
Thanks a lot Victor. And of course thanks Michel for the patch!
Comment 36 Quality Engineering 2010-02-03 21:44:23 UTC
Integrated into 'main-golden', will be available in build *201002040200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/64b5cdf19f2b
User: Victor G. Vasilyev <vvg@netbeans.org>
Log: #119818 -  [60cat] search results sort buttons gone. The patch with id=93540 has been applied.
Comment 37 Victor Vasilyev 2010-03-18 08:16:09 UTC
Regression.
The patch destroys logic of both actions "Go to previous matching string" and "Go to next matching string" due to different orders in the lists of elements in view and model.
Temporarily the fix is reverted.
Comment 38 Michel Graciano 2010-03-18 16:53:29 UTC
Is there someone looking this or could I take care of it? BTW, thanks for the rolback in time.
Comment 39 Quality Engineering 2010-03-19 05:10:28 UTC
Integrated into 'main-golden', will be available in build *201003190200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/266d90ec7f0e
User: Victor G. Vasilyev <vvg@netbeans.org>
Log: Regression after fix #119818. Temporarily the fix is reverted.
Comment 40 Victor Vasilyev 2010-03-20 04:15:11 UTC
Fixed again in the main trunk
http://hg.netbeans.org/main/rev/26c411a6285d
Comment 41 Victor Vasilyev 2010-03-22 12:15:46 UTC
(In reply to comment #38)
> Is there someone looking this or could I take care of it?

Michel,

If you have interest in the resolving of this issue then,  could you please, test the search feature after my changes to be sure that all is OK.

Thanks,
Victor
Comment 42 Michel Graciano 2010-03-22 14:15:58 UTC
(In reply to comment #41)
> (In reply to comment #38)
> > Is there someone looking this or could I take care of it?
> 
> Michel,
> 
> If you have interest in the resolving of this issue then,  could you please,
> test the search feature after my changes to be sure that all is OK.
> 
> Thanks,
> Victor

Hi Victor,
thanks for your fix. I tested all features I could but looks like click 'Show All Details' action nothing is shown at output tab.
Should we reopen it or file another issue, since I can't found any problem at search ordering, it is awesome now.

v. Build 100322-8d160876e56c
Comment 43 Victor Vasilyev 2010-03-22 14:40:40 UTC
(In reply to comment #42)
Thanks for the testing. 
Please, file a separate bug report against utilities/Search if the 'Show
All Details' action issue exists.
Comment 44 Victor Vasilyev 2010-03-23 04:29:02 UTC
I can reproduce the 'Show All Details' action issue.
The bug 182540 has been filed.
Comment 45 Michel Graciano 2010-03-23 13:19:10 UTC
I agree. Thanks for your really quick fix. Marking this as verified and will be a pleasure to verify the another one too. Thanks again.


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo