Moved watch onto dynamic file extensions

[twolfson]

I am developing a test framework on top of mocha which uses .json and .yaml files as part of its usage. To make --watch work with the relevant files, I have expanded utils.files to watch all file extensions we have registered.

Additionally, this patches #804.

If you are interested in the framework, it is called doubleshot.

[fredericosilva]

I was about to add json to the regex (/.(js|coffee|json)$/). This is a better solution.

+1

[mal]

This won't escape extensions such as .coffee.md, the second . would become a wildcard. This may or may not be an issue worthy of consideration given the very small chance of any detriment occurring. Just thought I'd point it out.

[twolfson]

@mal Can you elaborate further? require.extensions looks like:

{
  '.js': function () { /* Process JS */ },
  '.json': function () { /* Process JSON */ },
  '.node': function () { /* Process node binary */ }
}

so the RegExp will look like RegExp("(.js|.json|.node)$"). When coffee-script is used, we gain .coffee and .litcoffee; RegExp("(.js|.json|.node|.coffee|.litcoffee)$"). I don't see the case where that would match .coffee.md.

However, now that you mention it, I am not escaping the . of extensions which would allow for matching files like .ejs. I will patch that now.

[twolfson]

...and patched.

[mal]

Ahh my mistake, I assumed that .coffee.md was included by coffee after seeing #924. Solution looks good though. 👍

[twolfson]

@visionmedia What is it going to take to get this PR merged?

[tj]

I'd move these stuff out of exports.files and do var extRegexp = requireExtensionsRegexp() or similar, not a huge deal even a rebase would be sweet and I'll merge

[twolfson]

Alright, cool. Unfortunately, I am swamped this weekend but I will get to it by Wednesday.

[twolfson]

@visionmedia To prevent further delays, I am going with the shortest route. I have rebased and squashed the PR.

[travisjeffery]

Not sure this implementation is a good idea, since require.extensions is deprecated. See http://nodejs.org/api/globals.html#globals_require_extensions, nodejs/node-v0.x-archive@7bd8a5a, and nodejs/node-v0.x-archive#5430 (comment).

[twolfson]

Ah, I had no idea. In that case, it might be wise to use a default + feature detection + optional CLI based overrides (the overriding was my purpose for the original PR).

Default + feature detection is easy:

var exts = require.extensions ? require.extensions : ['.js', '.json', '.node', '.coffee', '.litcoffee'];
// If you want to be nice, you would add in .yml, yaml for js-yaml users.

For CLI based overrides, you would probably need to start using an instance level this and read in parameters inside of bin.

[travisjeffery]

@twolfson deprecation in the sense that we probably shouldn't use this due to it being unreliable, rather than the api missing. see these messages (and discussion): https://groups.google.com/d/msg/nodejs/QsOEWvptQpA/LKqTL4hLK_4J and https://groups.google.com/d/msg/nodejs/QsOEWvptQpA/ZVMSu8Sq66AJ

i think we should go with default + optional cli overrides only and not use require.extensions.

[twolfson]

What I was suggesting is feature detection, the API is not going to change, it will only be removed.

I no longer have time to dedicate to this pull request. Please choose to use it, fork it, or scrap it. Thank you for your consideration.

[twolfson]

This PR is 7 months old. Please let me know what the decision is (for merge, fork, or deletion).

[tj]

if it's deprecated then yeah screw that, we'll have to add flags or something, though I just wouldn't use watch personally, just complicates things, watch make test works fine