Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Restuta opened this issue Aug 4, 2015 · 18 comments · Fixed by #370
Closed

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

Restuta opened this issue Aug 4, 2015 · 18 comments · Fixed by #370

Comments

@Restuta
Copy link

Restuta commented Aug 4, 2015

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.

@alallier
Copy link
Owner

alallier commented Feb 3, 2016

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

@gersongoulart
Copy link

gersongoulart commented Jul 31, 2016

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

@alallier
Copy link
Owner

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

@gersongoulart
Copy link

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

@jprichardson
Copy link
Collaborator

jprichardson commented Aug 5, 2016

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?

@gersongoulart
Copy link

@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... :)

@alallier
Copy link
Owner

alallier commented Aug 9, 2016

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

@jprichardson
Copy link
Collaborator

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

@Restuta
Copy link
Author

Restuta commented Aug 9, 2016

@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.

@alallier
Copy link
Owner

alallier commented Aug 12, 2016

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']

@jprichardson
Copy link
Collaborator

jprichardson commented Aug 12, 2016

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.

@Restuta
Copy link
Author

Restuta commented Aug 12, 2016

@jprichardson

it should?

yes, of course

@alallier
Copy link
Owner

alallier commented Aug 29, 2016

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 Extend watch options by exposing arguments from supervisor #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

@jprichardson
Copy link
Collaborator

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

@alallier
Copy link
Owner

Great 3 was also my choice!

@gersongoulart
Copy link

Yeah @alallier I think #3 is brilliant!

@alallier alallier added this to the v2.0.0 milestone Apr 27, 2017
@alallier alallier removed this from the v2.0.0 milestone Jun 25, 2017
@FireController1847
Copy link

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.

@alallier
Copy link
Owner

alallier commented Aug 14, 2024

Reload now supports this.

Watching subdirectories was handled by #367 and ignoring files was added in #370

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants