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

Include/Exclude filters? #79

Open
lox opened this issue May 22, 2015 · 11 comments
Open

Include/Exclude filters? #79

lox opened this issue May 22, 2015 · 11 comments

Comments

@lox
Copy link

lox commented May 22, 2015

It would be great to have an easy way of specifying include/exclude filters when registering watches. This would allow for inotify and other backends that have a limited number of watches to not watch folders that would be excluded by the user.

@rjeczalik
Copy link
Owner

@lox Something like this?

package notify

// Filter adds a regexp against which each path is matched during watchpoint
// setup or event dispatch; in case of a match the watchpoint is skipped
// and not set or the event is ignored. The returned number says how many
// filters is currently registred.
//
// If out is "", the last added filter is removed.
func Filter(out string) int

Example usage:

package main

import "github.com/rjeczalik/notify"

func main() {
    c := make(chan notify.EventInfo, 16)

    notify.Filter(".svn|.bzr|.git")

    err := notify.Watch(os.Getenv("GOPATH")+"/src", c)
    if err != nil {
        panic(err)
    }
    for ei := range c {
        println("This event surely does not come from inside .git, .svn or .bzr:", ei)
    }
}

PS I'm afraid per-watch filters would be too cumbersome to implement.

@nathany
Copy link
Collaborator

nathany commented May 27, 2015

Or a slice of Glob patterns?

Though I must ask, what would be the advantage of direct support over filtering the EventInfo that is received?

(fsnotify ref https://github.com/go-fsnotify/fsnotify/issues/10)

@rjeczalik
Copy link
Owner

@nathany The use-case is to filter paths before the watch is set. It could help when you have 2028 fds left, the directory tree you're about to watch has 7228 directories and the .git directory - 5200 ;)

@nathany
Copy link
Collaborator

nathany commented May 27, 2015

Makes sense. At least on systems with fds. How would this work on fsevents?

@rjeczalik
Copy link
Owner

How would this work on fsevents?

@nathany As a nop, it will just filter out reported events before dispatching them to the channel. FSEvents listens to the whole tree so there's no facility to make it ignore particular leaf.

@nathany
Copy link
Collaborator

nathany commented May 27, 2015

Makes sense. Then it's just a matter of API.

I would suggest Exclude as Filter could be taken as inclusive or exclusive. Except, Without...

Why use a pipe separated string rather than commas or a slice? Or just multiple calls to exclude more things?

@rjeczalik
Copy link
Owner

Why use a pipe separated string rather than commas or a slice?

@nathany It's just regex, the example is unfortunate enough to suggest multiple paths are separated with pipe char - I think it should be properly described by method's inline doc. Imho there's no sense for inclusive filters, as if no filter is set every directory within a tree is watched for recursive watchpoints. The purpose of the filter is to filter out trees which user knows are better ignored. Eventually the method could be func FilterOut(regex string) int to fix the confusion.

If the method accepted slices instead the API would be a bit more complicated to make it possible to e.g. remove all filters. I like how raw yet simple the stack-based filters are :)

notify.Filter(".git")
notify.Filter(".svn")
notify.Filter(".bzr")

// remove all filters
for notify.Filter("") > 0 {
}

notify.Filter(".git|.svn|.bzr")

@unbalancedparentheses
Copy link

It would be really useful to have this feature :)

@imsodin
Copy link
Contributor

imsodin commented Apr 12, 2016

I was pointed here by Zillode after filing PR Zillode#3. I needed the ignore functionality for syncthing-inotify and filed it against the Zillode fork, as my approach is limited to the part of the notify package that was accessed by the toplevel function Watch on linux/ext4. My approach is that a function is given to the Watch function that takes as a argument a path as a string and returns true, if this path is to be ignored. This of course is not ideal as it is not backward compatible (from what I read go does not support optional function arguments). I would appreciate if you would have a look at it and told me, what you think.

@imsodin
Copy link
Contributor

imsodin commented Apr 21, 2016

I now separated the ignoreTest specification from the Watch function. Thus there is no change in how to use the Watch function and everything remains just as it was before. Ignoring is only enabled by calling SetIgnoreTest with the desired function that determines which paths are ignored.
If you have no intention of incorporating/adapting my changes please say so. I am aware it is very minimalist and I have almost zero coding skills (at least no collaborative experience) and thus my contribution may be of low quality. It still would be great to have any kind of feedback - thanks.

@vladyslav2
Copy link

and still, I believe it will be nice feature to have!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants