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

Switch Inotify to irmin-watcher #65

Merged
merged 1 commit into from
Sep 12, 2024

Conversation

patricoferris
Copy link
Contributor

Thanks for slips @panglesd !

Irmin-watcher is a portable, filesystem notification library. On linux it uses inotify and on macOS it uses fsevents. It has a portable, if slow, polling mode too.

This makes slipshow installable on macOS \o/

(( Sorry about all of the formatting that's been sucked into this PR :S -- feel free to cherry pick just the meaninful changes ))

@panglesd
Copy link
Owner

panglesd commented Sep 9, 2024

Thanks a lot, this is great!

I had the plan to create a multiplatform filesystem watcher but independent of irmin (irmin-watcher might not have irmin as a dependency, but if I understand correctly irmin influences irmin-watcher's API).
I would be based on inotify, fsevents but also kqueue (for Mac but also for BSDs). I also planned to add some higher level API and some docs: the topic of file watching is more difficult than I though, with multiplatform, atomic writes, emacs, vim and vscode all saving in different ways...
I definitely made progress in this direction, but it'll still require some time.

Having your PR ready now is really nice for all Mac users! Thanks!

Copy link
Owner

@panglesd panglesd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm excited to cut a release with your changes! This will fix #56

I have some questions, mostly on changes unrelated to the core of this PR.

Thanks again!

Comment on lines -54 to +57
[%blob "compiler/src/bin/native/client/client.bc.js"]
[%blob "client/client.bc.js"]
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm it seems to work in both situation, is it some blob ppx magic? Anyway, the short form is better.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The long form doesn't work when you vendor (in the dune sense) slipshow ^^"

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes! I understand. Thanks for the explanation :)

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you have ocamlformatted this file, but there is a .ocamlformat in the directory which should have disabled that...
Can you revert the change?

; bos
))
(preprocessor_deps
(file client/client.bc.js))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the dune manual, (file <filename>) and <filename> is the same. What is the reason to prefer on to the other?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh that was from testing the ppx_blob issue I mentioned above

@panglesd
Copy link
Owner

(The CI issue seems unrelated to this. I'll need to fix to do a release...)

Irmin-watcher is a portable, filesystem notification library.
On linux it uses inotify and on macOS it uses fsevents. It has
a portable, if slow, polling mode too.

This makes slipshow installable on macOS.
@panglesd panglesd merged commit bc71b9a into panglesd:master Sep 12, 2024
4 checks passed
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