use stdout in stead of util.print and no stdout.flush()

d-vine commented 13 years ago

Dear FND

I'm using the jslint-reporter with node.js on windows and ran into two problems.

1) You use util.print for printing the errors (line 82) but stderr.write for everthing else. When using the wrapper from the console, this works fine, but running it as a subprocess from a python script I cannot access the console output of util.print. Changing it to stderr.write works fine.

2) process.stdout.flush() (line 187) results in an error on widows. commenting it out works fine. maybe you could find a way to make that work on all systems

Thank you for this very useful script!

Cheers Georg

FND commented 13 years ago

Thanks for the feedback!

1) You use util.print for printing the errors (line 82) but stderr.write for everthing else.

Indeed, it seems kinda asymmetrical - but that's what the Node standard library provided at the time.

It appears that util.print has now been deprecated - at least I can't find it in the docs for 0.4 or 0.5, yet it doesn't appear in the changelog either. Will need to investigate...

When using the wrapper from the console, this works fine, but running it as a subprocess from a python script I cannot access the console output of util.print. Changing it to stderr.write works fine.

This sounds like it might be a bug in the Windows build - can you create a minimal test case and raise an issue with the Node folks?

2) process.stdout.flush() (line 187) results in an error on widows. commenting it out works fine. maybe you could find a way to make that work on all systems

As it happens, this line was introduced only recently: https://github.com/FND/jslint-reporter/issues/1 I have to admit, I'm still not entirely sure why it's necessary, since I could never reproduce that issue on my machine.

Can you post the exact error, and also try working around the issue by wrapping that line in try { ... } catch(exc) {}? (I don't have access to a Windows machine myself.)

I'm using the jslint-reporter with node.js on windows and ran into two problems.

FWIW, it's generally preferable to create separate tickets for separate issues.

muszek commented 13 years ago

here's an error node throws at me. Ubuntu 11.04, nodejs v0.5.9.

muszek@homer:~/www/katarzynamucha/app/webroot/js$ jslint inventory.js 
inventory.js:92:1:'$' was used before it was defined.
inventory.js:92:3:'document' was used before it was defined.
inventory.js:92:27:Expected exactly one space between 'function' and '('.

node.js:203
        throw e; // process.nextTick error, or 'error' event on first tick
              ^
TypeError: Object #<WriteStream> has no method 'flush'
    at /home/muszek/scripts/jslint-reporter/wrapper.js:190:17
    at /home/muszek/scripts/jslint-reporter/wrapper.js:93:2
    at Object.<anonymous> (/home/muszek/scripts/jslint-reporter/wrapper.js:194:1)
    at Module._compile (module.js:432:26)
    at Object..js (module.js:450:10)
    at Module.load (module.js:351:31)
    at Function._load (module.js:310:12)
    at Array.<anonymous> (module.js:470:10)
    at EventEmitter._tickCallback (node.js:195:26)

I commented out line 190 (process.stdout.flush();) and it stopped complaining.

FND commented 13 years ago

Thanks for the information. I couldn't reproduce this issue with the current stable version of Node (0.4.12), and would rather wait for the 0.5 branch to hit stable before changing things again - so leaving this open in anticipation of future review.

FND commented 13 years ago

I felt compelled to fix this the brute-force way: 6c13985291f33eda54dd25084c46f45509a9d511

Please let me know if you have any further input.

FND / jslint-reporter

use stdout in stead of util.print and no stdout.flush() #2