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

Bug 366529

Summary: Preferences DefaultPreferences shouldn't trim() product preference values
Product: [Eclipse Project] Equinox Reporter: James Blackburn <jamesblackburn+eclipse>
Component: ComponentsAssignee: equinox.components-inbox <equinox.components-inbox>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: daniel_megert
Version: 3.7.1   
Target Milestone: Juno M5   
Hardware: PC   
OS: Linux-GTK   
Whiteboard:
Bug Depends on:    
Bug Blocks: 394359    

Description James Blackburn CLA 2011-12-13 08:44:28 EST 
DefaultPreferences trim()s the default value of preferences. This means it's impossible to set a product default which ends in a new-line, say.  This is important as some prefs (such as auto-generated comments, commands to be run, etc.) are whitespace sensitive.  So while it's possible to represent these preference values as Instance prefs, setting them as default doesn't work...

DefaultPreferences.translatePreference(String, Properties) line: 384	
DefaultPreferences.applyDefaults(String, Properties, Properties) line: 144	
DefaultPreferences.applyProductDefaults() line: 198	
DefaultPreferences.load() line: 242	
DefaultPreferences(EclipsePreferences).create(EclipsePreferences, String, Object) line: 308	
DefaultPreferences(EclipsePreferences).internalNode(String, boolean, Object) line: 544	
DefaultPreferences(EclipsePreferences).node(String) line: 670	
DefaultScope(AbstractScope).getNode(String) line: 38	
DefaultScope.getNode(String) line: 76	
ScopedPreferenceStore.getDefaultPreferences() line: 250	
ScopedPreferenceStore.getPreferenceNodes(boolean) line: 285
Comment 2 James Blackburn CLA 2011-12-13 14:49:34 EST 
Thanks DJ!

Possible issue: there seems to be some logic which looks for whitespace later on:
  int ix = value.indexOf(" "); //$NON-NLS-1$
Could this now go wrong?

Perhaps a safer bet would be:
String retVal = value.trim();
...
existing logic uses retVal
...
// fallback 
return  value;
Comment 3 DJ Houghton CLA 2011-12-13 15:21:45 EST 
Hmm... yep good catch. I did some tests with spaces but didn't go into the loop.
Comment 4 DJ Houghton CLA 2011-12-13 16:00:45 EST 
Ok, I've released that change but it still doesn't quite work if we have a substitution with whitespace at the end of the default value. But considering no one has complained about it yet and I can't find the doc spec'ing what that would look like, I think we're ok for now.