feat: FileWatcher now ignores changes from patterns in .gitignore

[jonathanj]

FileWatcher will build a gitignore ruleset from all .gitignore files in the repo and any global .gitignore, and use this to ignore file change events. This prevents a lot of FileUpdate events being generated when ignored items like build folders change, which may have been contributing to freezes.

If a change is detected involving a .gitignore file, the ruleset is rebuilt.

Changes are also now no longer watched from a thread, which caused problems with some watcher implementations such as Fsevents on macOS.

Fixes #359
Fixes #358

[jonathanj]

I've removed starting the watcher in a thread because I suspect that the cause was triggering a lot of update events as a result of should-have-been-ignored files changing (such as target/). The notify docs mention notify_debouncer_mini, which may be yet another incremental improvement.

This is an optmistic change since I am not sure how to reproduce the freezes myself, so I can't say whether this fixes them or not. If there is a repo that will reproduce this, I'm happy to check it myself although I'll be using the Fsevents watcher on macOS.

If you think it's best to make that change separate to the gitignore changes, or if it's not easy to reproduce the freezing, I'm also happy to remove those changes from this PR.

#359 (comment)

[jonathanj]

There are no tests because there are no existing tests for FileWatcher to extend, and I am really not sure how to write them from scratch. Happy to add them if you have a suggestion for how to approach it.

[altsem]

Nice! I do agree we can drop spawning it in a thread and fixing things properly instead.
Tests would be cool I thought too, not sure where to begin. Perhaps if there was some way to refresh the UI only (without updating the state via git diff, git status etc.) in tests.

I believe this approach is missing some of the variants of ignoring stuff though, as seen in https://git-scm.com/docs/gitignore.

Did some digging and found this actually: https://git-scm.com/docs/git-check-ignore and https://docs.rs/git2/latest/git2/struct.Repository.html#method.status_should_ignore

And in gitu:

gitu on  359-filewatcher-apply-gitignore [!] ❯ git check-ignore target
target
gitu on  359-filewatcher-apply-gitignore [!] ❯ echo $status
0

Works in subdirs:

gitu/target on  359-filewatcher-apply-gitignore [!] ❯ git check-ignore .
.
gitu/target on  359-filewatcher-apply-gitignore [!] ❯ echo $status
0

I've disabled showUntrackedFiles in my git-managed home repo, it seems check-ignore returns false:

~ ❯ GIT_DIR="$HOME/repos/cfg.git" GIT_WORK_TREE="$HOME" git check-ignore .cache/gnome-calculator/un-daily.xls
~ ❯ echo $status # I'd hope for a 0 (ignored), but seems it doesn't consider showUntrackedFiles
1
~ ❯ GIT_DIR="$HOME/repos/cfg.git" GIT_WORK_TREE="$HOME" git config status.showUntrackedFiles
no

[altsem]

But let's leave the tests aside. I believe it'd be messy, since file update events would come in async. And it'd need to have some sort of sleep duration in hope of catching it.

[jonathanj]

Did some digging and found this actually: https://git-scm.com/docs/git-check-ignore and https://docs.rs/git2/latest/git2/struct.Repository.html#method.status_should_ignore

Oops, in hindsight this seems like such an obvious thing to have looked for first, good find! If nothing else, this dramatically simplifies the ignore logic, to the point that I don't think the ignore crate is even necessary any more? I've pushed a change that switches to this and removes the ignore crate.

Aside, I noticed (after logging the event type for watch events) that some of these happen multiple times, which probably also creates a lot of churn that have no real effect. It might be worth investigating notify_debouncer_mini as a separate PR to make this less twitchy.

[00:00:30.041] (16bd9f000) INFO   File changed: "/Users/jonathan/Coding/gitu/src/file_watcher.rs" (Modify(Metadata(Any)))
[00:00:30.041] (16bd9f000) INFO   File changed: "/Users/jonathan/Coding/gitu/src/file_watcher.rs" (Modify(Data(Content)))
[00:00:30.042] (16bd9f000) INFO   File changed: "/Users/jonathan/Coding/gitu/src/file_watcher.rs~" (Create(File))
[00:00:30.042] (16bd9f000) INFO   File changed: "/Users/jonathan/Coding/gitu/src/file_watcher.rs~" (Remove(File))
[00:00:30.042] (16bd9f000) INFO   File changed: "/Users/jonathan/Coding/gitu/src/file_watcher.rs~" (Modify(Metadata(Any)))
[00:00:30.042] (16bd9f000) INFO   File changed: "/Users/jonathan/Coding/gitu/src/file_watcher.rs~" (Modify(Metadata(Ownership)))
[00:00:30.042] (16bd9f000) INFO   File changed: "/Users/jonathan/Coding/gitu/src/file_watcher.rs~" (Modify(Metadata(Extended)))
[00:00:30.043] (16bd9f000) INFO   File changed: "/Users/jonathan/Coding/gitu/src/file_watcher.rs~" (Modify(Data(Content)))

But let's leave the tests aside. I believe it'd be messy, since file update events would come in async. And it'd need to have some sort of sleep duration in hope of catching it.

Okay. I'll give it some thought, it would be nice to invest in those tests at some stage given the number of issues that have come up in this fairly small set of code.

[altsem]

I recently merged a whole bunch of refactoring, where the creation of the file watcher was moved. Now it'd be real easy to just accept a Repository instead of a Path as argument. Then you'd don't need to call open_repo anymore!

It's created here!

gitu/src/state.rs

Line 91 in e7b7719

repo.workdir().expect("Bare repos unhandled"),

[altsem]

(I'll test the PR out again soon, but looks promising!)

[altsem]

Regading debouncing events.
I suppose it would be nice to iron out some redundant updates. But those cases should be quite rare, as Gitu polls the file-watcher every 100ms (rate increases when there are terminal events). So lets leave it for later :)

[codecov]

Codecov Report

Attention: Patch coverage is 0% with 15 lines in your changes missing coverage. Please review.

Project coverage is 87.56%. Comparing base (740a86f) to head (904b1d7).
Report is 9 commits behind head on master.

Files with missing lines	Patch %	Lines
src/file_watcher.rs	0.00%	15 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #360      +/-   ##
==========================================
+ Coverage   87.50%   87.56%   +0.06%     
==========================================
  Files          66       66              
  Lines        6537     6532       -5     
==========================================
  Hits         5720     5720              
+ Misses        817      812       -5     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[altsem]

Gj on this! I feel relieved the file watcher is in a better state.
I'll merge it in!