Using UTF_8 as default in core.resources.PreferenceInitializer
https://github.com/eclipse-platform/eclipse.platform.resources/pull/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 :stopwatch: - 6m 21s 3 819 tests - 153 3 793 :white_check_mark: - 157 22 :zzz: ±0 4 :x: + 4 12 054 runs - 459 11 883 :white_check_mark: - 469 161 :zzz: ±0 10 :x: +10
For more details on these failures, see this check.
Results for commit d4d794a9. ± Comparison against base commit 0f64336d.
This pull request removes 153 tests.
AllCompareTests org.eclipse.compare.tests.AsyncExecTests ‑ testCancelOnRequeue
AllCompareTests org.eclipse.compare.tests.AsyncExecTests ‑ testQueueAdd
AllCompareTests org.eclipse.compare.tests.AsyncExecTests ‑ testWorker
AllCompareTests org.eclipse.compare.tests.CompareFileRevisionEditorInputTest ‑ testPrepareCompareInputWithNonLocalResourceTypedElements
AllCompareTests org.eclipse.compare.tests.CompareUIPluginTest ‑ testFindContentViewerDescriptorForTextType_StreamAccessor
AllCompareTests org.eclipse.compare.tests.CompareUIPluginTest ‑ testFindContentViewerDescriptor_TextType_NotStreamAccessor
AllCompareTests org.eclipse.compare.tests.CompareUIPluginTest ‑ testFindContentViewerDescriptor_UnknownType
AllCompareTests org.eclipse.compare.tests.ContentMergeViewerTest ‑ testFFFX
AllCompareTests org.eclipse.compare.tests.ContentMergeViewerTest ‑ testFFTX
AllCompareTests org.eclipse.compare.tests.ContentMergeViewerTest ‑ testFFXF
…
@iloveeclipse
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:
https://github.com/eclipse-platform/eclipse.platform/blob/0f64336d2bc4c785643f38d429bd36bdbae0337e/resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/Workspace.java#L2339-L2383
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?
@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. I will be able ti review this later, hopefully on Friday, as I'm two days mostly not on my workstation.
@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 for the clarification
Thanks Ed! Lars, it os not as trivial as one could think, I recommend to read the code Ed referenced and related tickets. I will be able ti review this later, hopefully on Friday, as I'm two days mostly not on my workstation.
See also https://github.com/eclipse-platform/eclipse.platform/issues/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.
@vogella
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:
https://github.com/eclipse-platform/eclipse.platform/blob/0f64336d2bc4c785643f38d429bd36bdbae0337e/resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/Workspace.java#L2354-L2357
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:
- org.eclipse.ui.ide.dialogs.ResourceEncodingFieldEditor.findDefaultEncoding()
- org.eclipse.ui.WorkbenchEncoding.getWorkbenchDefaultEncoding()
When pressing the Restore Defaults:
- org.eclipse.ui.ide.dialogs.ResourceEncodingFieldEditor.loadDefault()
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.