Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fix issue #63, accepts input via stdin #64

Closed
wants to merge 1 commit into from

2 participants

@hallettj

To read files from stdin, give a dash (-) in place of a file path. For example:

cat some_file.js | jshint -

This matches the conventions of established Unix tools like grep.

The implementation in this changeset is platform dependent, since the only way that I know of to read from stdin synchronously is to call fs.readFileSync('/dev/stdin', 'utf-8'). I also put together an async refactor that might allow reading from stdin to work on platforms that do not have a /dev/stdin file (i.e. Windows). But that would be a much bigger change.

@hallettj hallettj Fix issue #63, accepts input via stdin
To read files from stdin, give a dash (-) in place of a file path.  For example:

    cat some_file.js | jshint -
3fe4a80
@brentlintner
Collaborator

This is great (and tests!). It definitely would be ideal (like you mentioned) to be platform in dependant though... However, couldn't you have just used process.stdin (unless that's what you meant by async refactor)?

@hallettj hallettj referenced this pull request from a commit in hallettj/node-jshint
@hallettj hallettj WIP #64: refactor to read input asynchronously 587560d
@hallettj

Yeah, as far as I know as a Readable Stream process.stdin has no blocking read method. So I think that if you want to use the standard stdin API, then the only option is to read input asynchronously.

I got the idea of using /dev/stdin from this thread: http://stackoverflow.com/questions/3430939/node-js-readsync-from-stdin/5794483#comment10542253_5794483

The refactor that I worked on is at hallettj@587560d. The code is working - though it could probably be simplified a bit. I got partway through updating the tests to match the new async behavior when I decided to look for a simpler solution.

@brentlintner
Collaborator

Ah, thanks for the information (a bit more complicated, indeed). I have no objections to the /dev/stdin way... although it would be super awesome to make this platform independent (right away)... So, If you wouldn't mind finishing and submitting the async implementation, that would be much appreciated. However, If you really want to get this in, I can merge this, do a publish, and create a techdebt Issue to make it work on windows (via process.stdin). :-s

PS Would you mind adding some info about this to the README? If not I can do that.

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 15, 2011
  1. @hallettj

    Fix issue #63, accepts input via stdin

    hallettj authored
    To read files from stdin, give a dash (-) in place of a file path.  For example:
    
        cat some_file.js | jshint -
This page is out of date. Refresh to see the latest.
View
8 lib/cli.js
@@ -84,15 +84,19 @@ module.exports = {
}());
}
- targets = typeof targets === "string" ? null : targets.slice(1);
+ targets = typeof targets === "string" ? [] : targets.slice(1);
+ // Accept a dash as a file path to read from stdin.
+ if (options['-']) {
+ targets.push('/dev/stdin');
+ }
if (options["--version"]) {
_version();
return;
}
- if (!targets || options["--help"]) {
+ if (targets.length < 1 || options["--help"]) {
_help();
return;
}
View
2  lib/hint.js
@@ -78,7 +78,7 @@ function _collect(filePath, files, ignore) {
fs.readdirSync(filePath).forEach(function (item) {
_collect(path.join(filePath, item), files, ignore);
});
- } else if (filePath.match(/\.js$/)) {
+ } else if (filePath.match(/\.js$/) || filePath.match(/stdin/)) {
files.push(filePath);
}
}
View
6 test/unit/cli.js
@@ -182,4 +182,10 @@ describe("cli", function () {
expect(process.stdout.on.argsForCall[0][0]).toBe("drain");
expect(process.exit).toHaveBeenCalledWith(1);
});
+
+ it("reads input from stdin if `-` is given as a file path", function () {
+ cli.interpret(["node", "hint", "-"]);
+
+ expect(hint.hint.mostRecentCall.args[0]).toContain("/dev/stdin");
+ });
});
View
8 test/unit/hint.js
@@ -168,6 +168,14 @@ describe("hint", function () {
expect(fs.readFileSync.argsForCall[0][0]).toBe("dir/foo/test.js");
});
+ it("reads input from stdin", function() {
+ spyOn(fs, "readFileSync").andReturn("data");
+
+ hint.hint(["/dev/stdin"]);
+
+ expect(fs.readFileSync).toHaveBeenCalledWith("/dev/stdin", "utf-8");
+ });
+
// TODO: handles jshint errors (will tighten custom reporter assertions)
// TODO: handles file open error
// TODO: handling of JSHINT.data()
Something went wrong with that request. Please try again.