repl: display error message when loading directory#4170
Closed
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
lib/repl.js Outdated
Member
There was a problem hiding this comment.
maybe is not a file -> is not a valid file
princejwesleyforce-pushed the repl-load-dir branch 2 times, most recently from 88ebc6d to 3e03a6aCompare
ContributorAuthor
princejwesley commented Dec 8, 2015
@JungMinu Updated. |
Member
JungMinu commented Dec 9, 2015
/cc @evanlucas@thefourtheye PTAL :) |
jasnell added the semver-major 
Member
jasnell commented Dec 9, 2015
Semver-major due to the added throw |
Member
jasnell commented Dec 9, 2015
LGTM |
Member
JungMinu commented Dec 9, 2015
LGTM |
Member
bnoordhuis commented Dec 10, 2015
Can you add a regression test to |
ContributorAuthor
princejwesley commented Dec 10, 2015
@bnoordhuis Sure. I'll add and update here |
When loading directory instead of file, no error message is displayed. Its good to display error message for this scenario. Before: > .load /Users/princejohnwesley/Projects/Playground/Node > After: > .load /Users/princejohnwesley/Projects/Playground/Node Failed to load:/Users/princejohnwesley/Projects/Playground/Node is not a valid file >
princejwesleyforce-pushed the repl-load-dir branch from 3e03a6a to 8a6db50CompareContributorAuthor
princejwesley commented Dec 10, 2015
@bnoordhuis Test added |
Member
bnoordhuis commented Dec 10, 2015
LGTM, thanks. CI: https://ci.nodejs.org/job/node-test-pull-request/975/ |
Member
bnoordhuis commented Dec 10, 2015
@JungMinu I see you've assigned this to yourself. Feel free to land it when the CI checks out. |
Member
JungMinu commented Dec 11, 2015
CI is happy except for unrelated Windows test :) |
JungMinu pushed a commit that referenced this pull request Dec 11, 2015
When loading directory instead of file, no error message is displayed. It's good to display error message for this scenario. PR-URL: #4170 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
Member
JungMinu commented Dec 11, 2015
Thanks, landed in aad6b9f |
Contributor
Fishrock123 commented Dec 11, 2015
@jasnell how is this semver-major? |
Member
jasnell commented Dec 11, 2015
@Fishrock123 ... precautionary based on the new throw. If folks are
|
Member
ChALkeR commented Dec 11, 2015
@jasnell Is there a new throw? |
Member
jasnell commented Dec 11, 2015
Sorry, not new throw... sigh, I'm doing too many things at once. I meant the new output message. While it's not technically a throw, it is a new output indicating that the operation failed. The ctc had decided to treat changes to error messages as semver-major... this one comes across as a grey area to me so to be safe I flagged it. Like I said tho, I'm perfectly fine with removing that if others are -- I may just be being overly conservative on it :-) |
Member
ChALkeR commented Dec 11, 2015
Ah, that makes sense then, thanks for the explanation! =) |
Contributor
Fishrock123 commented Dec 11, 2015
I don't think REPL warnings count as error messages. (We've added and changed lots of them with the repl history stuff anyways.) |
Member
jasnell commented Dec 11, 2015
@Fishrock123 ... ok! works for me then. Removing the flag :-) |
jasnell removed the semver-major 
cjihrig pushed a commit that referenced this pull request Dec 15, 2015
When loading directory instead of file, no error message is displayed. It's good to display error message for this scenario. PR-URL: #4170 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
Closed
MylesBorins pushed a commit that referenced this pull request Dec 29, 2015
When loading directory instead of file, no error message is displayed. It's good to display error message for this scenario. PR-URL: #4170 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
When loading directory instead of file, no error message is displayed. It's good to display error message for this scenario. PR-URL: #4170 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
Merged
MylesBorins pushed a commit that referenced this pull request Jan 19, 2016
Notable changes: * assert - accommodate ES6 classes that extend Error (Rich Trott) #4166 * build - add "--partly-static" build options (Super Zheng) #4152 * deps - backport 066747e from upstream V8 (Ali Ijaz Sheikh) #4655 - backport 200315c from V8 upstream (Vladimir Kurchatkin) #4128 - upgrade libuv to 1.8.0 (Saúl Ibarra Corretgé) * docs - various updates landed in 70 different commits! * repl - attach location info to syntax errors (cjihrig) #4013 - display error message when loading directory (Prince J Wesley) #4170 * tests - various updates landed in over 50 commits * util - allow lookup of hidden values (cjihrig) #3988
MylesBorins pushed a commit that referenced this pull request Jan 20, 2016
Notable changes: * assert - accommodate ES6 classes that extend Error (Rich Trott) #4166 * build - add "--partly-static" build options (Super Zheng) #4152 * deps - backport 066747e from upstream V8 (Ali Ijaz Sheikh) #4655 - backport 200315c from V8 upstream (Vladimir Kurchatkin) #4128 - upgrade libuv to 1.8.0 (Saúl Ibarra Corretgé) * docs - various updates landed in 70 different commits! * repl - attach location info to syntax errors (cjihrig) #4013 - display error message when loading directory (Prince J Wesley) #4170 * tests - various updates landed in over 50 commits * util - allow lookup of hidden values (cjihrig) #3988 PR-URL: #4768
MylesBorins pushed a commit that referenced this pull request Jan 20, 2016
Notable changes: * assert - accommodate ES6 classes that extend Error (Rich Trott) #4166 * build - add "--partly-static" build options (Super Zheng) #4152 * deps - backport 066747e from upstream V8 (Ali Ijaz Sheikh) #4655 - backport 200315c from V8 upstream (Vladimir Kurchatkin) #4128 - upgrade libuv to 1.8.0 (Saúl Ibarra Corretgé) * docs - various updates landed in 70 different commits! * repl - attach location info to syntax errors (cjihrig) #4013 - display error message when loading directory (Prince J Wesley) #4170 * tests - various updates landed in over 50 commits * util - allow lookup of hidden values (cjihrig) #3988 PR-URL: #4768
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
When loading directory instead of file, no error message is displayed. It's good to display error message for this scenario. PR-URL: nodejs#4170 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
scovetta pushed a commit to scovetta/node that referenced this pull request Apr 2, 2016
Notable changes: * assert - accommodate ES6 classes that extend Error (Rich Trott) nodejs#4166 * build - add "--partly-static" build options (Super Zheng) nodejs#4152 * deps - backport 066747e from upstream V8 (Ali Ijaz Sheikh) nodejs#4655 - backport 200315c from V8 upstream (Vladimir Kurchatkin) nodejs#4128 - upgrade libuv to 1.8.0 (Saúl Ibarra Corretgé) * docs - various updates landed in 70 different commits! * repl - attach location info to syntax errors (cjihrig) nodejs#4013 - display error message when loading directory (Prince J Wesley) nodejs#4170 * tests - various updates landed in over 50 commits * util - allow lookup of hidden values (cjihrig) nodejs#3988 PR-URL: nodejs#4768
This was referenced Jul 31, 2021
This was referenced Aug 12, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
7 participants
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.
When loading directory instead of file, no error message
is displayed. Its good to display error message for
this scenario.
Before:
After: