- Notifications
You must be signed in to change notification settings - Fork1.2k
Launch Native REPL using terminal link#24734
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
If the changes appear safe, you can manually trigger the pipeline by commenting |
| @@ -77,3 +76,8 @@ def __str__(self): | |||
| if sys.platform != "win32" and (not is_wsl) and use_shell_integration: | |||
| sys.ps1 = PS1() | |||
| if sys.platform == "darwin": | |||
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.
is it weird if only part of this string is fixed to create a link with l10n? Will that translate only half the command?
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.
Do you meantooltip: l10n.t('Launch VS Code Native REPL'), part?
I think the the tooltip would only showLaunch VS Code Native REPL
If the changes appear safe, you can manually trigger the pipeline by commenting |
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.
Missing test
Uh oh!
There was an error while loading.Please reload this page.
| assert.isNotNull(links, 'Expected links to be not undefined'); | ||
| assert.isArray(links, 'Expected links to be an array'); |
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.
You should checklink.command here and validate start and end lengths. if you put the localized string inlocalize.ts then you can verify tooltip.
You also need a negative test case.
just realized this has been non-draft mode. Sorry for the spam everyone :) |
25411e5 intomicrosoft:mainUh oh!
There was an error while loading.Please reload this page.
I'm going to delete the outdated comments mentioned in the test file in separate PR. |
Uh oh!
There was an error while loading.Please reload this page.
Resolves:#24270
Perhaps further improve via#24270 (comment) ?