Skip to content

Add WebSocket Terminal support #1150

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

Merged
merged 5 commits into from
May 25, 2023

Conversation

isc-bsaviano
Copy link
Contributor

This PR adds support for for the new terminal endpoint added in Atelier API v7. It will be available in the next preview of IRIS 2023.2. See the new documentation included in this PR for how to start the terminal and what features it supports.

isc-rsingh
isc-rsingh previously approved these changes May 15, 2023
@gjsjohnmurray
Copy link
Contributor

I look forward to trying this out as soon as a suitable 2023.2 preview becomes available.

@isc-bsaviano isc-bsaviano marked this pull request as ready for review May 24, 2023 13:06
@isc-bsaviano isc-bsaviano requested a review from daimor as a code owner May 24, 2023 13:06
@isc-bsaviano
Copy link
Contributor Author

This can be tested with the 2nd 2023.2 developer preview kit (2023.2.0.201.0)

@daimor
Copy link
Member

daimor commented May 24, 2023

That's very interesting

@daimor
Copy link
Member

daimor commented May 24, 2023

Well,
The terminal thing itself looks good, looks working

But there are a few Issues

When connection is disable, it keeps showing in this add terminal menu
image

But, in the list of main commands shown only when any ObjectScript file is open, even if the connection is active

And in the project with a Python virtual environment, made with venv, VSCode activates this virtual environment when opened a new terminal. No idea if it can be configured somehow, probably it's just implemented a way, that it sends this command to any terminal.
image

to reproduce it,
create a new virtual environment in the project and switch to it, VSCode should notice a new environment and offer to switch to it

python -m venv venv
source venv/bin/activate

@isc-bsaviano
Copy link
Contributor Author

@daimor The terminal.profiles contribution point doesn't take a when clause, so I can' hide it when there's no active connection. I could open a VS Code issue but I doubt it would be resolved before this PR is merged. The second image looks like a bug in whatever code sends that command. It should check that the terminal isn't an extension terminal before sending it. I'm not sure where you should report that.

@daimor
Copy link
Member

daimor commented May 24, 2023

well, ok then

daimor
daimor previously approved these changes May 24, 2023
@isc-bsaviano
Copy link
Contributor Author

FYI reported the when clause issue: microsoft/vscode#183354

@gjsjohnmurray
Copy link
Contributor

For the venv issue see microsoft/vscode-python#20562

@isc-bsaviano
Copy link
Contributor Author

@gjsjohnmurray Do you have any feedback before I merge this?

Copy link
Contributor

@gjsjohnmurray gjsjohnmurray left a comment

Choose a reason for hiding this comment

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

Some comments made. I haven't yet had time to install a 2023.2 that I can actually test the terminal with.

@isc-bsaviano isc-bsaviano merged commit 37a3af8 into intersystems-community:master May 25, 2023
@isc-bsaviano isc-bsaviano deleted the terminal branch May 25, 2023 16:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants