search

SoptikHa2 / desed

Debugger for Sed: demystify and debug your sed scripts, from comfort of your terminal.
https://soptik.tech/articles/building-desed-the-sed-debugger.html
GNU General Public License v3.0
1.12k stars 22 forks source link

Autoreload on source change #20

Closed suve closed 3 years ago

suve commented 4 years ago

This changeset aims to solve #12.

Currently, only Linux is supported via inotify. Some work on supporting BSDs using kqueue is underway, but not yet commited to the repo, since I have to set up my cross-compiler first and I don't want to commit code that most probably doesn't even compile.

I've only been writing Rust for a few weeks now, so I recommend checking the code thoroughly. :P

SoptikHa2 commented 4 years ago

Thanks a lot for the work! The code looks really good, I wouldn't even guess you have just few weeks of Rust under your belt.

The multiplatform code will be interesting problem to solve - we will want inotify as backend on linux, kqueue on *BSD and, well, nothing, on other systems.

One way of doing it would probably (I'm not sure whether it'll work, I never wrote anything like this before) be specifying generic trait - something similar to the way UI is defined generically and using it instead of the linux version of FileWatcher, and then having two different implementation of the trait - one that would be compiled only on linux, and one that would be compiled only on *BSDs [1]. And some way to handle platform that isn't supported at all.

Anyway, I understand this is probably very confusing and I'm not helping too much with my explanation of the idea. I can take care of the multiplatform code if you want to.

Regarding crosscompiler, I guess you don't have to really mess with it if you don't want to. I'll probably brush up my qemu skills and spin up some *BSD to test the code regardless.

Last, I would like to thank you for your contribution! It's really good code, and I'd like to release it as soon as the multiplatform code will be taken care of.

[1]: https://doc.rust-lang.org/reference/conditional-compilation.html#target_os, https://doc.rust-lang.org/rust-by-example/attribute/cfg.html

suve commented 4 years ago

Added some BSD code, and while it compliles, it doesn't work - I can't get the events to appear. Also, when reloading the code by-hand, FileWatcher::start() returns an error - "os error 22", which is EINVAL; the kqueue(2) man page says that is is caused by an invalid event (watch) description. Which is kinda weird, since the error is not thrown on program start, only on reload. Maybe you can't have two kqueue(2) instances watching the same file?

SoptikHa2 commented 4 years ago

No idea why the events don’t appear. Also, bsd on qemu doesn’t work on my old substitute laptop, so I won’t be able to debug it until the end of the week (hopefully) when my work laptop is fixed.

And yeah, it looks like watchers should be removed when we exit TUI, altough I didn’t test it. I’ll try to setup some cargo check tests so we can use github CI.

worr commented 4 years ago

@SoptikHa2 @suve Hey, idk if y'all use or check your Gitlab accounts very much, but I wanted to provide an update that I have a branch that may resolve the issue with kqueue, it just needs some verification before I merge it and cut a release.

suve commented 4 years ago

The issue with the kqueue crate, as reported on GitLab, has been solved and the code in the PR works for me™ when tested on my Linux box and on a FreeBSD 12.1 VM.

worr commented 4 years ago

I also did a small test on OpenBSD-current, which seems to work just fine as well.

SoptikHa2 commented 3 years ago

Thanks a lot for coding this! (And sorry it took so long to merge).

I released desed v1.2.0 which ships with this code. Everything looks great so far!