-
Notifications
You must be signed in to change notification settings - Fork 115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Using UTF_8 as default in core.resources.PreferenceInitializer #1462
Using UTF_8 as default in core.resources.PreferenceInitializer #1462
Conversation
eclipse-platform/eclipse.platform.resources#51 implemented logic to set UTF-8 as standard. I suggest to also set the default preference value to UTF-8.
Test Results 1 653 files - 81 1 653 suites - 81 1h 21m 19s ⏱️ - 6m 21s For more details on these failures, see this check. Results for commit d4d794a. ± Comparison against base commit 0f64336. This pull request removes 153 tests.
|
Sorry for the reviewer bounce. I have a feeling this is not correct. Looking at the logic here, the default depends on a bunch of things such as the system property or the encoding preference: Lines 2339 to 2383 in 0f64336
But the initializer logic of this PR hard codes the value. |
@merks I do not get what is different in your review request? Can you explain? |
I accidentally removed him and then added him back. |
Thanks Ed! Lars, it os not as trivial as one could think, I recommend to read the code Ed referenced and related tickets. |
Thanks for the clarification |
See also #1458, currently the reset of the preference sets the encoding to an undesired value. This is NOT changed by this PR, I could not find the responsible preference initializer. |
My concern is that if the preference is set, then the logic at line 2357 will find a non-blank value and will not longer consider the explicit file.encoding system property: Lines 2354 to 2357 in 0f64336
When I debugged this I see we do get to org.eclipse.core.internal.resources.PreferenceInitializer.initializeDefaultPreferences() before we get to org.eclipse.core.internal.resources.Workspace.setExplicitWorkspaceEncoding() so this is definitely of concern. In the preferences page, these things are all of interest:
When pressing the Restore Defaults:
Unfortunately when debugging it sets it to UTF-8 which is not what I see in actual IDE when I do that... Anyway, more debugging is required. We need to be cautious that this seemingly innocent change doesn't actually break the logic that @iloveeclipse has carefully implemented and tested... I see that this test failed which appears to be the effect I describe above: |
Thanks @merks for spending so much time on it. I also tried to find the place in which the preference page flips back to the wrong value. Maybe @iloveeclipse may find it, as he implemented the changed in the past. |
eclipse-platform/eclipse.platform.resources#51 implemented logic to set UTF-8 as standard.
I suggest to also set the default preference value to UTF-8.