haskell / haskeline

A Haskell library for line input in command-line programs.
https://hackage.haskell.org/package/haskeline
BSD 3-Clause "New" or "Revised" License
223 stars 75 forks source link

Move away from Chan to STM #61

Closed bgamari closed 7 years ago

bgamari commented 7 years ago

The isEmptyChan broken has been long known to be broken. Moreover, the atomicity guarantees provided by STM simplify things a bit.

bgamari commented 7 years ago

Note that this still needs to be testing on Windows.

hvr commented 7 years ago

@bgamari btw, a side-effect of this is that GHC would start including stm in its global package db!

(see also https://ghc.haskell.org/trac/ghc/wiki/Commentary/Libraries/VersionHistory)

bgamari commented 7 years ago

Oh no! Yes, I suppose you are right. That is quite unfortunate but I'm not sure I see an alternative. I suppose we could implement a locked queue with the semantics, but this doesn't seem like that much better of an alternative.

hvr commented 7 years ago

@bgamari as I said already on IRC, I think we should invest some time during the GHC 8.3.x dev cycle to figure out a way to decouple haskeline/terminfo from GHC's global pkg-db again so that haskeline can pick up new libraries (within reason) more easily :-)

judah commented 7 years ago

Thanks for the patch!

@hvr, I agree that decoupling from the global package DB would be a big help. As another example, it currently limits which version of Haskeline can be included in a given Stackage LTS release.

Since it won't be necessary until ghc-8.4, and is a non-trivial change, I'll hold off on this PR until after we've cut the Haskeline release corresponding to ghc-8.2.1.

bgamari commented 7 years ago

Perhaps we could merge this now?

judah commented 7 years ago

@bgamari, LGTM except it looks like the PR is failing on Travis CI, I think due to missing a dependency on stm in haskeline.cabal. Can you add that and try again?

bgamari commented 7 years ago

I think this should be ready now.