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

Integrated terminal should not be resused if a process is running in it #6767

Closed
a1994846931931 opened this issue Dec 18, 2019 · 1 comment · Fixed by #6769
Closed

Integrated terminal should not be resused if a process is running in it #6767

a1994846931931 opened this issue Dec 18, 2019 · 1 comment · Fixed by #6769
Labels
bug bugs found in the application debug issues that related to debug functionality help wanted issues meant to be picked up, require help vscode issues related to VSCode compatibility

Comments

@a1994846931931
Copy link
Contributor

Description

If a process is running in an integrated terminal, then a new integrated terminal should be provided for runInTerminal request. Otherwise, it could be problematic, even dangerous.

Reproduction Steps

integrated-terminal
In the demo above, if the terminal is already running python, sending command text to that terminal will end in chaos.

What's worse? Think about a situation where a user happens to be operating a database, e.g. MySQL, in the opened integrated terminal, it could be dangerous to send commands to that terminal!

Propose

  1. Remove integrated terminals with any child process from for-reuse candidates
    I learned the logic from VSCode. Please refer to:
    https://github.com/microsoft/vscode/blob/4636be2b71c87bfb0bfe3c94278b447a5efcc1f1/src/vs/workbench/api/node/extHostDebugService.ts#L83-L87
    and
    https://github.com/microsoft/vscode/blob/4636be2b71c87bfb0bfe3c94278b447a5efcc1f1/src/vs/workbench/contrib/debug/node/terminals.ts#L50-L75

Demo from VSCode:
integrated-terminal-vscode

  1. Don't change the title of the integrated terminal
    Compare the title of the integrated terminal of VSCode to theia:
    VSCode:
    image
    Theia:
    image
  • In the case of theia, I believe it is hard for users to distinguish the special integrated terminals from the normal ones and they are more likely to run command in that terminal.
  • In the case of VSCode, it's not hard to imagine that users will be aware of which ones are integrated terminal. And even if they mistakenly run a process in that terminal, which ends in prompting VSCode opening a new terminal, they are more likely to understand what's happening and avoid using that terminal the next time.

OS and Theia version:
OS: Linux
Theia: Latest(582ae9d)

Diagnostics:

@akosyakov akosyakov added bug bugs found in the application debug issues that related to debug functionality help wanted issues meant to be picked up, require help vscode issues related to VSCode compatibility labels Dec 18, 2019
@akosyakov
Copy link
Member

@a1994846931931 PRs are welcomed.

a1994846931931 added a commit to a1994846931931/theia that referenced this issue Dec 18, 2019
…d processes

Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
a1994846931931 added a commit to a1994846931931/theia that referenced this issue Dec 18, 2019
…d processes

Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
a1994846931931 added a commit to a1994846931931/theia that referenced this issue Dec 18, 2019
Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
a1994846931931 added a commit to a1994846931931/theia that referenced this issue Dec 18, 2019
…d processes

Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
a1994846931931 added a commit to a1994846931931/theia that referenced this issue Dec 18, 2019
Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
a1994846931931 added a commit to a1994846931931/theia that referenced this issue Dec 19, 2019
…d processes

Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
a1994846931931 added a commit to a1994846931931/theia that referenced this issue Dec 19, 2019
Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
a1994846931931 added a commit that referenced this issue Jan 21, 2020
Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
a1994846931931 added a commit that referenced this issue Jan 21, 2020
Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
akosyakov pushed a commit to akosyakov/theia that referenced this issue Feb 24, 2020
…d processes

Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
akosyakov pushed a commit to akosyakov/theia that referenced this issue Feb 24, 2020
Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
JesterOrNot pushed a commit to JesterOrNot/theia that referenced this issue Mar 12, 2020
…d processes

Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
JesterOrNot pushed a commit to JesterOrNot/theia that referenced this issue Mar 12, 2020
Signed-off-by: Cai Xuye <a1994846931931@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug bugs found in the application debug issues that related to debug functionality help wanted issues meant to be picked up, require help vscode issues related to VSCode compatibility
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants