GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Revert "repl: disable Ctrl+C support on win32 for now" by addaleax · Pull Request #8645 · nodejs/node

Conversation

@addaleax

@addaleax addaleax commented Sep 18, 2016

Copy link
Copy Markdown
Member
Checklist
  • make -j4 test (UNIX), or vcbuild test nosign (Windows) passes
  • commit message follows commit guidelines
Affected 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
joshgav reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@addaleax addaleax added repl Issues and PRs related to the REPL subsystem. windows Issues and PRs related to the Windows platform. labels Sep 18, 2016
@nodejs-github-bot nodejs-github-bot added the repl Issues and PRs related to the REPL subsystem. label Sep 18, 2016
@seishun

seishun commented Sep 18, 2016

Copy link
Copy Markdown
Contributor

Too early for that since libuv hasn't been upgraded.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@addaleax addaleax added the blocked PRs that are blocked by other issues or PRs. label Sep 18, 2016
@addaleax

addaleax commented Sep 18, 2016

Copy link
Copy Markdown
Member Author

@seishun yeah, I realize that you’d have to apply the libuv change as well :)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
cjihrig
cjihrig approved these changes Sep 18, 2016
View reviewed changes

@cjihrig cjihrig left a comment

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
bzoz
bzoz approved these changes Sep 19, 2016
View reviewed changes

@bzoz bzoz left a comment

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@addaleax addaleax added the dont-land-on-v4.x label Sep 20, 2016
jasnell
jasnell approved these changes Sep 20, 2016
View reviewed changes

@jasnell jasnell left a comment

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Trott
Trott force-pushed the master branch from b0df363 to c5ce7f4 Compare September 21, 2016 00:10
@jasnell

jasnell commented Oct 6, 2016

Copy link
Copy Markdown
Member

@addaleax ... ping :-)
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@addaleax

addaleax commented Oct 6, 2016

Copy link
Copy Markdown
Member Author

This is still blocked … presumably on #8280, but generally whatever libuv update includes the fix.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell

jasnell commented Oct 6, 2016

Copy link
Copy Markdown
Member

ok, figured as much, just wanted to check in on it tho :-)
addaleax reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@rvagg
rvagg force-pushed the master branch 2 times, most recently from c133999 to 83c7a88 Compare October 18, 2016 17:02
@addaleax
Revert "repl: disable Ctrl+C support on win32 for now"
c6b00bf 
This reverts commit f59b888
now that the libuv update containing the proper fix has
landed in 63243bc.

Ref: libuv/libuv#1054
Ref: nodejs#7837
@addaleax
addaleax force-pushed the revert-repl-ctrl-c-win32 branch from 9a0fe1f to c6b00bf Compare October 27, 2016 14:06
@addaleax addaleax added lts-watch-v6.x and removed blocked PRs that are blocked by other issues or PRs. labels Oct 27, 2016
@addaleax

addaleax commented Oct 27, 2016

Copy link
Copy Markdown
Member Author

Rebased now that the libuv update has landed – this should work now.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
seishun
seishun approved these changes Oct 27, 2016
View reviewed changes

@seishun seishun left a comment

Copy link
Copy Markdown
Contributor

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@addaleax

addaleax commented Oct 27, 2016

Copy link
Copy Markdown
Member Author

@seishun 🎉 Thanks for confirming!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@addaleax

addaleax commented Oct 30, 2016

Copy link
Copy Markdown
Member Author

CI: https://ci.nodejs.org/job/node-test-commit/5871/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@addaleax

addaleax commented Nov 16, 2016

Copy link
Copy Markdown
Member Author

Landed in 2e28875
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@addaleax addaleax closed this Nov 16, 2016
@addaleax
addaleax deleted the revert-repl-ctrl-c-win32 branch November 16, 2016 18:45
addaleax added a commit that referenced this pull request Nov 16, 2016
@addaleax
Revert "repl: disable Ctrl+C support on win32 for now"
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

MylesBorins commented Nov 18, 2016

Copy link
Copy Markdown
Contributor

this should be backported when we update libuv to v1.10.x
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MylesBorins MylesBorins mentioned this pull request Nov 18, 2016
deps: upgrade libuv to 1.10.1 #9647
Merged
2 tasks done
@addaleax addaleax removed the dont-land-on-v6.x label Jan 23, 2017
@addaleax

addaleax commented Jan 23, 2017

Copy link
Copy Markdown
Member Author

@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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
MylesBorins pushed a commit that referenced this pull request Jan 23, 2017
@addaleax @MylesBorins
Revert "repl: disable Ctrl+C support on win32 for now"
3d87150 
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 MylesBorins added land-on-v6.x and removed lts-watch-v6.x labels Jan 23, 2017
@MylesBorins

MylesBorins commented Jan 23, 2017

Copy link
Copy Markdown
Contributor

thanks for the heads up
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
MylesBorins pushed a commit that referenced this pull request Jan 24, 2017
@addaleax @MylesBorins
Revert "repl: disable Ctrl+C support on win32 for now"
83f095a 
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 MylesBorins mentioned this pull request Jan 24, 2017
v6.10.0 proposal #10974
Merged
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 24, 2017
@addaleax Italo A. Casas
Revert "repl: disable Ctrl+C support on win32 for now"
465c585 
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]>
@italoacasas italoacasas mentioned this pull request Jan 25, 2017
Proposal to update v7.x-staging #10989
Closed
italoacasas pushed a commit to italoacasas/node that referenced this pull request Jan 27, 2017
@addaleax Italo A. Casas
Revert "repl: disable Ctrl+C support on win32 for now"
e7b656d 
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]>
@italoacasas italoacasas mentioned this pull request Jan 29, 2017
v7.5.0 proposal #11062
Merged
MylesBorins pushed a commit that referenced this pull request Jan 31, 2017
@addaleax @MylesBorins
Revert "repl: disable Ctrl+C support on win32 for now"
1d400ea 
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]>
@seishun seishun mentioned this pull request Mar 28, 2017
Sometimes REPL cannot display the result #12085
Closed
@MylesBorins

MylesBorins commented Mar 28, 2017

Copy link
Copy Markdown
Contributor

@addaleax it would appear that this shouldn't have landed on v6.x before the libuv upgrade

reverted the revert in 1978911
addaleax reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MylesBorins MylesBorins added lts-watch-v6.x and removed land-on-v6.x labels Mar 28, 2017
@MylesBorins MylesBorins mentioned this pull request Mar 28, 2017
deps: upgrade libuv to 1.10.0 #9267
Merged
2 tasks done
@MylesBorins MylesBorins mentioned this pull request Mar 29, 2017
Revert "Revert "repl: disable Ctrl+C support..." #12123
Merged
MylesBorins added a commit to MylesBorins/node that referenced this pull request Mar 29, 2017
@MylesBorins
Revert "Revert "repl: disable Ctrl+C support..."
ee453a4 
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]>
MylesBorins added a commit that referenced this pull request Mar 29, 2017
@MylesBorins
Revert "Revert "repl: disable Ctrl+C support..."
59b869e 
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]>
MylesBorins added a commit that referenced this pull request Mar 29, 2017
@MylesBorins
Revert "Revert "repl: disable Ctrl+C support..."
64fc5a4 
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]>
MylesBorins pushed a commit that referenced this pull request May 16, 2017
@addaleax @MylesBorins
Revert "repl: disable Ctrl+C support on win32 for now"
b0a6e67 
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 MylesBorins added land-on-v6.x and removed lts-watch-v6.x labels May 16, 2017
MylesBorins pushed a commit that referenced this pull request May 18, 2017
@addaleax @MylesBorins
Revert "repl: disable Ctrl+C support on win32 for now"
827411c 
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 MylesBorins mentioned this pull request May 23, 2017
v6.11.0 proposal #13059
Merged
andrew749 pushed a commit to michielbaird/node that referenced this pull request Jul 19, 2017
@addaleax @andrew749
Revert "repl: disable Ctrl+C support on win32 for now"
6ae1b26 
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes
@cjihrig cjihrig cjihrig approved these changes
+2 more reviewers
@seishun seishun seishun approved these changes
@bzoz bzoz bzoz approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

repl Issues and PRs related to the REPL subsystem. windows Issues and PRs related to the Windows platform.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@addaleax @seishun @jasnell @MylesBorins @cjihrig @bzoz @nodejs-github-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.