mojolicious / mojo

:sparkles: Mojolicious - Perl real-time web framework
https://mojolicious.org
Artistic License 2.0
2.66k stars 576 forks source link

unwatch feature: morbo should not monitor files matching a user-specified list of regexes. #1504

Closed hadjiprocopis closed 3 years ago

hadjiprocopis commented 4 years ago

Steps to reproduce the behavior

This is a feature request accompanied with a patch file implementing the request. The feature is "unwatch" and provides script/morbo with an added command line switch "--unwatch regex [--unwartch regex2]". A monitored file is not monitored any more if any of the provided regex matches against its name. This saves morbo restarting every time I edit a file because my editor (uEmacs) creates a ".lock" file next to the filename and morbo restarts!

Expected behavior

Ideally, with this feature morbo should not restart if files matching the unwatch list of regexes are created and/or modified.

Actual behavior

script/morbo restarts every time I open for editing a file in lib directory (e.g. lib/MyApp/Controller/Main.pm) before I even change that file or save the changes to disk. Because my editor creates a ".lock" temporary file each time I open that file with the editor.

I have attached a patch file with my modifications to: script/morbo Mojo/File.pm Mojo/Server/Morbo/Backend.pm Mojo/Server/Morbo/Backend/Poll.pm t/mojo/morbo.t

Which implement the suggested feature, amend the documentation appropriately and add a new test section in t/mojo/morbo.t for testing it.

PatchFile.txt

kraih commented 4 years ago

Attached patches are pretty inconvenient, so i doubt that will get much attention.

Grinnz commented 4 years ago

See https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/proposing-changes-to-your-work-with-pull-requests

hadjiprocopis commented 4 years ago

ok, I will try that some time next week.

On 03/05/2020 20:15, Dan Book wrote:

See https://help.github.com/en/github/collaborating-with-issues-and-pull-requests/proposing-changes-to-your-work-with-pull-requests

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mojolicious/mojo/issues/1504#issuecomment-623146743, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5PLOQECDCWSNMXERSV2Z3RPWRE3ANCNFSM4MYDQPLA.

jhthorsen commented 4 years ago

I think it would be better if morbo would read .gitignore or another similar file. Staring morbo with --unwatch seems like a lot of work.

When that is said, I've configured vim not create lock/temp files, so I don't see myself implementing this.

kraih commented 4 years ago

Didn't Morbo once just ignore all dotfiles/dirs?

hadjiprocopis commented 4 years ago

It does try to ignore dotfiles (aka hidden) but it can't pass it on to File::Find because the latter no longer supports it, if it ever did

Mojo/File.pm:

sub list_tree { my ($self, $options) = (shift, shift // {});

This may break in the future, but is worth it for performance

local $File::Find::skip_pattern = qr/^./ unless $options->{hidden}; ...

btw the .lock file uEmacs creates is not a dotfile. '' is the name of the actual file being edited. (and uEmacs is a 1990's editor!)

In my previous email I have shown a possible remedy - the patch points at the 4 or 5 places changes need to be made and how to test these - admittedly, more test is proabably needed. But either plain or a more generic feature via a .gitignore (as it has been suggested by Jan Henning Thorsen) it will be a nice feature imo. I will wait for the consensus before I make a push request or whatever git calls it.

On 04/05/2020 22:52, Sebastian Riedel wrote:

Didn't Morbo once just ignore all dotfiles/dirs?

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mojolicious/mojo/issues/1504#issuecomment-623670731, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB5PLOSN2YLG6B54RPJAPTLRP4MGNANCNFSM4MYDQPLA.

kiwiroy commented 4 years ago

With MOJO_MORBO_BACKEND set to some new class/role, say Poll::SiftEphemeral seems more simple implementation path.

package Mojo::Server::Morbo::Backend::Poll::SiftEphemeral {
  use Mojo::Base qw(Mojo::Server::Morbo::Backend::Poll);
  use Class::Method::Modifiers;
  use Text::Gitignore;
  has git_ignored       => sub { ... };
  has skip_extensions   => sub { split /,/ => $ENV{MOJO_MORBO_UNWATCH} || '' };
  around modified_files => sub { ... };
}
stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

kraih commented 3 years ago

I'm not opposed to adding a feature to resolve this, but unfortunately this issue never really evolved into a discussion about how that feature should look like. Please open a new issue or use the new GitHub discussions feature if you're interested in changing that.