Implemented fix for first parameter being a substring of the second p…#106432
Conversation
| return param.label; | ||
| } else { | ||
| const idx = signature.label.lastIndexOf(param.label); | ||
| const idx = signature.label.lastIndexOf(`${param.label},`) ?? signature.label.lastIndexOf(`${param.label})`); |
There was a problem hiding this comment.
I don't think the part after ?? will ever be executed since lastIndexOf always returns a number
There was a problem hiding this comment.
Yes that's correct. I have updated my PR. Also took into account these scenarios for idx. Note that I add one because lookup contains additional character at start.
//() -> 0
//(param) -> 1
//(param, xparam) -> 1
//(xparam,param) -> 8
//(xparam,param,yparam) -> 8
//(xparam, param,yparam) -> 9
//(xparam, param) -> 9
//(xparam,param) -> 9
|
@mjbvz I'm sorry I'm not familiar with this code ping @joaomoreno |
| } else { | ||
| const idx = signature.label.lastIndexOf(param.label); | ||
| return idx >= 0 | ||
| const idx = Math.max( |
There was a problem hiding this comment.
This fix seems specific to JavaScript style signatures. How about looking for the last match of a regular expression such as \b${param.label}\b which would check for the param label surrounded by word boundaries? Not perfect but that way param would not match parameter
There was a problem hiding this comment.
Thanks for your feedback. Let me give that a shot
There was a problem hiding this comment.
Updated. Let me know what you think
mjbvz
left a comment
mjbvz
left a comment
There was a problem hiding this comment.
Looks good overall. Just one minor change and then I think we can merge this
| return param.label; | ||
| } else { | ||
| const idx = signature.label.lastIndexOf(param.label); | ||
| const regex = new RegExp(`\\b${param.label}\\b`, 'g'); |
There was a problem hiding this comment.
In case the param.label contains special regular expression, make sure to use escapeRegExpCharacters
|
Thanks! This will be in the next VS Code insiders build and is scheduled to ship with VS Code 1.50 |
Awesome happy to help! |
3 participants
Problem was that 'param' was a substring of 'parameter'. Fixed it such that lookup in the signature label requires parameters to either end with a comma or bracket.
This PR fixes #106129