Community
Participate
Working Groups
I20070417-0800 Progress notification in the FilteredItemsSelectionDialog needs some polish: - FilterJob#filterContent(GranualProgressMonitor) keeps 50% of the progress monitor for virtually no work (checking whether the PM has been cancelled and remembering results, which is a short-running operation). Due to the GranualProgressMonitor's coarse-grained updates, this results in only 40% of the PM being actually used for reporting search progress from #fillContentProvider(..). => The implementor's search job should get about 95% of the total PM. - The GranualProgressMonitor's decision to show only 20 discrete progress states is not helpful in practice. There's no good reason why the user should be informed of an update from 5% to 6%, but not for an update from 35% to 36%. => The best way to handle progress updates is not by displaying only selected progress steps, but by throttling the RefreshCacheJob to run only every 200ms. See also 178256 for delaying progress updates after filter changes.
Created attachment 67469 [details] Proposition of changes
The patch does not fix the problem in FilterJob#filterContent(..) that just 500 of 1000 ticks are available for reporting progress in the client's content provider. Furthermore, I would delay the RefreshProgressMessageJob a bit longer initially, e.g. 500 ms.
This will have to wait until 3.4
As this is blocking RC2 bugs this should be done for RC2.
Created attachment 68451 [details] Proposition of modifications 001
schedulePrgressRefresh is mispelled what is this code in internalWorked about? It seems awfully complicated so it will need some documentation if ((((int) (((worked - work) * 10) / totalWork)) < ((int) ((worked * 10) / totalWork)))|| (((int) ((worked * 10) / totalWork)) == 0)) if (!isCanceled()) updateProgressMessage();
Method ContentProvider#setProgressMessage should be removed. It is no longer used.
From the GranualProgressMonitor constructor isFiltering argument should be removed. It is not used.
(In reply to comment #5) > Created an attachment (id=68451) [details] > Proposition of modifications 001 - constructor FilterHistoryJob() has a wrong Javadoc - GranualProgressMonitor(..) Javadoc: remove "@param contentProvider" - schedulePrgressRefresh(): spelling error and broken @param Javadoc - first part of comment 0 / comment 2 not fixed - The change in updateProgressLabel() is problematic: Letting clients set the message to "" will interfere with the RefreshProgressMessageJob. Note that clients can also call refresh() or scheduleProgressMessageRefresh(), which will both set the progress message to "". You have to ensure that a) progress is being updated as long as a filter job is running b) progress is only set to "" after filter job is done I think this cannot be done without having a field somewhere that stores the current message.
Created attachment 68570 [details] Proposition of modifications 002
Szymon could you refersh this patch against HEAD please
(In reply to comment #10) > Created an attachment (id=68570) [details] > Proposition of modifications 002 Looks good in the runtime workbench when I change schedule(); in RefreshProgressMessageJob.scheduleProgressRefresh(..) to this: //Schedule with initial delay to avoid flickering when the user types quickly: schedule(200); Please add this initial delay to the updated patch.
Created attachment 68597 [details] Patch refreshed to head Updated according to Markus comment.
The version in HEAD looked good in a quick test drive. Problems in the source: - "receiver" was spelled correctly in 1.47. "reciever" is wrong. - Please add the suggested documentation (or a variation) from comment 12. The next one reading the source should see immediately why the delay is necessary.
Created attachment 68624 [details] Typos fix
Typo patch released.
