Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 107753

Summary: [Decorators] Exception handling message in decoration job could be improved
Product: [Eclipse Project] Platform Reporter: Michael Valenta <Michael.Valenta>
Component: UIAssignee: Krzysztof Daniel <krzysztof.daniel>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P5 CC: daniel_megert, Kevin_McGuire, krzysztof.daniel, Tod_Creasey
Version: 3.2Keywords: helpwanted
Target Milestone: 3.5 M5   
Hardware: PC   
OS: Windows XP   
Whiteboard: hasPatch
Bug Depends on: 251193    
Bug Blocks:    
Attachments:
Description Flags
Fix
none
tests
none
Patch with tests
none
Fix
none
Is this better? none

Description Michael Valenta CLA 2005-08-23 13:10:43 EDT 
It appears that the decoration manager will disable a decoration when it 
throws a runtime exception. This behavior is good but the error message should 
be improved. The current message is: 

   Exception in Decorator

An improved message woudl be something like

   Exception in CVS decorator. This decorator has been disabled.
Comment 1 Tod Creasey CLA 2007-06-14 15:03:16 EDT 
There are currently no plans to work on this although we would be happy to review a patch
Comment 2 Krzysztof Daniel CLA 2008-08-19 11:09:26 EDT 
Created attachment 110346 [details]
Fix

New StatusHandling facility is used.
Comment 3 Kevin McGuire CLA 2008-10-03 10:55:55 EDT 
Christophe, can you also attach a test?
Comment 4 Krzysztof Daniel CLA 2008-10-14 09:31:03 EDT 
I tried to write a JUnit test but this is quite difficult. In the meantime I have found that status handling was introduced there once and reverted (see bug 169226). 

Tod, do you remember why the patch was reverted?
Comment 5 Tod Creasey CLA 2008-10-14 09:48:41 EDT 
I'm afraid I don't recall but I suspect it was something to do with the exception being in the UI Thread.


There are crash tests already for decorators in the UI test suite - you may want to start from there.
Comment 6 Krzysztof Daniel CLA 2008-10-17 05:00:18 EDT 
The workbench is closed while the status handling animation is proceeding during JUnit test.
Comment 7 Krzysztof Daniel CLA 2008-10-17 05:46:08 EDT 
Created attachment 115371 [details]
tests

Kevin, I am not sure about those tests... any hint?
Comment 8 Krzysztof Daniel CLA 2008-10-23 07:42:43 EDT 
Created attachment 115915 [details]
Patch with tests
Comment 9 Kevin McGuire CLA 2008-12-01 12:15:30 EST 
Krzysztof, sorry I never reviewed this.  Do you want to just commit it now that you have rights or do you still want me to review?
Comment 10 Krzysztof Daniel CLA 2008-12-15 06:49:15 EST 
Created attachment 120456 [details]
Fix
Comment 11 Krzysztof Daniel CLA 2008-12-15 06:53:04 EST 
I have resigned from introducing SH in the decorators, only updated message as requested. Fix released on 2008-12-15
Comment 12 Dani Megert CLA 2008-12-16 03:32:01 EST 
The new code:

	String message = WorkbenchMessages.DecoratorError;
	if (decorator != null) {
		message += " " + 
		NLS.bind(WorkbenchMessages.DecoratorWillBeDisabled, //$NON-NLS-1$
		decorator.getName());
	}

is not correctly NLSed: concatenating strings in code will cause problems for translation and hence mixing NLSed strings with concatenation in code must be avoided.
Comment 13 Krzysztof Daniel CLA 2008-12-16 03:49:29 EST 
Created attachment 120545 [details]
Is this better?

Thanks for paying attention!
Comment 14 Dani Megert CLA 2008-12-16 04:02:32 EST 
Yes, this is better.
Comment 15 Krzysztof Daniel CLA 2008-12-16 04:05:23 EST 
String concatenation removed on 2008-12-16