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

[NETBEANS-2832] Added input reading to Gradle projects. #1461

Merged
merged 1 commit into from Sep 1, 2019

Conversation

@lkishalmi
Copy link
Contributor

lkishalmi commented Aug 25, 2019

No description provided.

Copy link
Contributor

matthiasblaesing left a comment

Looks sane with a minimal comment inline.

@@ -152,6 +156,12 @@ public void run() {

outStream = new EscapeProcessingOutputStream(new GradlePlainEscapeProcessor(io, config, false));
errStream = new EscapeProcessingOutputStream(new GradlePlainEscapeProcessor(io, config, true));
try {
inStream = new ReaderInputStream(io.getIn(), "UTF-8"); //NOI18N

This comment has been minimized.

Copy link
@matthiasblaesing

matthiasblaesing Aug 26, 2019

Contributor

Is the JVM running the daemon configured to used UTF-8 as default encoding? I know that the java ecosystem is moving to UTF-8 as the default encoding, but is this a safe assumption here?

This comment has been minimized.

Copy link
@lkishalmi

lkishalmi Aug 26, 2019

Author Contributor

AFAIK the daemon runs specifically set with UTF-8 encoding. However the IDE might run in a different one and that's the encoding we are reading from.
I'd let this as it is to see how it work for the users before start to over-engineer the thing. We might have a problem with EOL as well, as IDE uses always LF, however the platform might use CRLF on the Gradle side.

This comment has been minimized.

Copy link
@matthiasblaesing

matthiasblaesing Aug 26, 2019

Contributor

Actually the IDE should not be the problem. The input comes directly from Swing, so it is (module bugs) clean, then it is passed through a UTF-8 writer, that converts the string to a byte sequence. So if the target is clean, this should be stable (yeah famous last words). I would also let it stay as is.

@lkishalmi lkishalmi merged commit 547263d into apache:master Sep 1, 2019
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@junichi11 junichi11 added this to the 11.2 milestone Sep 2, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.