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

When using the Latest Release of JetBrains IDEs, if the workspace has tasks defined on .gitpod.yml, the IDE will start with one terminal opened for each task #10595

Merged
merged 1 commit into from
Jun 27, 2022

Conversation

felladrin
Copy link
Contributor

@felladrin felladrin commented Jun 10, 2022

Description

When using the Latest Release of JetBrains IDEs, if the workspace has tasks defined on .gitpod.yml, the IDE will start with one terminal opened for each task, behaving similar to VS Code on Gitpod.

When using the Stable Release the behavior remains the current one: one terminal will be displayed, with a welcome message and invoking gp tasks attach.

This separation of versions was required for backward compatibility, as there were breaking changes on the API between IntelliJ 2022.1 and 2022.2. Also, the fix for Shared Terminals Losing Reference was applied only to v2022.2.

Known limitations

  • It's not possible to resize the terminal (max line width) from Thin Client. Resize happens only if the terminal width is changed on the Backend IDE. [1][2][3][4]
  • If we have more than two terminals opened at once, only two of them will be displaying correctly their title in the tab, while the other will be displaying "Local" until the user focuses on them - at this moment the info about the terminal tab is fetched and the tab title is updated. Seems to be a limitation from the backend. I'm going to file an issue for JetBrains and leave the link here later.
  • It's not possible to split shared terminals. This is not on plans to be supported, at this moment.
  • When we stop a workspace, restart, and reconnect to it, if there were terminals running, they will show only the last few lines of the history. The number of history lines displayed is currently hardcoded on JetBrains RemoteDev. The only way to see the full history is by looking at the terminals from the Backend IDE.

To be done in a follow-up PR

  • Update the terminal title whenever there's a change on the title running under Supervisor
  • Close the IDE Terminal tab if Gitpod Terminal was closed remotely and emit a 'close' signal for Gitpod Terminal if user clicks the "X" button to close the IDE Terminal.

Related Issue(s)

Part of #10579

How to test

Release Notes

When using the Latest Release of JetBrains IDEs, if the workspace has tasks defined on .gitpod.yml, the IDE will start with one terminal opened for each task, behaving similar to VS Code on Gitpod.

Werft options:

  • /werft with-preview

@felladrin felladrin self-assigned this Jun 10, 2022
@felladrin felladrin force-pushed the felladrin/10579-multiple-terminals-in-intellij branch 2 times, most recently from ffea919 to be9c325 Compare June 16, 2022 11:04
@felladrin
Copy link
Contributor Author

/hold This PR should only be merged after the Stable version of JetBrains IDEs becomes 2022.2, which contains the fix for https://youtrack.jetbrains.com/issue/CWM-6093.

@felladrin felladrin marked this pull request as ready for review June 16, 2022 17:26
@felladrin felladrin requested a review from a team June 16, 2022 17:26
@gitpod-io gitpod-io deleted a comment from werft-gitpod-dev-com bot Jun 16, 2022
@gitpod-io gitpod-io deleted a comment from werft-gitpod-dev-com bot Jun 16, 2022
@gitpod-io gitpod-io deleted a comment from werft-gitpod-dev-com bot Jun 16, 2022
@akosyakov
Copy link
Member

Should we review it? build seems to be broken

@felladrin
Copy link
Contributor Author

felladrin commented Jun 17, 2022

I've re-run the build, and it's working now.
It's ready for testing. The PR description should have all info needed.

@felladrin felladrin force-pushed the felladrin/10579-multiple-terminals-in-intellij branch 2 times, most recently from e9963af to 99dd2bb Compare June 17, 2022 15:26
@akosyakov
Copy link
Member

akosyakov commented Jun 20, 2022

/werft run

👍 started the job as gitpod-build-felladrin-10579-multiple-terminals-in-intelli.18
(with .werft/ from main)

@akosyakov
Copy link
Member

@felladrin why it does not work in stable anymore? Should not we use conditional compilation to have one version for EAP and another for stable? see how it is done in Rust: https://github.com/intellij-rust/intellij-rust/blob/eefa4f43e19e5dc6e3923e99478b504bdec24028/build.gradle.kts#L388-L398

@akosyakov
Copy link
Member

akosyakov commented Jun 20, 2022

@felladrin I was trying with https://github.com/akosyakov/parcel-demo and for some reasons the second terminal is Local. I need to click to see real terminal label. Can we update them dynamically? Also after clicking it shows outdated title, i.e. Gitpod Task2: idea-cli while a terminal does not run anything anymore.

I'm also not sure about welcome terminal. I'm kind of liking it, but at the same time maybe a user wants to see the last terminal.

@akosyakov
Copy link
Member

Also explicit closing of the frontend terminal still does not trigger closing of the underlying supervisor task. We should make sure that it works. Do we track it already for JB remote dev? @felladrin

@felladrin
Copy link
Contributor Author

@felladrin I was trying with https://github.com/akosyakov/parcel-demo and for some reasons the second terminal is Local. I need to click to see real terminal label. Can we update them dynamically?

I tried. Checked every method available inside inside terminalWidget.terminalTitle.change and it didn't work. Seems to be a limitation from the backend, which only fetches the info from terminal when the Thin Client focus on it.

I've also tried using terminalWidget.grabFocus() (also ith ClientId.withClientId() to force executing on the Thin Client), but it didn't work to programmatically focus on the terminal to fetch the info.
I'm going to file an issue for JetBrains today.

I'm also not sure about welcome terminal. I'm kind of liking it, but at the same time maybe a user wants to see the last terminal.

I agree! Let me remove it.

@felladrin
Copy link
Contributor Author

Also explicit closing of the frontend terminal still does not trigger closing of the underlying supervisor task. We should make sure that it works. Do we track it already for JB remote dev? @felladrin

Hmm, it was not implemented this time.

I'll put this PR back to Draft state while I implement it with the following specs:

  • If we run 'exit' we need to react to the Supervisor info that the terminal was closed, then we close JetBrians Terminal.
  • If we click X button to close the terminal, we intercept the JetBrains Terminal Closing action and emit to Supervisor a close command.

@felladrin
Copy link
Contributor Author

Also after clicking it shows outdated title, i.e. Gitpod Task2: idea-cli while a terminal does not run anything anymore.

Good point. I need to set up a listener for terminal title changes.
Let me add it.

@felladrin
Copy link
Contributor Author

@felladrin why it does not work in stable anymore? Should not we use conditional compilation to have one version for EAP and another for stable? see how it is done in Rust: https://github.com/intellij-rust/intellij-rust/blob/eefa4f43e19e5dc6e3923e99478b504bdec24028/build.gradle.kts#L388-L398

Thanks for sharing! It could work for conditionally using terminalWidget.terminalTitle.change (which was only added on 2022.2).

But terminals losing reference is not related to the plugin SDK, but to the Backend IDE compilation. And the fix is only available in Backend IDE 2022.2+ builds.
So even if we compile the backend-plugin using the 2022.2 SDK, the plugin can still run in 2022.1 IDE if we don't use any API added on 2022.2, but issues from the Backend IDE remain there.

@felladrin
Copy link
Contributor Author

felladrin commented Jun 20, 2022

I thought that we should have 2 different source sets one for stable backend without using the fix, and another for version with a fix. What I'm missing?

💡Got it!
The one for the stable version would still display the Welcome Message in a single shared terminal, while the other, for EAP, would display one terminal for each task (without the welcome message), right?
Let me try.

@akosyakov
Copy link
Member

akosyakov commented Jun 20, 2022

💡Got it!
The one for the stable version would still display the Welcome Message in a single shared terminal, while the other, for EAP, would display one terminal for each task (without the welcome message), right?
Let me try.

Yes, we should avoid long standing PRs. We should have one plugin which can work in both versions at any time and after new release we just need to clean up old version code.

@felladrin felladrin force-pushed the felladrin/10579-multiple-terminals-in-intellij branch from 9549e89 to f23e6e9 Compare June 21, 2022 08:33
@roboquat roboquat added size/XL and removed size/L labels Jun 21, 2022
@akosyakov
Copy link
Member

We agreed that we should separate that this logic is only applied if latest is toggled for now, i.e. experimentation phase in [1]. After that it can be merged and worked on with follow-up PRs.

@felladrin felladrin force-pushed the felladrin/10579-multiple-terminals-in-intellij branch 4 times, most recently from d5036b4 to a8307db Compare June 22, 2022 11:44
@gitpod-io gitpod-io deleted a comment from werft-gitpod-dev-com bot Jun 22, 2022
@felladrin felladrin marked this pull request as ready for review June 22, 2022 16:10
@felladrin
Copy link
Contributor Author

Alright, code updated, along with the description for the PR and the "how to test" section.
The new implementation will be available only when the "Latest Release (Unstable)" box is checked on the Gitpod Preferences page.
If it's good to be merged, we just need to unhold the PR.

@felladrin felladrin requested a review from akosyakov June 22, 2022 16:15
@felladrin felladrin changed the title Display a terminal for each Gitpod Task when running on JetBrains IDEs When using the Latest Release of JetBrains IDEs, if the workspace has tasks defined on .gitpod.yml, the IDE will start with one terminal opened for each task Jun 22, 2022
… tasks defined on .gitpod.yml, the IDE will start with one terminal opened for each task
@felladrin felladrin force-pushed the felladrin/10579-multiple-terminals-in-intellij branch from ab3f2ca to 0229fdc Compare June 22, 2022 16:22
@github-actions
Copy link
Contributor

⚠️ Hey reviewer! BE CAREFUL ⚠️
Review the code before opening in your Gitpod. .gitpod.yml was changed and it might be harmful.

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

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

It is great work! Thank you for such nice separation.

Resolves #10579

I don't think it does. We should do all follow-up PRs, right? And address outstanding usability issues?

@felladrin
Copy link
Contributor Author

It is great work! Thank you for such nice separation.

Resolves #10579

I don't think it does. We should do all follow-up PRs, right? And address outstanding usability issues?

Thank you!
I've updated the description and unlinked the issue, so it won't be closed when this PR gets merged.

@felladrin
Copy link
Contributor Author

/unhold

@roboquat roboquat merged commit 41efa4a into main Jun 27, 2022
@roboquat roboquat deleted the felladrin/10579-multiple-terminals-in-intellij branch June 27, 2022 07:32
@roboquat roboquat added deployed: IDE IDE change is running in production deployed Change is completely running in production labels Jun 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: IDE IDE change is running in production deployed Change is completely running in production editor: jetbrains release-note size/XL team: IDE
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants