-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Read sources using the UTF_8 charset during hot reload compilation #3471
Read sources using the UTF_8 charset during hot reload compilation #3471
Conversation
I think that this is fine for now, but I would like to know what @gsmet thinks as well |
Thanks @geoand! I took some time to explore the dynamic encoding option. It's pretty simple with the Maven plugin (I've got some code ready for a review if needed) and not that simple with Gradle (I didn't find a clean way to retrieve the encoding so far). I'll put this on hold for now and continue the exploration later if needed. |
If you could get that working for Gradle too, it would be awesome. I am of the opinion that we should fix the most common case now and improve later on |
@@ -42,7 +43,8 @@ public void compile(Set<File> filesToCompile, Context context) { | |||
throw new RuntimeException("No system java compiler provided"); | |||
} | |||
DiagnosticCollector<JavaFileObject> diagnostics = new DiagnosticCollector<>(); | |||
try (StandardJavaFileManager fileManager = compiler.getStandardFileManager(diagnostics, null, null);) { | |||
try (StandardJavaFileManager fileManager = compiler.getStandardFileManager(diagnostics, null, | |||
StandardCharsets.UTF_8)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree we should have a fix in Wednesday's release.
Could you promote this PR and also create a proper GH issue. And I think it would be interesting to create a draft PR with your Maven work so that we don't lose it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR promoted. Regarding the GH issue, there's already #3467.
Merged, thanks! |
I created #3487 for the dynamic encoding issue. I'll continue investigating the Gradle case this week. |
Works around #3467