Modification to fs core module

[jasonknight]

Hi,

For some reason, fs.existsSync wasn't working for me, I didn't find the problem, maybe a misuse on my part, but when I looked at the code, I noticed that it was using a call to fs.stat, and returning true/false based on whether or not this threw an error because the path didn't exist.

Changing from stat to lstatSync made the function work again, but it seemed to me unecessarily expensive to base the function off of throwing an error so I added a simple exists method in c++ that returns true/false based on a call to open. The existing async call is left alone, as I only needed the sync version and was a bit unsure how to implement the async version, but I will look into that. I wrote a little test in the test directory to make sure it works, and have been using it locally so far.

If you'd like to pull it, or make some suggestions how I could better do it the Node way, i'd be happy to refactor it.

Cheers,
/Jason Knight

[isaacs]

There are a few problems with this:

1. "can be opened" !== "exists". If you want to know if the file can be opened, then open it. fs.exists is a wart in the API, and would be removed if we could get away with it. As it stands, there's a good chance it'll be deprecated in 0.9. We are not going to change the semantics of it.
2. There's a backup copy of lib/fs.js as lib/fs.js~ in the commit.
3. Test is not in a folder, so it won't be called by make test. (There are already tests that cover fs.exists{,Sync} in test/simple/.)

I appreciate the effort, but I hope you understand that I cannot merge this in. Thanks.

[jasonknight]

Hi,

Thanks for your response, and I appreciate your candor and pointers and taking the time to respond!

A few notes: calling fopen with "r" returns null if the file does not exist, so it does seem to work as a quick exists call, at least on my machine. See: http://www.cplusplus.com/reference/clibrary/cstdio/fopen/, when using the "r" option the file must exist.

This is just my personal opinion, FWIW, fs.exists is a wart on the API, but not the concept of checking for the existence of a file. Doing what you suggest is the functional equivalent of the current way of doing things, i.e. most people would just end up writing a fileExists function which tries to open the file and catches the error.

Either way, it's such a small thing really that it hardly matters, I only found it odd that there was no exists function, and being very new, I tried to do something small. I do hope in the end you change your mind about deprecating it.

Good luck, and keep up the great work!
/Jason Knight

[isaacs]

@jasonknight open(file, 'r') does not test that the file exists, though. It tests that it exists and is a readable file, which is a very different thing. Of course, stat can fail for permission reasons as well, and testing for file existence is usually an antipattern; you should just try to do whatever it is you want to do, and get an error if it fails.

[jasonknight]

@isaacs Thanks for your feedback again, you have given me a lot of food for thought, and I looked into it a little further. I really do appreciate you taking the time to answer. I hope you have a great summer!