libs: remove redundant RegExp parts #13519
Conversation
nodejs-github-bot
added
dns
lib/repl.js
Outdated
| @@ -772,7 +772,7 @@ function complete(line, callback) { | |||
| name = files[f]; | |||
| ext = path.extname(name); | |||
| base = name.slice(0, -ext.length); | |||
| if (base.match(/-\d+\.\d+(\.\d+)?/) || name === '.npm') { | |||
| if (base.match(/-\d+\.\d+/) || name === '.npm') { | |||
There was a problem hiding this comment.
Also may as well change this to a regex.test() since the return value is discarded.
| @@ -1067,7 +1067,7 @@ REPLServer.prototype.memory = function memory(cmd) { | |||
| self.lines.level.push({ | |||
| line: self.lines.length - 1, | |||
| depth: depth, | |||
There was a problem hiding this comment.
just depth would be fine instead of depth: depth
There was a problem hiding this comment.
This seems to be the object of another PR. But I recall that some collaborators oppose partial shorthands: this is preferred to be consistent, all shorthands or all full pairs.
There was a problem hiding this comment.
I wouldn't worry about this for this PR.
There was a problem hiding this comment.
@vsemozhetbyt Didn't know about the guideline :p I had a review in my PR for shorthand that's why!
There was a problem hiding this comment.
It is not a guideline yet, just a comment in some PR or proposition that I recall)
|
The first CI was green. I've replaced New CI: https://ci.nodejs.org/job/node-test-pull-request/8530/ 1 flaky test fails on arm, seems unrelated. |
lib/repl.js
Outdated
| @@ -1067,7 +1067,7 @@ REPLServer.prototype.memory = function memory(cmd) { | |||
| self.lines.level.push({ | |||
| line: self.lines.length - 1, | |||
| depth: depth, | |||
| isFunction: /\s*function\s*/.test(cmd) | |||
| isFunction: /function/.test(cmd) | |||
There was a problem hiding this comment.
Micro-nit: Might be better as /\bfunction\b/ or /\sfunction\s/?
|
Nit addressed. CI: https://ci.nodejs.org/job/node-test-pull-request/8533/ |
|
Sorry, it seems I have a bit wider PR emerging, so I will close this for now, as it will interweave with the new one. |
|
Absorbed by #13536 |
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passesAffected core subsystem(s)
dns, repl
If I get this correctly, a RegExp part with a quantifier '0 or more' is redundant if:
test());match()/exec()results (full or partial) or does not affect match indices.However, these parts may improve readability and comprehensibility. If this is the case for the changed fragments, feel free to reject this PR.
Also, I can miss some other nuances, so, please, correct me.