Skip to content

lib: avoid REPL exit on completion error#3358

Merged
Trott merged 1 commit intonodejs:masterfrom
Trott:fix-issue-3346
Oct 21, 2015
Merged

lib: avoid REPL exit on completion error#3358
Trott merged 1 commit intonodejs:masterfrom
Trott:fix-issue-3346

Conversation

@Trott
Copy link
Member

@Trott Trott commented Oct 14, 2015

If a tab completion is attempted on an undefined reference inside of a
function, the REPL was exiting without reporting an error or anything
else. This change results in the REPL reporting the ReferenceError and
continuing.

Fixes: #3346

@Trott Trott mentioned this pull request Oct 14, 2015
Closed
@Trott
Copy link
Member Author

Trott commented Oct 14, 2015

CI: https://ci.nodejs.org/job/node-test-commit/834/

@mscdex mscdex added the repl Issues and PRs related to the REPL subsystem. label Oct 14, 2015
@Trott
Copy link
Member Author

Trott commented Oct 14, 2015

Fixed borkage. CI take two: https://ci.nodejs.org/job/node-test-commit/836/

@Trott
Copy link
Member Author

Trott commented Oct 14, 2015

Removed a line, so one more CI: https://ci.nodejs.org/job/node-test-commit/837/

@Trott
Copy link
Member Author

Trott commented Oct 14, 2015

More tweaks! I should go to sleep. Last CI for a while I hope: https://ci.nodejs.org/job/node-test-commit/838/

@bnoordhuis
Copy link
Member

bnoordhuis commented Oct 14, 2015

LGTM. The addition of the .top property makes it technically a semver-minor, I think?

@Trott
Copy link
Member Author

Trott commented Oct 14, 2015

@bnoordhuis Since it's intended for internal use only, perhaps I can hide it a bit by making the property key a Symbol rather than the string top. Or if that's overkill, maybe I can at least prefix it with _.

@bnoordhuis
Copy link
Member

bnoordhuis commented Oct 14, 2015

I'd go with a symbol.

@Trott
Copy link
Member Author

Trott commented Oct 14, 2015

@bnoordhuis I did a Symbol implementation but then went a step further and hid the properties entirely using a WeakMap. PTAL

@Trott
Copy link
Member Author

Trott commented Oct 14, 2015

CI: https://ci.nodejs.org/job/node-test-commit/845/

@Trott
Copy link
Member Author

Trott commented Oct 20, 2015

Bump. I think this is good for a merge... /cc @bnoordhuis

@jasnell
Copy link
Member

jasnell commented Oct 21, 2015

LGTM... err. I mean, WTIC /cc @Trott

@jasnell
Copy link
Member

jasnell commented Oct 21, 2015

@Trott ... can I ask you to squash the commits and I'll get it landed.

@Trott
lib: avoid REPL exit on completion error
b354be7 
If a tab completion is attempted on an undefined reference inside of a
function, the REPL was exiting without reporting an error or anything
else. This change results in the REPL reporting the ReferenceError and
continuing.

Fixes: nodejs#3346
PR-URL: nodejs#3358
Reviewed-By: James M Snell <jasnell@gmail.com>
@Trott Trott force-pushed the fix-issue-3346 branch 2 times, most recently from f8e494b to b354be7 Compare October 21, 2015 22:35
@Trott
Copy link
Member Author

Trott commented Oct 21, 2015

Rebased to master, squashed, running one last CI before landing on master: https://ci.nodejs.org/job/node-test-commit/909/

@Trott Trott merged commit b354be7 into nodejs:master Oct 21, 2015
@Trott
Copy link
Member Author

Trott commented Oct 21, 2015

Landed in b354be7

Trott added a commit that referenced this pull request Oct 22, 2015
@Trott @rvagg
lib: avoid REPL exit on completion error
97d0817 
If a tab completion is attempted on an undefined reference inside of a
function, the REPL was exiting without reporting an error or anything
else. This change results in the REPL reporting the ReferenceError and
continuing.

Fixes: #3346
PR-URL: #3358
Reviewed-By: James M Snell <jasnell@gmail.com>
Trott added a commit that referenced this pull request Oct 26, 2015
@Trott @jasnell
lib: avoid REPL exit on completion error
1888f84 
If a tab completion is attempted on an undefined reference inside of a
function, the REPL was exiting without reporting an error or anything
else. This change results in the REPL reporting the ReferenceError and
continuing.

Fixes: #3346
PR-URL: #3358
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell
Copy link
Member

jasnell commented Oct 26, 2015

Landed in v4.x-staging in 1888f84

This was referenced Oct 27, 2015
Merged
Closed
Trott added a commit that referenced this pull request Oct 29, 2015
@Trott @jasnell
lib: avoid REPL exit on completion error
f49c7c6 
If a tab completion is attempted on an undefined reference inside of a
function, the REPL was exiting without reporting an error or anything
else. This change results in the REPL reporting the ReferenceError and
continuing.

Fixes: #3346
PR-URL: #3358
Reviewed-By: James M Snell <jasnell@gmail.com>
@jasnell jasnell mentioned this pull request Oct 29, 2015
Closed
@Fishrock123
Copy link
Contributor

Fishrock123 commented Nov 3, 2015

@Trott just noticing this, could you please make sure to be more specific for the subsystem? i.e. repl instead of lib (Only use lib when it really affects like everything, or more than 2-3). :D

@Trott
Copy link
Member Author

Trott commented Nov 3, 2015

👍

@Trott Trott deleted the fix-issue-3346 branch January 13, 2022 22:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

repl Issues and PRs related to the REPL subsystem.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Trott @bnoordhuis @jasnell @Fishrock123 @mscdex