73rhodes / Sublime-JSLint

JSLint for Sublime Text 2 and 3.
80 stars 14 forks source link

No output with exit code=1 #4

Closed AxxL closed 11 years ago

AxxL commented 11 years ago

Dear Darren, I like the slickness of your script, but unfortunetaly I don't get an output in my Windows system. I had to change the JSLint.sublime-build the following way:

// "/usr/local/bin/node", "C:\Program Files\nodejs\node.exe", // "/usr/local/bin/jslint", "C:\Users\me\AppData\Roaming\npm\node_modules\jslint\bin\jslint.js",

This is how it looks on Windows sigh. Node is installed in program files, jslint is placed in my appdata. If I run the command line with that it looks like this:

"c:\Program Files\nodejs\node.exe" C:\Users\me\AppData\Roaming\npm\node_modules\jslint\bin\jslint.js --sloppy --indent 2 --node --nomen --vars --plusplus --stupid C:\temp\myfile.js

1 'dojo' was used before it was defined.

dojo.require("dijit.layout.BorderContainer"); // Line 1, Pos 1

[... a lot of other linting hints ...]

If I run it from Sublime, the console says:

C:\temp\myfile.js [Finished in 0.2s with exit code 1]

If I change the node and jslint path to something not valid, I'll get an expected error on the console (System cannot find the file...).

So I assume the customizations in the JSLint.sublime-build file are OK and the error is related to the internal mechanism to pipe the output to the console. Dunno, if it's maybe the regexp for the file? or the content? Can you help?

Best regards and thanks in advance, Axel

darrenderidder commented 11 years ago

Probably Windows' backwards slashes in file path not matching the forward slashes in the file_regex. Windows... ouch.

atesgoral commented 11 years ago

I was able to get around the Windows path problems by invoking "jslint" which should already be in your PATH if you've installed it with npm. You also need to set the shell option to true so that the builder doesn't look for an actual executable:

    "windows": {
        "cmd": [
            "jslint",
            "$file"
        ],
        "shell": true
    },
atesgoral commented 11 years ago

But I still didn't get any success even after tampering with the regexes. Tried matching the file name part after adding support for backslashes, tried matching the full path. No success...

AxxL commented 11 years ago

Thanks for your responses. I don't know what you want to regex with this file and what it is for? The Sublime docu mentions this but I didn't find a proper explanation why this param has to be filled. Also I didn't look at the syntax. Is it comparable to this syntax: http://regexr.com?328eb Or does Sublime use another regexp syntax? This tool in the link doesn't highlight anything.

A typical command looks like this (from the console log, so the Running is the output of the console):

Running node C:/Users/me/AppData/Roaming/npm/node_modules/jslint/bin/jslint.js --sloppy --indent 2 --node --nomen --vars --plusplus --stupid C:\temp\tester.js

Where the node command is in my path and it has the path to the jslint.js as an argument.

If Sublime is logging this command correctly and getting the exit-code right then I guess the error is in the regexp or in a missing cmd parameter - like the "shell" param as atesgoral mentioned.

atesgoral commented 11 years ago

Using jslint.cmd on Windows, the output is something like:

jslint --indent 4 --nomen --vars --plusplus C:\@Sandbox\test.js

C:\@Sandbox\test.js
 #1 Expected 'a' at column 13, not column 8.
    a: 1, // Line 5, Pos 8
 #2 Expected 'b' at column 13, not column 8.
    b: 2 // Line 6, Pos 8
 #3 Expected '}' at column 9, not column 5.
    }; // Line 7, Pos 5

Testing in the browser console, you can use the following regex to capture the file name part:

new RegExp("^.+\\\\(.+)$", "m")
    .exec(""
        + "C:\\@Sandbox\\test.js\n"
        + " #1 Expected 'a' at column 13, not column 8.\n"
        + "    a: 1, // Line 5, Pos 8\n"
        + " #2 Expected 'b' at column 13, not column 8.\n"
        + "    b: 2 // Line 6, Pos 8\n"
        + " #3 Expected '}' at column 9, not column 5.\n"
        + "    }; // Line 7, Pos 5\n"
    ); // ["C:\@Sandbox\test.js", "test.js"]

And the following to capture the line and pos:

new RegExp("^\\s.+\\/\\/ Line (\\d+), Pos (\\d+)", "m")
    .exec(""
        + "C:\\@Sandbox\\test.js\n"
        + " #1 Expected 'a' at column 13, not column 8.\n"
        + "    a: 1, // Line 5, Pos 8\n"
        + " #2 Expected 'b' at column 13, not column 8.\n"
        + "    b: 2 // Line 6, Pos 8\n"
        + " #3 Expected '}' at column 9, not column 5.\n"
        + "    }; // Line 7, Pos 5\n"
    ); // ["    a: 1, // Line 5, Pos 8", "5", "8"]
smhg commented 11 years ago

I think the issue is the same as https://github.com/uipoet/sublime-jshint/issues/23 (near the end) where output from node is not correctly "caught" on Windows in Packages/Default/exec.py.

atesgoral commented 11 years ago

Thinking that the issue could be around consuming stdout versus stderr, I also tried redirecting stderr to stdout with 1<&2, to no avail... I guess I'll have to sit tight and wait for a fix.

smhg commented 11 years ago

I could get it to work by changing the way node-jslint (ried/node-jslint) exits.

Based on your original question, there are 2 "problems" actually. 1) Having a working default JSLint.sublime-build on Windows. That is fairly easy if you structure the file like this:

{
    // ... current content ...
    "windows": {
        "cmd": [
            "jslint.cmd",
            // ... options ...
        ]
    }
}

2) Exiting in a different way in jslint.js. Currently line 63 in jslint.js makes node exit with process.exit(). This does not seem to flush buffers (at least not on Windows in combination with subprocess in Python in exec.py of Sublime Text). This is all far beyond my knowledge of any of these technologies, but if you remove the process.exit() line, it works as you would expect.

AxxL commented 11 years ago

Thanks samhauglustaine. I changed the jslint.js, saved it as my_jslint.js (i guess the orginal file is managed by npm) and changed the lines (line 63):

// process.exit(ok ? 0 : 1);
process.on('exit', function () {
  var exitcode = ok ? 0 : 1;
  console.log('my_jslint.js exit code: ' + exitcode);
});

I added the process.on(exit) function to see, if the code exits. I guess it does and I get the exit code logged to the output console.

I don't know if Darrens package by can get around this issue. It doesn't seem that there is a switch for the command to handle the output of node.js and jslint.js.

smhg commented 11 years ago

I think we can summarize it as one of these 2 scenario's:

But both are more or less outside the scope of Sublime-JSLint. The only solution to handle this inside this component would be to not rely on exec.py and the Sublime command format, but to create a separate Python script. That might however be a bit overkill for a Windows-only issue, no?

darrenderidder commented 11 years ago

New R2.0 branch uses a different approach to running jslint. Looking for some Windows users to beta test it. It's available as an update via the PackageControl plugin.

darrenderidder commented 11 years ago

Tested on Ubuntu, Windows 7 and Mac OS. Merged R2.0 changes to main branch. Would appreciate comments if it works for you or if you have any problems.

Erid commented 11 years ago

It's working great for me on Windows 7.

AxxL commented 11 years ago

Sorry for my late response. It works well. Thank you for your code!

darrenderidder commented 11 years ago

Glad to hear it!

rufwork commented 11 years ago

I'm getting the error now on Windows 7. Flipped the switch for Sublime Text 2 to run as Administrator, and started getting a dialog with "Error trying to parse build system: No data in ~/.config/sublime-text-2/Packages/JSLint/JSLint.sublime-build:1:1". Later, after an uninstall/reinstall and turning the administrator option off for the shortcut, this somehow shifted to...

C:\temp\temp.js [Finished in 0.2s with exit code 1]

... no matter what the file.

Even npm'd jslint -- not sure what to do to debug more.

(Apologies; had pulled the error text from a google hit when I was searching for the error earlier instead of typing from scratch. Am using JSLint, with an "L".)

listart commented 11 years ago

Only the first stdout message makes it to the build results, so we should buffer all the console.logs and print them at once in report.js

var buffer = '';
var log = function(){
  var args = Array.prototype.slice.call(arguments);
  buffer += args.join('');
  buffer += '\n';
}
...
console.log(buffer);
darrenderidder commented 11 years ago

Ok. A dependency on node-jslint was re-introduced and I'm afraid that may be causing some issues. Also please say what OS you're running this on.

listart commented 11 years ago

on Win7 x64

darrenderidder commented 11 years ago

@rufwork please select Tools -> Build System -> Automatic and retry. JSLint.sublime-build was removed in the last patch and this should reset it.

@listart this may be a separate issue, I need to get onto a Windows 7 machine to test it.

hrundik commented 11 years ago

I also have the same problem as @rufwork. I'm on Win7 x64. Tools -> Build System -> Automatic didn't work for me. How can I help fixing the problem?

darrenderidder commented 11 years ago

Can you please try selecting another build system from the menu temporarily, then switching back to Auto?

hrundik commented 11 years ago

I switched to another build system, invoked JSLint though saving the file, and then changed back to Auto. No result.

darrenderidder commented 11 years ago

Alright sorry for the mess. I've rolled the last major pull request into an R3.0 branch for cleanup and restored the original from the R2.0 branch. Let me know if that resolves the issue for Windows users.

janraasch commented 11 years ago

Monkey patching node-jslint/lib/reporter.js as @listart suggested seems like an okay idea, if it solves the problem.

This probably also means that Windows users will not be able to lint though folders with more than one file to lint. (At least, they will only get to see the output for one file.) So we should hide the JSLint: Containing Folder and JSLint Folder commands for Windows.

Luckily, I don't currently have access to a Windows machine to test this.

As a last resort we could also serve different versions of the plugin using a custom packages.json. It would be quite nice to have a better versioning than only the latest state of the master branch on github anyway. This way switching back to an older (working) version is as easy as downloading a zip file.

rufwork commented 11 years ago

I un- and re-installed (second ago), and am working fine now. I'm assuming that's an updated pull and what was expected. Thanks!

darrenderidder commented 11 years ago

Node-jslint a great command line utility, but its been difficult to integrate with Sublime Text on Windows. This issue was originally due to node-jslint, so it was fixed by using Crockford's jslint directly. This is similar to how the Sublime Linter and JSHint plugins work, and seems to be the best way to ensure consistent cross-platform behaviour. I moved the new features into R3.0 where we can look for a cross-platform solution before merging back to main. Regarding versions, I agree that it can be improved.