REPL: tab complete for Arrays #25819
Conversation
Slight modified version of @seebees original commit: seebees@d624a68 The original message: The tab complete for Arrays will include any integer indexes. This is suboptimal because obj.21 is not valid syntax and a large array will contain a very cluttered list. This alternative tweaks the original commit to use Array.isArray and isNaN for the filter. Everything else in untouched.
|
@joyent/node-tsc .. quick review? |
| .push(Object | ||
| .getOwnPropertyNames(obj) | ||
| .filter(function(item) { | ||
| return isNaN(parseInt(item)); |
There was a problem hiding this comment.
Maybe use +item instead of parseInt(). For example, parseInt('3c') returns 3.
There was a problem hiding this comment.
Yeah, that's actually a 'feature' ;-) ... consider.. var m = []; m['123abc'] = 'a' .. if you do tab completion, the 123abc and use +item, the 123abc would still appear, which ends up having the same problem this patch is trying to fix. Using parseInt filters those out, even if they are valid keys for the object.
|
@jasnell How cool is that! thanks! What's the best way to get a PR reviewed? |
|
In an ideal world, all PR's will be reviewed ;-) ... some just may take longer than others. This particular one is being actively reviewed now. Barring any issues, it should land shortly. |
| 'asdf.hasOwnProperty', 'asdf.isPrototypeOf', | ||
| 'asdf.propertyIsEnumerable', | ||
| 'asdf.toLocaleString', 'asdf.toString', | ||
| 'asdf.valueOf', '', 'asdf.1'], |
There was a problem hiding this comment.
Looking into the test... it appears that somewhere along the line, test-repl-tab-complete stopped working properly. The entire test is not run and exits with a success without executing all of the code. Investigating now
There was a problem hiding this comment.
The issue with the test exiting part of the way through has been fixed in the converged repo.
|
Will be updating this shortly... |
|
Closing here in favor of nodejs/node#2409, which is a more complete approach. That will need to be cherry picked back to v0.12 and v0.10 at some point |
Refactored version of nodejs/node-v0.x-archive#25819 Removes integer keys (and keys starting with numbers) from candidate list on repl tab complete. Refactored the originally submitted change to simplify and ensure that the integer keys do not show up on objects either.
Refactored version of nodejs/node-v0.x-archive#25819 Removes integer keys (and keys starting with numbers) from candidate list on repl tab complete. Refactored the originally submitted change to simplify and ensure that the integer keys do not show up on objects either. Reviewed By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> PR-URL: #2409
Refactored version of nodejs/node-v0.x-archive#25819 Removes integer keys (and keys starting with numbers) from candidate list on repl tab complete. Refactored the originally submitted change to simplify and ensure that the integer keys do not show up on objects either. Reviewed By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> PR-URL: #2409
Slight modified version of @seebees original commit:
seebees@d624a68
The original message:
The tab complete for Arrays will
include any integer indexes. This is
suboptimal because obj.21 is not
valid syntax and a large array will
contain a very cluttered list.
This alternative tweaks the original commit to use
Array.isArray and isNaN for the filter. Everything
else in untouched.