Community
Participate
Working Groups
The Externalize Strings wizard offers the option "Use Eclipse's string externalization mechanism" which is a superior choice. However, it does not default to that option when available nor does it remember the user's selection of that option for future invocations of the wizard. It should be encouraging use of the newer mechanism, especially since converting form the old to the new is a tedious manual process.
Would a patch contribution, if received within the next couple of days, have any chance for 3.5? I know M6 (API freeze) has passed but I'm not sure if this would be considered "too much" for M7 inclusion. What about SR1? Are UI changes permitted in the service releases?
Eric, thanks for the offer. As long as your patch does not add new API and does not hit the size that triggers a legal review (250 added/changed/deleted lines) we can still accept it for M7. >What about SR1? Are UI changes permitted in the service releases? Only if it fixes a critical bug, which isn't the case here.
Created attachment 131431 [details] Patch for org.eclipse.jdt.ui (against HEAD) Attached a patch that makes the default be the Eclipse NLS mechanism when it is available (ie, org.eclipse.osgi is on the classpath of the project being "refactored").
The patch goes into the right direction. What I don't like is the duplicate call to NLSRefactoring.setIsEclipseNLS(boolean). Can you also add the code that remembers the last setting? Also, please add your credentials to the copyright notice of each file in the following form: name <e-mail> - bugzilla summary - bugzilla URL e.g. Dani Megert <dani@eclipse.org> - this is a bug - https://bugs.eclipse.org...
I am not crazy about the duplicate call, either. But the only other option involves changing the way detectIsEclipseNLS() works (see how it tries to determine the false state and, failing that returns the current value of the instance variable) and because that is public I was hesitant to do so. Do you think it is prudent to hack away on detectIsEclipseNLS() at this stage? As for remembering the last selection: that seems to get a bit complicated because that logic has to cooperate with the detection of whether or not the OSGi NLS class is available. Because of that "requirements" complication it feels like a larger scope than I really need or want to tackle right now. I'll await further feedback on these two before proceeding.
You could extract the part that tests whether there's an accessor type in the CU from detectIsEclipseNLS() to its own private method. You can then combine this new method with the other one and call setIsEclipseNLS(boolean) just once. >As for remembering the last selection: that seems to get a bit complicated OK. I suggest you file a new bug report for that so we can close this one once fixed.
Created attachment 131951 [details] Patch for org.eclipse.jdt.ui (against HEAD) Attached a new patch that addresses feedback from comment 4. I am hesitant to embed my email address in source code; hopefully that is not a hard requirement. Also, I added a TODO comment about the null check in the beginning of NLSRefactoring.detectIsEclipseNLS(); it can be removed if it does not apply.
Hi Eric, thanks for the updated patch! Applied patch with the following modifcations: 1) removed the null-check as you suggested 2) also return the new default from the JavaModelException 3) adjusted copyright date in ExternalizeWizardPage >I am hesitant to embed my email address No problem but I have to keep it in my files and provide it to the EMO for their files. Committed to HEAD. Available in builds > N20090416-2000.
