Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34.2k
Revert "repl: disable Ctrl+C support on win32 for now"#8645
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
Uh oh!
There was an error while loading. Please reload this page.
Conversation
seishun commented Sep 18, 2016
Too early for that since libuv hasn't been upgraded. |
addaleax commented Sep 18, 2016
@seishun yeah, I realize that you’d have to apply the libuv change as well :) |
cjihrig left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, once it's time to land this. The libuv update should be in the near future.
bzoz left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
jasnell left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
jasnell commented Oct 6, 2016
@addaleax ... ping :-) |
addaleax commented Oct 6, 2016
This is still blocked … presumably on #8280, but generally whatever libuv update includes the fix. |
jasnell commented Oct 6, 2016
ok, figured as much, just wanted to check in on it tho :-) |
c133999 to 83c7a88CompareThis reverts commit f59b888 now that the libuv update containing the proper fix has landed in 63243bc. Ref: libuv/libuv#1054 Ref: nodejs#7837
9a0fe1f to c6b00bfCompareaddaleax commented Oct 27, 2016
Rebased now that the libuv update has landed – this should work now. |
seishun left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Can't reproduce #7837 anymore.
addaleax commented Oct 27, 2016
@seishun 🎉 Thanks for confirming! |
addaleax commented Oct 30, 2016
addaleax commented Nov 16, 2016
Landed in 2e28875 |
This reverts commit f59b888 now that the libuv update containing the proper fix has landed in 63243bc. Ref: libuv/libuv#1054 Ref: #7837 PR-URL: #8645 Reviewed-By: Nikolai Vavilov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Bartosz Sosnowski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins commented Nov 18, 2016
this should be backported when we update libuv to v1.10.x |
addaleax commented Jan 23, 2017
@MylesBorins This should land on v6.x (because the commit that is reverted by this PR is also in v6.x). But together with or after the libuv upgrade, yes. |
This reverts commit f59b888 now that the libuv update containing the proper fix has landed in 63243bc. Ref: libuv/libuv#1054 Ref: #7837 PR-URL: #8645 Reviewed-By: Nikolai Vavilov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Bartosz Sosnowski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins commented Jan 23, 2017
thanks for the heads up |
This reverts commit f59b888 now that the libuv update containing the proper fix has landed in 63243bc. Ref: libuv/libuv#1054 Ref: #7837 PR-URL: #8645 Reviewed-By: Nikolai Vavilov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Bartosz Sosnowski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit f59b888 now that the libuv update containing the proper fix has landed in 63243bc. Ref: libuv/libuv#1054 Ref: nodejs#7837 PR-URL: nodejs#8645 Reviewed-By: Nikolai Vavilov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Bartosz Sosnowski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit f59b888 now that the libuv update containing the proper fix has landed in 63243bc. Ref: libuv/libuv#1054 Ref: nodejs#7837 PR-URL: nodejs#8645 Reviewed-By: Nikolai Vavilov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Bartosz Sosnowski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit f59b888 now that the libuv update containing the proper fix has landed in 63243bc. Ref: libuv/libuv#1054 Ref: #7837 PR-URL: #8645 Reviewed-By: Nikolai Vavilov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Bartosz Sosnowski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
MylesBorins commented Mar 28, 2017
Full original message: Revert "repl: disable Ctrl+C support on win32 for now" This reverts commit 1d400ea. Fixes: nodejs#12085 Refs: nodejs#8645 PR-URL: nodejs#12123 Reviewed-By: Anna Henningsen <[email protected]>
Full original message: Revert "repl: disable Ctrl+C support on win32 for now" The original fix was a stop gap until a libuv update landed. As the libuv update has not yet landed on v6.x the revert should not have landed. This commit reverts 1d400ea reapplying the stopgap fix until we update libuv. Fixes: #12085 Refs: #8645 PR-URL: #12123 Reviewed-By: Anna Henningsen <[email protected]>
Full original message: Revert "repl: disable Ctrl+C support on win32 for now" The original fix was a stop gap until a libuv update landed. As the libuv update has not yet landed on v6.x the revert should not have landed. This commit reverts 1d400ea reapplying the stopgap fix until we update libuv. Fixes: #12085 Refs: #8645 PR-URL: #12123 Reviewed-By: Anna Henningsen <[email protected]>
This reverts commit f59b888 now that the libuv update containing the proper fix has landed in 63243bc. Ref: libuv/libuv#1054 Ref: #7837 PR-URL: #8645 Reviewed-By: Nikolai Vavilov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Bartosz Sosnowski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit f59b888 now that the libuv update containing the proper fix has landed in 63243bc. Ref: libuv/libuv#1054 Ref: #7837 PR-URL: #8645 Reviewed-By: Nikolai Vavilov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Bartosz Sosnowski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
This reverts commit f59b888 now that the libuv update containing the proper fix has landed in 63243bc. Ref: libuv/libuv#1054 Ref: nodejs/node#7837 PR-URL: nodejs/node#8645 Reviewed-By: Nikolai Vavilov <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Bartosz Sosnowski <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Checklist
make -j4 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
repl
Description of change
This reverts commit f59b888, since apparently libuv/libuv#1054 fixed the underlying problem that led to temporary disabling. I’d really like Windows users (@seishun? @bzoz?) to check that everything works as expected.
Ref: libuv/libuv#1054
Ref: #7837