@nfischer: Ready for Review.

Tried this out. I saw a slight perf win, which is awesome! I just have one comment above about correctness of error messages. If that gets fixed, I can merge

Also, just tried this, and it has the output Error: cd: no such file or directory: file.txt if I set('-e'), which is also not the proper output.

@nfischer: What did you run exactly?

@ariporad

Here's what I ran:

$ cat script1.js
#!/usr/bin/env node
require('shelljs/global');

touch('file.txt');
set('-e');
cd('file.txt');

$ node script1.js
/home/nate/PR_367/node_modules/shelljs/src/common.js:248
        throw e;
        ^

Error: cd: no such file or directory: file.txt
    at Object.error (/home/nate/PR_367/node_modules/shelljs/src/common.js:42:11)
    at _cd (/home/nate/PR_367/node_modules/shelljs/src/cd.js:29:12)
    ... (there's more lines of the stack trace)

$ cat script2.js
#!/usr/bin/env node
require('shelljs/global');

touch('file.txt');
config.silent = true;
cd('.');
echo(error());
cd('fakedirectoryname');
echo(error());
cd('file.txt');
echo(error());

$ node script2.js
null
cd: no such file or directory: fakedirectoryname
cd: not a directory: file.txt
cd: no such file or directory: file.txt

@ariporad pinging this to see if the issue with error messages has been fixed yet.

@nfischer: I'm not experiencing the behavior that you're describing. I tried both your scripts, they work as expected for me. (The tests are also passing).

I'll try again today. The issue is with stderr output, not the value of error() (only error() is checked, not the actual output, in the tests)

@ariporad I just updated the code a bit to improve performance a bit more. We should rename this PR to "only run stat never 👍".

It turns out you were right. The incorrect error message isn't due to your code, it's due to a bug that was introduced in #357. I'm currently trying to work out a fix for this (it shouldn't be too hard). Expect to see the fix sometime tonight.

Here's some perf stats I saw on my machine. This is the script I ran (named cd.js):

require('shelljs/global');
for (var k=0; k<20000; k++) {
  cd('dir/'); // this is a directory that exists
  cd('..');
}

Here's the perf I saw:

# for the original implementation
$ time -p node cd.js
real 2.61
user 2.50
sys 0.13

# for your revision
$ time -p node cd.js
real 2.11
user 1.98
sys 0.15

# for my revision
$ time -p node cd.js
real 1.96
user 1.90
sys 0.07

I also tested things with cd'ing into a file and into a non-existent directory. I saw perf wins for my revision there as well, but this is mainly designed to optimize for the common case. I don't think performance is as big of a concern for when the command errors-out (and with config.fatal, it isn't a concern at all).

Merge at will.

Update: I thought a bit more about the error message, and I decided that it's probably easier to fix the issue here than elsewhere. So I updated the PR with the fix. This will now only output the proper error message.

Perf stats are still the same.

@nfischer: LGTM, but one thing just occurred to me: should we eliminate tey/catch statements? Might be faster (http://git.io/fast-v8)

@ariporad I think try-catch may be necessary here. I know it adds some performance overhead, but statSync() throws an error if the file doesn't exist (and the alternative function is deprecated). I think we should just stick with this. We'll never beat Bash's cd (it's a shell builtin written in C), but this is probably good enough.

Ok