search

rjeczalik / notify

File system event notification library on steroids.
MIT License
906 stars 128 forks source link

export event.IsDir() #123

Closed temoto closed 7 years ago

temoto commented 7 years ago

https://github.com/rjeczalik/notify/issues/100

ppknap commented 7 years ago

The signature IsDir() (bool, error) is not idiomatic - Is* commands should return bool without error part. Can we find a better name for this? AFAIR this was the main reason why this method is private. 🤔

temoto commented 7 years ago

@ppknap I'm confused. IsDir should return bool, but isDir can return anything else?

ppknap commented 7 years ago

We did know that IsDir() (bool, error) is a bad function signature. Thus, we left it unexported so we won't introduce breaking API changes if we come up with a better name.

temoto commented 7 years ago

How about IsDirError?

rjeczalik commented 7 years ago

Hey @temoto, it is not about the naming but rather API surface. The goal here is to keep notify.EventInfo interface possibly small (meaning also interface upgrades).

What I think would fit nice is:

type Stater interface {
    Stat() (os.FileInfo, error)
}

And then some notify.fileInfo struct that will implement os.FileInfo - and set notify.fileInfo.isDir to the desired bool value. From os.FileInfo methods that we're unable to provide meaningful values, just return zero ones + proper inline documentation.

Another advantage besides better API would be possibility to just return os.FileInfo for those watcher implementations that do os.Stat either way (kqueue, future poller).

If you'd want to take try at that you may update this PR or start a new one.

temoto commented 7 years ago

Thank you. I only care about linux target and happy with fork.