Conversation
weswigham
left a comment
weswigham
left a comment
There was a problem hiding this comment.
I don't actually have any objections to this, conceptually, since we've said that we wanted to offer completions for strings of all kinds for awhile. I do have a remark on the regex being used for number matching, though.
src/services/completions.ts
Outdated
| } | ||
|
|
||
| function isNumericLiteralText(name: string): boolean { | ||
| return /^-?\d+(\.\d+)?(e\d+)?$/.test(name); |
There was a problem hiding this comment.
There are contexts like property names:
const foob = {
-0() {
return 1;
}
};where a leading minus would never be allowed (only an identifier, string literal, or number) - I don't think this should include the check for -??
There was a problem hiding this comment.
Also, this helper probably should be somewhere central, since it will need to be updated once we support the numeric seperator proposal.
There was a problem hiding this comment.
We can just use a string literal in this case then, which makes this function unnecessary. 👍
…nd use string literals for number keys
| // We, thus, need to check if whatever was inside the quotes is actually a valid identifier name. | ||
| if (performCharacterChecks && !isIdentifierText(name, target)) { | ||
| return undefined; | ||
| return allowStringLiteral ? JSON.stringify(name) : undefined; |
There was a problem hiding this comment.
that would be another place where a user-defined quote setting will need to be honored.
ghost
merged commit 
3 participants
Fixes #17040
This was a pretty simple change as I just had to change the code that had previously been returning
undefinedon any quoted name.There may be objections to this as there were to #16864, so CC @rbuckton @weswigham