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

Fix cabal repl handling of Ctrl-C #1535

Merged
merged 1 commit into from
Oct 11, 2013
Merged

Conversation

dagit
Copy link
Collaborator

@dagit dagit commented Oct 7, 2013

The code here does fix #1448, but since it "borrows" code directly from the process package this may not be the best way to fix it. See for example, http://ghc.haskell.org/trac/ghc/ticket/8419.

Please let me know if I need to change anything so that we can squash this C-c handling bug :)

@23Skidoo
Copy link
Member

23Skidoo commented Oct 8, 2013

I think that this a fine workaround, but it'd be nice to fix the issue in process itself.

@tibbe
Copy link
Member

tibbe commented Oct 11, 2013

We should do both, fix process and merge this workaround. @23Skidoo if the code looks fine to you feel free to merge.

23Skidoo added a commit that referenced this pull request Oct 11, 2013
Fix cabal repl handling of Ctrl-C
@23Skidoo 23Skidoo merged commit f038519 into haskell:master Oct 11, 2013
@sol
Copy link
Member

sol commented Oct 12, 2013

Awesome. Any chance to get this in a bug fix release on Hackage?

@23Skidoo
Copy link
Member

Any chance to get this in a bug fix release on Hackage?

That's up to @tibbe.

23Skidoo added a commit to 23Skidoo/cabal that referenced this pull request Oct 12, 2013
23Skidoo added a commit to 23Skidoo/cabal that referenced this pull request Oct 12, 2013
@23Skidoo
Copy link
Member

@dagit Apparently you didn't test your code on Windows. Fixed in 940079c. @tibbe, if you decide to backport this to 1.18, please also include my Windows fix.

@dagit
Copy link
Collaborator Author

dagit commented Oct 14, 2013

@23Skidoo You're right. I didn't test on windows. Sorry about that. Things got merged a) quicker than I had time to update them b) quicker than I expected :)

I was hoping to switch to bracket here and in process this weekend, but I was busy with other stuff. I'll try to get that patch submitted this week, before I get bogged down in work + courseras.

Thanks for merging this and fixing up the windows stuff!

@23Skidoo
Copy link
Member

Nice. Improvements welcome, of course.

23Skidoo added a commit that referenced this pull request Oct 15, 2013
Broken by #1535.

(cherry picked from commit 940079c)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cabal repl gets killed by interrupts
5 participants