-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
repl: backslash bug fix #2952
Closed
Closed
repl: backslash bug fix #2952
Conversation
This file contains 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
When some string is copied and pasted in REPL, if it has a newline (\n) in it, then it is considered as end of line and REPL throws a SyntaxError, because a line cannot end with a unclosed string literal. This behavior is because of the fact that `readline` module breaks at all the `\n`s in the input string and treats them as a seperate line. So whenever it encounters a new line character it will emit a `line` event. This commit basically reverts nodejs@81ea52a, which was an attempt to improve the way string literals were handled by REPL. Fixes: nodejs#2749
As it is, the trimWhitespace function will not remove the white sapce characters at the end of the string, as the greedy capturing group .+ captures everything till end of the string, leaving \s* with nothing. This patch replaces the buggy function with the built-in, String prototype's trim function.
LGTM, but your description regarding
|
|
thefourtheye
added a commit
to thefourtheye/io.js
that referenced
this pull request
Sep 20, 2015
The actual problem was with the line parsing logic for string literals. When we use backslash in the string literals, it used to remember the `\` as the previous character even after we parsed the character next to it. This leads to REPL thinking that the end of string literals is not reached. This patch replaces the previous character with `null`, so that it will properly skip the character next to it. Previous Discussion: nodejs#2952 Fixes: nodejs#2749
@silverwind I could not be more wrong. This problem has nothing to do with the |
thefourtheye
added a commit
that referenced
this pull request
Sep 22, 2015
The actual problem was with the line parsing logic for string literals. When we use backslash in the string literals, it used to remember the `\` as the previous character even after we parsed the character next to it. This leads to REPL thinking that the end of string literals is not reached. This patch replaces the previous character with `null`, so that it will properly skip the character next to it. Previous Discussion: #2952 Fixes: #2749 PR-URL: #2968 Reviewed-By: Roman Reiss <me@silverwind.io>
thefourtheye
added a commit
that referenced
this pull request
Sep 22, 2015
The actual problem was with the line parsing logic for string literals. When we use backslash in the string literals, it used to remember the `\` as the previous character even after we parsed the character next to it. This leads to REPL thinking that the end of string literals is not reached. This patch replaces the previous character with `null`, so that it will properly skip the character next to it. Previous Discussion: #2952 Fixes: #2749 PR-URL: #2968 Reviewed-By: Roman Reiss <me@silverwind.io>
This was referenced Jun 29, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
1st commit:
When some string is copied and pasted in REPL, if it has a newline (\n)
in it, then it is considered as end of line and REPL throws a
SyntaxError, because a line cannot end with a unclosed string literal.
This behavior is because of the fact that
readline
module breaks atall the
\n
s in the input string and treats them as a seperate line.So whenever it encounters a new line character it will emit a
line
event.
This commit basically reverts 81ea52a, which was an attempt to improve
the way string literals were handled by REPL.
Fixes: #2749
2nd commit
As it is, the trimWhitespace function will not remove the white sapce
characters at the end of the string, as the greedy capturing group .+
captures everything till end of the string, leaving \s* with nothing.
This patch replaces the buggy function with the built-in, String
prototype's trim function.
Note: The second commit is necessary because, the revert brings back the old buggy trimming function. So, I though we could fix it now itself.
cc @silverwind @Fishrock123