-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix issue #1440, #1472: wrap BasedFileManager for jdk9 #1495
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
Conversation
Thanks for the pull request. Can you add your name the the AUTHORS file, so we are legally covered? Also, there is still a TODO in the code. Did you have a look at how the charset is used? |
I've added my name to AUTHORS. And I updated the TODO code to use default charset of JVM. |
I think we need to get the encoding passed to the compiler. If you can find out how to do that, great. Otherwise I will probably be able to figure it out. |
I digged the implementation of IntelliJ... The wrappedManager is created in org.jetbrains.jps.javac.JavacMain.wrapWithCallDispatcher(), and it proxies JavaFileManager to org.jetbrains.jps.javac.JavacFileManager. One possible way to get encoding is get OutputFileObject instance using JavaFileManager#getJavaFileForOutput(), and access myEncodingName private field. |
I successfully get the encoding. The code is 4298d97. |
Note that I get a Proxy-wrapped manager using Win10/Gradle and has nothing to do w/ IntelliJ. So there needs to be a wrapper solution that does not depend on IntelliJ internals. You can pass |
As far as I know, both compilers use JavaC uder the hood, I'll see if it is possible to get to the encoding passed to the compiler. |
Alternatively, and if everything else fails, we can use the default platform encoding unless a lombok specific property is set. |
@wrprice Can you point me to the code that gradle uses to create the wrapper? |
I don't know at this time, but I'll start looking. Per my comments on #1472 I was coming at this from the other side. I know the end result is essentially the same, but not the cause or its location. |
Well, that was quick. A GitHub search for That leads to two implementation classes in the org.gradle.api.internal.tasks.compile.reflect package. |
Oh, cool. It even has the encoding in it. |
What about #1435, all these issues seem related regressions in .18 |
I've created a workaround to fix the issue #1440, #1472 for IntelliJ + Java9.
It works for me with IntelliJ 2017.3 EAP.
The wrappedManager is proxied with java.lang.reflect.Proxy, so it can't be cast to BaseFileManager.
So I created BaseFileManagerWrapper to wrap it.