Skip to content

Switch over to executeCommand from sendText - #24078

Merged
Anthony Kim (anthonykim1) merged 37 commits into
microsoft:mainfrom
anthonykim1:executeCommandNM
Sep 12, 2024
Merged

Switch over to executeCommand from sendText #24078
Anthony Kim (anthonykim1) merged 37 commits into
microsoft:mainfrom
anthonykim1:executeCommandNM

Conversation

@anthonykim1

@anthonykim1 Anthony Kim (anthonykim1) commented Sep 9, 2024

Copy link
Copy Markdown

Resolves: #23929

TODO: (debt --> in separate PR) Have ensureTerminal return Promise instead of Promise and saving this in the TerminalService class. Would avoid many uses of the !, and maybe even get to throw away the TerminalService class itself.

@anthonykim1 Anthony Kim (anthonykim1) added feature-request Request for new features or functionality area-terminal labels Sep 9, 2024
Comment thread src/client/common/terminal/service.ts Outdated
@@ -146,9 +146,13 @@ export class SynchronousTerminalService implements ITerminalService, Disposable
lockFile.dispose();
}
}

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We should use less of these services when possible.

Comment thread src/client/common/terminal/service.ts
Comment thread src/client/common/terminal/service.ts Outdated
Comment thread src/client/common/terminal/service.ts Outdated
Comment thread src/test/common/terminals/service.unit.test.ts Outdated
Comment thread src/test/common/terminals/service.unit.test.ts Outdated
@anthonykim1
Anthony Kim (anthonykim1) marked this pull request as draft September 12, 2024 01:16
Comment thread src/test/common/terminals/service.unit.test.ts Outdated
await initialize();
return undefined;
});
// TODO: This test is being flaky for windows, need to investigate why only fails on windows

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we have a debt item for this? and also for the TODO to change how terminal is saved.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

They are now here: #24105 and #24106

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

some minor things to change

@anthonykim1
Anthony Kim (anthonykim1) marked this pull request as draft September 12, 2024 18:47
@anthonykim1
Anthony Kim (anthonykim1) merged commit 216c7ed into microsoft:main Sep 12, 2024
Anthony Kim (anthonykim1) added a commit to anthonykim1/vscode-python that referenced this pull request Sep 13, 2024
Resolves: microsoft#23929 

TODO: (debt --> in separate PR) Have ensureTerminal return
Promise<Terminal> instead of Promise<void> and saving this in the
TerminalService class. Would avoid many uses of the !, and maybe even
get to throw away the TerminalService class itself.
Anthony Kim (anthonykim1) added a commit to anthonykim1/vscode-python that referenced this pull request Sep 15, 2024
Resolves: microsoft#23929

TODO: (debt --> in separate PR) Have ensureTerminal return
Promise<Terminal> instead of Promise<void> and saving this in the
TerminalService class. Would avoid many uses of the !, and maybe even
get to throw away the TerminalService class itself.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area-terminal feature-request Request for new features or functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Use executeCommand instead of sendText to activate Start Terminal REPL

2 participants