Skip to content
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

use UTF-8 as default encoding #1020

Merged
merged 1 commit into from
Jan 17, 2025
Merged

Conversation

astrograph
Copy link
Contributor

the rest of Eclipse now uses UTF-8 as default encoding.

The tm.terminal code reverts to null when the encoding begins with "Default" and this leads to UTF-8 being selected from Charset.defaultEncoding in stead of ISO-8859 when the displayed default value is selected.

Copy link
Member

@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @astrograph

The next release is a major release, so this change is well timed as we have a bit more freedom to change defaults, etc.

I think a little more needs to be done to change this, for example this line looks like it needs updating too

int index = encodingCombo.indexOf("ISO-8859-1".equals(encoding) ? "Default (ISO-8859-1)" : encoding); //$NON-NLS-1$ //$NON-NLS-2$

There are a few other places that look like an incorrect default is in place too? search for ISO-8859-1 in terminal/**/*.java

WDYT?

@astrograph
Copy link
Contributor Author

astrograph commented Jan 16, 2025

Thanks @astrograph

The next release is a major release, so this change is well timed as we have a bit more freedom to change defaults, etc.

I think a little more needs to be done to change this, for example this line looks like it needs updating too

int index = encodingCombo.indexOf("ISO-8859-1".equals(encoding) ? "Default (ISO-8859-1)" : encoding); //$NON-NLS-1$ //$NON-NLS-2$

There are a few other places that look like an incorrect default is in place too? search for ISO-8859-1 in terminal/**/*.java

WDYT?

I added a commit with some more changes to reflect the new default encoding, I think these are all occurances

Copy link

github-actions bot commented Jan 16, 2025

Test Results

   601 files  ±0     601 suites  ±0   12m 50s ⏱️ -24s
10 200 tests ±0  10 177 ✅ ±0  23 💤 ±0  0 ❌ ±0 
10 238 runs  ±0  10 215 ✅ ±0  23 💤 ±0  0 ❌ ±0 

Results for commit 01e3b03. ± Comparison against base commit 957bb48.

♻️ This comment has been updated with latest results.

@jonahgraham
Copy link
Member

There are some IT issues that I am trying to resolve + wait on Eclipse IT team for. Please disregard any failed builds or other failed checks for now.

Copy link
Member

@jonahgraham jonahgraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great. Thanks @astrograph for the improvement!

I added a N&N entry

Once CI passes I can submit this.

the rest of Eclipse now uses UTF-8 as default encoding.

The tm.terminal code reverts to null when the encoding
begins with "Default" and this leads to UTF-8 being
selected from Charset.defaultEncoding instead of ISO-8859
when the displayed default value is selected.
@jonahgraham jonahgraham merged commit 3439e0e into eclipse-cdt:main Jan 17, 2025
5 checks passed
@astrograph astrograph deleted the patch-1 branch January 20, 2025 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants