haskell-fswatch / hfsnotify

Unified Haskell interface for basic file system notifications
BSD 3-Clause "New" or "Revised" License
136 stars 40 forks source link

System.FSNotify does not export type WatchConfig #108

Closed mpilgrem closed 1 year ago

mpilgrem commented 1 year ago

I understand the API change from 0.4.0.0 to cease to export the data constructors for the data type WatchConfig.

However, is it intentional not to export the data type too, despite exporting defaultConfig :: WatchConfig?

Stack depends on fsnotify and has a function:

fileWatchConf
  :: (HasLogFunc env, HasTerm env)
  => WatchConfig
  -> ((Set (Path Abs File) -> IO ()) -> RIO env ())
  -> RIO env ()

However, if type WatchConfig is not available, this function can't compile.

thomasjm commented 1 year ago

Stack currently has an fsnotify constraint of < 0.4, are you trying to update Stack to the new version?

Sure, I'd be fine with exporting WatchConfig.

thomasjm commented 1 year ago

6433c5a6b04732ee7cdf0059b6c341200e718372

mpilgrem commented 1 year ago

@thomasjm, many thanks. I am indeed in the process of making Stack 'fsnotify >= 0.4 ready'. The existing upper bound was added to help people who use Cabal (the tool) to build Stack.

mpilgrem commented 1 year ago

@thomasjm, may I ask another question: in the Cabal file, why do you use executable tests, rather than a test-suite section? (Stack and Cabal (the tool) build executables with the library, but only build test suites on request.)

thomasjm commented 1 year ago

It's because I like to use the TUI interface of Sandwich to run tests in development, which works better as an executable (https://github.com/codedownio/sandwich/issues/35).

We could backtrack on this if it's causing problems though.

If you're a Stack developer, any chance you'd be the right person to talk to about making stack test able to set up a TTY for the test process? :)

mpilgrem commented 1 year ago

On executable tests, 'problems' is putting it too high, but I think it would be good for users of the fsnotify library to be able to choose not to build the executable (and its dependencies). Would you consider using a Cabal flag?

On Stack, I am more of a janitor than an architect but if you have an idea about how stack test (aka stack build --test) could be extended, please do open an issue in the Stack repository.

thomasjm commented 1 year ago

Would you consider using a Cabal flag?

It's certainly possible, although I don't enjoy tweaking cabal flags in packages I use. The best solution in general would be to fix this in Stack (and also Cabal). But in the short term, if it's causing pain for hfsnotify users, it's not a big deal to turn it back into a test-suite.

thomasjm commented 1 year ago

https://github.com/haskell-fswatch/hfsnotify/commit/c7e9722664d0abd7e2d32cc1e9e7bb5f219b67be

mpilgrem commented 1 year ago

Thanks!