markis / watchman-processor

Folder synchronization tool with a simple dashboard
https://www.npmjs.com/package/watchman-processor
MIT License
43 stars 7 forks source link

Make watchman-processor librar-ish #133

Open ventuno opened 6 years ago

ventuno commented 6 years ago

Most npm packages that run as a binary have also the ability to be imported in a project and run as a library. It would be great if watchman-processor worked the same way, so that developers could extend its functionality and create new UIs or integrate it with other tools if needed. See watchman-processor-tray-icon [1] as an example that uses watchman-processor as a library to create a bitbar plugin [2] and send notifications to the notification center (see also this better implementation of a tray icon using electron [3]).

Changes summary: 1) Modify WatchmanProcessor.ts to emit node events instead of calling directly Terminal.ts; 1) In bin/watchman-processor listen to these events and call the corresponding Terminal.ts methods; 1) Update tests accordingly.

@markis, let me know if you like this idea, I think it would be a pretty powerful extension to this great package. More than happy to switch to more a appropriate architecture if needed, I kind of wanted to write down the code to convey the idea better.

[1] https://github.com/ventuno/watchman-processor-tray-icon/blob/f73d873463a3f682c649a1ca80b8b76cb2cbb11e/bitbar-plugin.js [2] https://github.com/matryer/bitbar#writing-plugins [3] https://github.com/ventuno/watchman-processor-tray-icon/pull/1

coveralls commented 6 years ago

Coverage Status

Coverage remained the same at 100.0% when pulling 874258e6483b82eadadd4477b4e0e7f94d388c68 on ventuno:make-watchman-processor-librar-ish into 98052a509a2f2b9b52d316a0aa76a46869fde3af on markis:master.

markis commented 6 years ago

Wow, cool! I love the idea of integrating this with bitbar.

I am not a fan of magic strings and I would not want the internals of this to be dependent on them. It inevitably leads to this: https://github.com/markis/watchman-processor/pull/133/commits/f7d93260e63a2f9bda57240d9f4fab08e498b2eb#diff-0ba683aead6f4617bf0950fcc8b0913dL72

I think all of this could have been done simply by building a different "terminal" class and just injecting it instead of rewiring all the internals to use events.

ventuno commented 6 years ago

Thanks @markis how do you suggest I proceed? How would the alternative "terminal" class emit events to the outside world and how do developers inject it? Eventing/hooks seemed the most standard approach. I could expose event strings as constants and improved tests should protect from "magic" strings error.

ventuno commented 6 years ago

@markis I spent some more time on this to use an enum instead of just strings to trigger/listen to events. I tried to improve tests, but due the overall asynchronicity of the whole program it's a little difficult to do so without introducing flakiness. Let me know what you think.

ventuno commented 6 years ago

@markis ping?

ventuno commented 6 years ago

Sure @markis, just wanted to know if this had still a chance :-). Good luck with your sale!

ventuno commented 5 years ago

Hey @markis any chance you could get to this? Maybe in the new year?