Using UTF_8 as default in core.resources.PreferenceInitializer

[vogella]

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.

[github-actions]

Test Results

 1 653 files   -  81   1 653 suites   - 81   1h 21m 19s ⏱️ - 6m 21s
 3 819 tests  - 153   3 793 ✅  - 157   22 💤 ±0   4 ❌ + 4 
12 054 runs   - 459  11 883 ✅  - 469  161 💤 ±0  10 ❌ +10 

For more details on these failures, see this check.

Results for commit d4d794a. ± Comparison against base commit 0f64336.

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
…

[merks]

@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:

eclipse.platform/resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/Workspace.java

Lines 2339 to 2383 in 0f64336

	/**
	* Writes explicit workspace encoding to workspace preferences
	* ("org.eclipse.core.resources/encoding"). If the user started Eclipse with
	* explicit encoding set (-Dfile.encoding=XYZ), this encoding used, otherwise
	* UTF-8.
	* <p>
	* With that, if user did not specified any encoding, all projects created in
	* this workspace will get explicit UTF-8 encoding set.
	*/
	private void setExplicitWorkspaceEncoding() {
	// ResourcesPlugin.getEncoding() defaults to JVM "file.encoding" value
	// which is *always* set in the JVM, so it can't be used.
	// Therefore check preferences directly (encoding could be set even in a new
	// workspace via product customization file, like
	// org.eclipse.core.resources/encoding=ISO-8859-1)
	String defaultEncoding = Platform.getPreferencesService().getString(ResourcesPlugin.PI_RESOURCES,
	ResourcesPlugin.PREF_ENCODING, /* default */null, /* all scopes */null);
	if (defaultEncoding == null || defaultEncoding.isBlank()) {
	// Check if -Dfile.encoding= startup argument was given to Eclipse's JVM
	List<String> commandLineArgs = ManagementFactory.getRuntimeMXBean().getInputArguments();
	for (String arg : commandLineArgs) {
	if (arg.startsWith("-Dfile.encoding=")) { //$NON-NLS-1$
	defaultEncoding = arg.substring("-Dfile.encoding=".length()); //$NON-NLS-1$
	// continue, it can be specified multiple times, last one wins.
	}
	}
	// Now we are sure user didn't specified encoding, so we can set
	// default encoding for the workspace
	if (defaultEncoding == null || defaultEncoding.isBlank()) {
	defaultEncoding = "UTF-8"; //$NON-NLS-1$
	}
	// Persist encoding to the workspace preferences - if same workspace is started
	// later by other Eclipse instance without product customization or system
	// property or with differently set properties, we want keep same encoding as on
	// the first startup. Otherwise users may end up with projects in same workspace
	// created with different encodings. This is surely supported but would surprise
	// users sooner or later. So better to set it first time, users can always
	// change it later if needed.
	IEclipsePreferences workspacePrefs = InstanceScope.INSTANCE.getNode(ResourcesPlugin.PI_RESOURCES);
	workspacePrefs.put(ResourcesPlugin.PREF_ENCODING, defaultEncoding);
	Assert.isTrue(defaultEncoding.equals(ResourcesPlugin.getEncoding()));
	}
	}

But the initializer logic of this PR hard codes the value.

[vogella]

@merks I do not get what is different in your review request? Can you explain?

[merks]

@merks I do not get what is different in your review request? Can you explain?

I accidentally removed him and then added him back.

[iloveeclipse]

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.

[vogella]

@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

[vogella]

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 #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.

[merks]

@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:

eclipse.platform/resources/bundles/org.eclipse.core.resources/src/org/eclipse/core/internal/resources/Workspace.java

Lines 2354 to 2357 in 0f64336

	String defaultEncoding = Platform.getPreferencesService().getString(ResourcesPlugin.PI_RESOURCES,
	ResourcesPlugin.PREF_ENCODING, /* default */null, /* all scopes */null);
	if (defaultEncoding == null || defaultEncoding.isBlank()) {

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:

[vogella]

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.