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

Modify watch_filesystem to take in a list of folders to watch #3377

Closed
wants to merge 3 commits into from

Conversation

leakec
Copy link

@leakec leakec commented May 25, 2024

This pull request addresses issue #3325.

leake and others added 2 commits May 4, 2024 11:41
defines the folders we want to watch and whether to recurse through them
or not. An empty map will result in the /host folder being watched with
recurse=true.
@imsnif
Copy link
Member

imsnif commented Oct 24, 2024

Hey @leakec - I first wanted to apologize for taking so embarrassingly long to get to this.

Otherwise - I'm a little concerned about the current watch_filesystem as I find it quite buggy on certain platforms and I'm not sure I want to keep giving plugins so much power over the application.

I guess you want this for multitask... Do you think it can be solved in a different way? Perhaps a UI that opens the user's $EDITOR and once it's closed reads the contents? There are new APIs now in the upcoming version that should accommodate this. I can point them out after the release if you like.

@leakec
Copy link
Author

leakec commented Oct 24, 2024

@imsnif No need to apologize at all. I know you are super busy maintaining this awesome tool we all love and enjoy!

You're correct in that this PR was targeting some improvements I was hoping to make on multitask. I agree that this could probably be solved (and maybe better to solve) by not using watch_filesystem at all.

One option would be to just have zellij read from a user-specified file (or create one on /tmp if none are specified) and have the user use a keybind to signal to the plugin that they are ready to run their multitasks; rather than the current signaling method which is once the file is saved/closed. One reason I like a file rather than a temporary editor pane is it allows users to come back and re-use that file across multiple sessions, multiple days, etc. I find myself often creating a multitask file and then re-using it for weeks or months, so it would be nice to keep this option.

Sounds good r.e. the APIs in the upcoming version. I'll read the updates when it comes out and let you know if I have any questions. I'm happy to close this PR rather than merge if you think that's best.

@imsnif
Copy link
Member

imsnif commented Oct 24, 2024

The new version includes APIs that will allow you to open a file in an $EDITOR pane and then get notified when it is closed. So my suggestion would be something like:

  1. Open the multitask file
  2. Change it if you like
  3. Save and close
  4. The plugin will be notified, read the file and do its thing

It will also be much easier to do the actual multitasking because there are also APIs for doing the same thing with command panes. Up to you though, of course!

Otherwise, let's leave this PR open for now. If you decide you want to go this direction after all, we can re-examine it.

@leakec
Copy link
Author

leakec commented Oct 26, 2024

@imsnif This sounds great! I think it might be nice to add a second way in addition to closing the editor pane (maybe an optional, opt-in keybind or something) so users can run the multitask with the editor window still open. This is really nice if the tasks run fast and you want to run them, make a change, run them again, etc.

Regardless, it sounds like all of this will fit really nicely within the new API :)

I like this plan a lot better than the PR here, since then we don't have to worry about permission for anything except the multitask file and running the commands, so we don't have to even worry about watching any part of the file sytem. Therefore, I'm going to close the PR. Thanks for all the suggestions and work on the upcoming, new API options!

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

Successfully merging this pull request may close these issues.

2 participants