search

owickstrom / komposition

The video editor built for screencasters
https://owickstrom.github.io/komposition/
Mozilla Public License 2.0
429 stars 21 forks source link

Use withSystemTempDirectory to clean up temporary directories #43

Closed tfausak closed 6 years ago

tfausak commented 6 years ago

Fixes #26.

owickstrom commented 6 years ago

Hey! Thanks for the PR. There seems to be a problem, though, I think with binding the pipes Producer in the withSystemTempDirectory combinator's inner action. IIRC, this is why I didn't use some bracket-like approach in the first place, because I didn't figure out how to do it with pipes.

tfausak commented 6 years ago

Ah, interesting! I'm not actually familiar with Pipes and hoped that this would work. I couldn't get things set up on my local machine, so I figured I'd throw up a PR for discussion and to see what Travis CI said about it.

That being said, maybe bracket from pipes-safe is the thing to use here? https://hackage.haskell.org/package/pipes-safe-2.3.1/docs/Pipes-Safe.html#v:bracket

owickstrom commented 6 years ago

Cool. Yeah, I think pipes-safe should work, as the SafeT transformer is already used.

tfausak commented 6 years ago

Great! I'll try to update this PR soon.

tfausak commented 6 years ago

I spent about an hour trying to get this project set up and didn't have anything to show for myself. I figured I'd make the changes I wanted to make and push them up. That way I can see what CI says. At any rate, I think the idea here is sound, but I suspect some of the types might not match up. I might need to sprinkle in some calls to liftBase.

tfausak commented 6 years ago

I think this is ready for review now!

owickstrom commented 6 years ago

Looks great, @tfausak! Thank you for the PR. :raised_hands: