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

stdout not captured on Windows 8 #33

Closed
timdp opened this issue Jul 24, 2015 · 14 comments
Closed

stdout not captured on Windows 8 #33

timdp opened this issue Jul 24, 2015 · 14 comments

Comments

@timdp
Copy link
Contributor

timdp commented Jul 24, 2015

I originally reported this as steelbrain/linter#768:

I was originally facing #30, but I've further debugged the issue and it now looks like there is something awry in atom-linter 2.0.1. More specifically, on Windows 8 at least, Atom 1.0.2 seems unable to capture the child process's stdout.

I instrumented helpers.coffee to log the various stages of process creation. Basically, I see linter-js-standard invoke atom.exe with ATOM_SHELL_INTERNAL_RUN_AS_NODE=1. If the file has errors, then the status code differs, so it is linting. However, there is never any standard output and hence no errors.

In a Windows Command Prompt, I did a set on the variable above and then ran atom.exe with the same arguments. Interestingly, it immediately went to a blank prompt and then showed the output of the linter. It doesn't exhibit this behavior with a regular node.exe.

Additionally, I tried overriding command with node.exe (which is in my PATH), and that actually fixed the issue. Hence, it seems as though Electron run as Node behaves differently than a regular Node. It might very well be that the issue is with Electron instead. In that case, electron/electron#1463 should probably be reopened. However, since I've only seen it occur with atom-linter, I wanted to report it here first.

However, it now looks like this issue is linter-js-standard-specific. I've verified that linter-jshint works as expected, as @steelbrain suggested.

@ricardofbarros
Copy link
Owner

Thank you for reporting this, I will try to replicate in a windows machine.

@timdp
Copy link
Contributor Author

timdp commented Jul 24, 2015

It might be as simple as updating atom-linter. I see some Windows-related stuff in the log and 3.0.0 also literally just came out.

@ricardofbarros
Copy link
Owner

If I update atom-linter I might break this package for the rest of us, it all depends if there is now an option to disable the promise rejection.

@ricardofbarros
Copy link
Owner

Need to check the new updates on atom-linter

@timdp
Copy link
Contributor Author

timdp commented Jul 24, 2015

Yes. I just updated to 3.0.0 and it works if you pass in { throwOnStdErr: false } to bypass standard's silly welcome message.

@ricardofbarros
Copy link
Owner

Well it seems like it was implemented that option options.throwOnStdErr

@ricardofbarros
Copy link
Owner

Yeah, exactly 😄

@timdp
Copy link
Contributor Author

timdp commented Jul 24, 2015

Incidentally, #34 is still compatible as well. :-)

@ricardofbarros
Copy link
Owner

And the @3.0.0 works for you ?

@timdp
Copy link
Contributor Author

timdp commented Jul 24, 2015

Well, it's showing the linter messages I expected, so at least it's capturing stdout now.

@ricardofbarros
Copy link
Owner

Good, will review your PR and ⬆️ atom-linter to 3.0.0

@timdp
Copy link
Contributor Author

timdp commented Jul 24, 2015

Cool. Thanks!

@ricardofbarros
Copy link
Owner

Just released 2.2.0, could you see if it solves this issue?

@timdp
Copy link
Contributor Author

timdp commented Jul 24, 2015

It sure does. 👍

@timdp timdp closed this as completed Jul 24, 2015
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 a pull request may close this issue.

2 participants