Specify UTF8 encoding. - Githubissues

vidstige / jadb

ADB Client in pure Java.

Apache License 2.0

640 stars 178 forks source link

Specify UTF8 encoding. #141

Open adamoutler opened 3 years ago

adamoutler commented 3 years ago

This fixes a problem with UTF-16 chars being read as less number of chars than expected which results in an unclosed expression expecting a closing single quote.

vidstige commented 2 years ago

Thanks for your contribution! Can you expand a bit on what is going wrong exactly? Perhaps I can add a unit test case for this as well.

adamoutler commented 2 years ago

Not needed. This is a best practice and recommended by static analysis tools. The system defaults to whatever the system default is set to. This constraints it to UTF-8 which is recommended. In the case the system is set to UTF-16, emojis fail.

adamoutler commented 2 years ago

It's preparation for when this repo upgrades to Java 11/16.

vidstige commented 2 years ago

Please rebase your branch on top of latest master to trigger a build :pray:

vidstige commented 2 years ago

There seems to be lot's of unrelated formatting, etc. Please add these in separate commits if needed at all.
How wide-spread is Java 11? What other versions would it be reasonable to upgrade to? Keep in mind that this is a library, so requiring too high version will prevent people from upgrading.

janosvitok commented 2 years ago

I've quickly run over this PR.

I think the only relevant change is the one line with addittion of StandardCharsets.UTF_8. The rest should be reverted. StandardCharsets is part of Java 7, so there's no need to upgrade.

It's strange that @adamoutler removed tests for strings with diacritics -- that's where UTF-8 is really used.

System.out.println is for temporary debugging and should have been removed prior to commit/PR.

Other than that, I agree that it is safer to explicitly specify encoding.

adamoutler commented 2 years ago

Yes. I guess I included items which should not have been in there. The UTF-8 part was discovered as the root of the other problems.