afewmail / afew

an initial tagging script for notmuch mail
ISC License
325 stars 98 forks source link

Use watchdog file system watcher to support OSX #312

Closed benmezger closed 2 years ago

benmezger commented 3 years ago

I wanted to get the watch feature to work on OSX, so I ended up having to modify and introduce watchdog, which supports multiple platforms. Hopefully, someone might find this interesting. At the moment, I am unable to test this on Linux, but it should work out of the box, however, if someone could this test on Linux I would appreciate it.

I haven't found a styling guide on this project, so I hope I got the correct coding style.

GuillaumeSeren commented 3 years ago

Hello @benmezger sorry to be late.

As with every patch I ask a tests that can implemented to help maintaining the code, in that specific case I am not sure

I think suporting more platform is good, and for coding style we try to follow flake8 when we can.

benmezger commented 2 years ago

Hey @GuillaumeSeren, thanks for your patience. I've added the corresponding tests.

codecov[bot] commented 2 years ago

Codecov Report

Merging #312 (d529b30) into master (8ef9a5b) will increase coverage by 9.63%. The diff coverage is 84.07%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #312      +/-   ##
==========================================
+ Coverage   47.08%   56.71%   +9.63%     
==========================================
  Files          30       31       +1     
  Lines        1079     1109      +30     
==========================================
+ Hits          508      629     +121     
+ Misses        571      480      -91     
Flag Coverage Δ
unittests 56.71% <84.07%> (+9.63%) :arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
afew/main.py 0.00% <0.00%> (ø)
setup.py 0.00% <0.00%> (ø)
afew/files.py 68.35% <63.63%> (+68.35%) :arrow_up:
afew/tests/test_files.py 100.00% <100.00%> (ø)
afew/filters/HeaderMatchingFilter.py 40.00% <0.00%> (-2.11%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8ef9a5b...d529b30. Read the comment docs.

GuillaumeSeren commented 2 years ago

You again @benmezger ! Thank you for sharing this with us.

So this would be a nice addition for mac users, we probably want to build up a small doc / usage / example of this setup.

Beside reviewing I think we have to talk about the storage of the configuration / default values in this patch, I've read in the other issue that you worked some change in the configuration file maybe some of it could be usefull here.