Conversation
princejwesley
left a comment
princejwesley
left a comment
There was a problem hiding this comment.
convertToContext is unused function. Do we need to correct it?
|
lgtm, but does pulling that array in allow you to pull entries up to previous line and satisfy line length restrictions? |
lib/repl.js
Outdated
There was a problem hiding this comment.
looks like a behavior change?
There was a problem hiding this comment.
@Fishrock123 No. we don't need to escape characters inside character class [``] except for - in non-tail position.
|
@rvagg asked:
It was possible to do that even without reducing the indentation, so yes, definitely possible. I didn't want to do it because I wanted to avoid hard-to-read diff that might be criticized as churn. A whitespace-only change is annoying enough, but at least it's easy to comprehend the diff. :-) However, if you or anyone else feels strongly that it's important to re-wrap the lines, I can do that. (Honestly, I'd prefer one-item-to-a-line over put-as-many-as-you-can-on-a-line because at least adding and deleting items becomes very clear in diffs. But that's just my preference.) |
|
@princejwesley wrote:
I guess we don't need to, but I figured I might as well remove all the no-op escapes while I was at it. |
|
your call on the array @Trott, no strong feelings from me either way |
lib/repl.js
Outdated
There was a problem hiding this comment.
Remove _ while you’re at it? It’s included in \w.
* remove unnecessary backslash (`\`) escaping in regular expressions * favor `===` over `==` * multiline arrays indentation consistent with other indentation
|
CI failures are infra/build related. Landing momentarily.... |
* remove unnecessary backslash (`\`) escaping in regular expressions * favor `===` over `==` * multiline arrays indentation consistent with other indentation PR-URL: nodejs#9374 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
|
Landed in 5cbb7a8 |
* remove unnecessary backslash (`\`) escaping in regular expressions * favor `===` over `==` * multiline arrays indentation consistent with other indentation PR-URL: #9374 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
|
@Trott do you want to backport this? it will need to be done manually |
* remove unnecessary backslash (`\`) escaping in regular expressions * favor `===` over `==` * multiline arrays indentation consistent with other indentation PR-URL: nodejs#9374 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
* remove unnecessary backslash (`\`) escaping in regular expressions * favor `===` over `==` * multiline arrays indentation consistent with other indentation PR-URL: nodejs#9374 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
|
@thealphanerd Backports done in #9747 and #9748 |
* remove unnecessary backslash (`\`) escaping in regular expressions * favor `===` over `==` * multiline arrays indentation consistent with other indentation PR-URL: #9374 Ref: #9747 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
* remove unnecessary backslash (`\`) escaping in regular expressions * favor `===` over `==` * multiline arrays indentation consistent with other indentation PR-URL: #9374 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
* remove unnecessary backslash (`\`) escaping in regular expressions * favor `===` over `==` * multiline arrays indentation consistent with other indentation PR-URL: #9374 Ref: #9747 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
* remove unnecessary backslash (`\`) escaping in regular expressions * favor `===` over `==` * multiline arrays indentation consistent with other indentation PR-URL: #9374 Reviewed-By: Rod Vagg <rod@vagg.org> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
9 participants
Checklist
make -j8 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
repl
Description of change
\) escaping in regular expressions===over==