Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Launch Native REPL using terminal link #24734

Merged
merged 22 commits into from
Jan 23, 2025

Conversation

anthonykim1
Copy link

@anthonykim1 anthonykim1 commented Jan 22, 2025

Resolves: #24270
Perhaps further improve via #24270 (comment) ?

@anthonykim1 anthonykim1 added debt Covers everything internal: CI, testing, refactoring of the codebase, etc. area-repl labels Jan 22, 2025
@anthonykim1 anthonykim1 self-assigned this Jan 22, 2025
@anthonykim1 anthonykim1 changed the title Launch native REPL when custom text from REPL in terminal is clicked. Provide option to launch native REPL when custom text from REPL in terminal is clicked. Jan 22, 2025
@anthonykim1 anthonykim1 added this to the January 2025 milestone Jan 22, 2025
@anthonykim1 anthonykim1 marked this pull request as ready for review January 22, 2025 22:22
Copy link

⚠️ This PR originates from a fork. Due to security restrictions, pipelines from forks are no longer triggered automatically. Learn more.

If the changes appear safe, you can manually trigger the pipeline by commenting /AzurePipelines run.

@anthonykim1 anthonykim1 marked this pull request as draft January 22, 2025 22:52
@@ -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":
Copy link
Member

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?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean tooltip: l10n.t('Launch VS Code Native REPL'), part?
I think the the tooltip would only show Launch VS Code Native REPL

@anthonykim1 anthonykim1 marked this pull request as ready for review January 22, 2025 23:51
Copy link

⚠️ This PR originates from a fork. Due to security restrictions, pipelines from forks are no longer triggered automatically. Learn more.

If the changes appear safe, you can manually trigger the pipeline by commenting /AzurePipelines run.

Copy link
Member

@karthiknadig karthiknadig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing test

@karthiknadig karthiknadig changed the title Provide option to launch native REPL when custom text from REPL in terminal is clicked. Launch native REPL using terminal link Jan 23, 2025
@karthiknadig karthiknadig changed the title Launch native REPL using terminal link Launch Native REPL using terminal link Jan 23, 2025
Comment on lines +164 to +165
assert.isNotNull(links, 'Expected links to be not undefined');
assert.isArray(links, 'Expected links to be an array');
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should check link.command here and validate start and end lengths. if you put the localized string in localize.ts then you can verify tooltip.

You also need a negative test case.

@anthonykim1
Copy link
Author

just realized this has been non-draft mode. Sorry for the spam everyone :)

@anthonykim1 anthonykim1 merged commit 25411e5 into microsoft:main Jan 23, 2025
46 checks passed
@anthonykim1
Copy link
Author

I'm going to delete the outdated comments mentioned in the test file in separate PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-repl debt Covers everything internal: CI, testing, refactoring of the codebase, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prompt for Native REPL when user types python in terminal
3 participants