reload icon indicating copy to clipboard operation
reload copied to clipboard

Add possibility to watch files in subdirectories with optional "exclude" paths

Open Restuta opened this issue 10 years ago • 17 comments

Currently "reload" command only watches files that are present in current directory which is very-limiting. It would be nice if by default it would watch all the files in all sub-directories.

Restuta avatar Aug 04 '15 04:08 Restuta

It watches all files and sub-directories from where you run the reload command

alallier avatar Feb 03 '16 19:02 alallier

It would be really nice to be able to configure this behaviour. Perhaps just exposing supervisor's --watch would suffice?!

gersongoulart avatar Jul 31 '16 01:07 gersongoulart

Yeah that should be easy enough to do and also be useful. Your thoughts on this @jprichardson?

alallier avatar Jul 31 '16 03:07 alallier

#48 should be a valid implementation. What do you guys think?

gersongoulart avatar Jul 31 '16 20:07 gersongoulart

I'm not a big fan of overcomplicating software. What's the use case @gersongoulart @Restuta? Why not just run reload at the root of the directory that you're working in?

jprichardson avatar Aug 05 '16 13:08 jprichardson

@jprichardson I'm sorry this sounded like overcomplicating. The goal would be exactly to avoid running reload every time absolutely any file at all from the root of the project downwards changes as that would mean a reload every time I update README.md, install a new package, change one of the .NET backend files, add a new image to the solution, have the build break and write a npm.log file, etc, etc, etc. (the list goes on indefinitely for unintended reloads). The proposed solution would be to just allow some more of the parameters already supported by the underlying dependency to pass through your abstraction (which I personally think adds zero complexity to reload itself). But that's just my two cents... :)

gersongoulart avatar Aug 08 '16 04:08 gersongoulart

What if we just exposed supervisor's ignore option? Would that satisfy you both?

alallier avatar Aug 09 '16 13:08 alallier

@alallier you can decide to do whatever you like here.

jprichardson avatar Aug 09 '16 13:08 jprichardson

@jprichardson what do you mean? I am running it at the root and if I have CSS files in /CSS directory it wouldn't reload. I guess smart defaults for web are the key, like reload on every change in every *.css, *.js, *.html files and images for all nested dirs.

Restuta avatar Aug 09 '16 19:08 Restuta

So I've gave this some thought and think I have a solution that will provide what @Restuta and @gersongoulart want but also keep it simple like @jprichardson wants. We should just abstract the supervisor arguments, provide an object or array argument (some optional argument) that can override our defaults but handle any supervisor flag. Then we can just point config options to supervisor's README and also the program would be dynamic to any change supervisor may make that either changes a flag or adds a new one, as reload would just take the argument of supervisor options.

Also this would add one new flag to reload and remove one, resulting in a wash.

Something like:

reload -b --supervisor-configs ['-s', '-pid', '--debug', '-w js,html']

or

reload -b -c ['-s', '-pid', 'debug', '-w js,html']

alallier avatar Aug 12 '16 11:08 alallier

what do you mean? I am running it at the root and if I have CSS files in /CSS directory it wouldn't reload.

@Restuta it should?

We should just abstract the supervisor arguments, provide an object or array argument (some optional argument) that can

@alallier something to think about if you do that, is that you tether reload to supervisor then. I think that's fine, but a lot has changed in this ecosystem that I'm not so sure supervisor is the right choice longterm. onchange is pretty cool btw. However, as a short-term solution, maybe what you're proposing is fine if that caveat was there: ("reload team reserves right to drop supervisor in a future release") or something like that so that people know that using supervisor options is risky. Your decision any way you go.

jprichardson avatar Aug 12 '16 14:08 jprichardson

@jprichardson

it should?

yes, of course

Restuta avatar Aug 12 '16 16:08 Restuta

So after some discussions here I see three possible different paths reload could take (command line use only)

  1. Reload doesn't support fine controls over the file watchers under the hood, so that it could easily be swapped out for another watcher at any time and not produce breaking changes because of flags changing. Changes: None
  2. Reload supports the flags proposed by #48 or something similar where we support specific flags for supervisor configuration. This option can be looked at as marrying ourselves to supervisor, which technically would true but in a way we already married to supervisor as it already runs under the hood now. This will bloat reload's flags for command line use and could possibly make it more confusing but it provides more control than option 1 Changes: New flags to support many different numerous supervisor flags.
  3. Reload supports one general flag that accepts all possible options for the given under the hood file watcher (right now that is supervisor, so the flag could be -o (options) which can take any supervisor flag and pass it to supervisor while using reload). This abstracts all control to supervisor through one flag, allowing the user to use any valid supervisor flag. This allows us to link to the file watchers documentation directly to reload's. The only breaking changes that could occur is if we were to change the file watcher under the hood. This would change the options passed to the -o flag. If we ever changed the file watcher under the hood we would link the new file watchers config option in reload's README and if people downloaded the new version they would have to learn the new file watchers flags. Changes: This option adds one flag -o which would live in reload forever regradless of the file watcher, allowing the -o flag to always take the current file watchers options.

*Note these none of these changes would affect how reload is used in an express app, as it's up to user to configure their own file watcher and options in their express application.

@jprichardson I would like you to make the decision on this though, as all of these options lead to different implemented/non-implemented outcomes

alallier avatar Aug 29 '16 18:08 alallier

@alallier 3 sounds great. Thanks for writing this out.

jprichardson avatar Aug 29 '16 19:08 jprichardson

Great 3 was also my choice!

alallier avatar Aug 29 '16 19:08 alallier

Yeah @alallier I think #3 is brilliant!

gersongoulart avatar Nov 08 '16 02:11 gersongoulart

So do we have any status on number 3? It sounds like it could improve this module 10 fold, and could be extremely useful for me (for example, the --ignore options found in node supervisor). I mean, considering it's two years after the last comment on this issue, it'd be awesome if we could get this implemented.

FireController1847 avatar Mar 05 '18 04:03 FireController1847