- Notifications
You must be signed in to change notification settings - Fork23
fix: SSHConfig: check for multiple start/end blocks#510
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
09777c5 toeda19a2CompareThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Nice find!
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Small suggestion but otherwise LGTM 👍🏻
| count++ | ||
| pos=haystack.indexOf(needle,pos+needle.length) | ||
| } | ||
| returncount |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
I think this could be simplified, like so?
return[...haystack.matchAll(needle)].length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
It looks way better, butmatchAll wants aRegExp, which meansneedle would need to be properly un-escaped first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Oh yeah, forgot not all these methods are string | RegExp. That can be simply amended withRegExp.escape(needle).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Only available in Node 24 and current version of VSCode is on 23.11.0 if I'm not mistaken 🥲
Note to self: add changelog updates in separate PR so we don't force reapproval. |
84acd8c intomainUh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Relates to#504
What this does:
Ports over the existing checks in
coder config-sshto handle malformed blocks.Now, if your SSH config is malformed, you will see an error similar to the below upon attempting to connect:
If one of the automatically generated blocks is missing a start or end comment, you will see the below error:
If there is more than one block with a given label, you will see the below error:
In both cases, it will unfortunately be necessary to edit the SSH config and fix the issue manually.