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

Minimal set of Theia terminal improvements for Che 7 #11789

Closed
6 of 7 tasks
l0rd opened this issue Oct 31, 2018 · 16 comments
Closed
6 of 7 tasks

Minimal set of Theia terminal improvements for Che 7 #11789

l0rd opened this issue Oct 31, 2018 · 16 comments
Assignees
Labels
kind/enhancement A feature request - must adhere to the feature request template.

Comments

@l0rd
Copy link
Contributor

l0rd commented Oct 31, 2018

Description

Here is a small list of desiderata for Che 7 regarding the terminal:

  • The command name should be "Open Terminal in specific container" instead of "Open new multi-machine terminal"
  • Add a new "Open Terminal" command that should automatically pick the first container among the ones created by the user without prompting for a choice Add as many "Open Terminal" commands as there are containers (but filter out the exec plugin container). So user could select directly from the command palette the terminal to open:
  • "Open Terminal" and "Open Terminal in specific container" should appear in Theia View menu ([menu] [terminal] align terminal menu with vscode eclipse-theia/theia#3356)
  • Use /etc/passwd shell field to select the terminal's shell. Use /usr/bin/sh if the shell is not specified (Add shell probe che-machine-exec#7)
  • The terminal panel should be closed when the user exit

cc @evidolob @ashumilova @slemeur @AndrienkoAleksandr

@l0rd l0rd added kind/enhancement A feature request - must adhere to the feature request template. team/ide2 labels Oct 31, 2018
@benoitf
Copy link
Contributor

benoitf commented Oct 31, 2018

@AndrienkoAleksandr
Copy link
Contributor

@l0rd why do we need move commands "Open terminal" to the View menu? View menu stores list "layouts"(panels). Layout it's a bigger UI scope than terminal widget. Terminal widget located on the bottom panel. So moving this command brakes this logical structure. As for me File menu is better place.

@l0rd
Copy link
Contributor Author

l0rd commented Oct 31, 2018

Thank you @benoitf I added the link for the /etc/passwd one. For https://github.com/eclipse/che-theia-terminal-extension/issues/15: I am not sure that's the same. What I would like is that, even if there are many machines (aka containers), we automatically select the one that makes more sense (the first dev container).

@benoitf
Copy link
Contributor

benoitf commented Oct 31, 2018

It may be disruptive that in upstream Theia, Terminal is in another menu (maybe need to change it as well in upstream)

@benoitf
Copy link
Contributor

benoitf commented Oct 31, 2018

@l0rd may replace "first container" by "container specified by the user" (not picking up one of the tooling container)

@l0rd
Copy link
Contributor Author

l0rd commented Oct 31, 2018

@AndrienkoAleksandr that's a feedback from eclipsecon. People could not find the terminal because they looked for the terminal where they usually find it for vscode:

image

@l0rd
Copy link
Contributor Author

l0rd commented Oct 31, 2018

@l0rd may replace "first container" by "container specified by the user" (not picking up one of the tooling container)

You are right. This is more clear. But there is the case where the user has specified multiple containers. So maybe "the first container among the ones specified by the user"?

@AndrienkoAleksandr AndrienkoAleksandr self-assigned this Oct 31, 2018
@benoitf
Copy link
Contributor

benoitf commented Oct 31, 2018

@l0rd @AndrienkoAleksandr about terminal there is an ongoing discussion in theia there eclipse-theia/theia#3355 (comment)

@AndrienkoAleksandr
Copy link
Contributor

AndrienkoAleksandr commented Nov 1, 2018

@lord

But there is the case where the user has specified multiple containers. So maybe "the first container among the ones specified by the user"?
What does mean "specified" by the user? Where is place to specified

Seems for now we can't implement this enhancement. Because we doesn't mark with help api containers like "tooling" or "user". So we can't even filter user containers among of others.
Issue #11804

@AndrienkoAleksandr
Copy link
Contributor

AndrienkoAleksandr commented Nov 2, 2018

Upstream related pull request:
Create terminal main-menu item

@AndrienkoAleksandr
Copy link
Contributor

AndrienkoAleksandr commented Nov 2, 2018

@lord @benoitf about

items "Open Terminal" and "Open Terminal in specific container" should appear in Theia View menu.

I think It should not be there.
Yes, vscode has command "Terminal", image from @lord:

image

But this command has another behavior. This command works with bottom panel and sections of this panel. Bottom panel inside vscode has Sections:

  • Terminal
  • Output
  • Problems
  • and so on...

exec-terminal-contribution ts working tree - che-theia-terminal-extension - visual studio code_002

When you click in the "View" command "Terminal", this command open/hide bottom panel and select section "Terminal". In case if this panel is empty, without terminals, this command really create new one terminal.
Theia doesn't has sections on the bottom panel. And Seems Theia doesn't need them, because it has another one feauture: it's ability to move each widget in the bottom panel to the any place on the IDE:
Flexible Window Layout in Theia IDE

So, what do you think?

@l0rd
Copy link
Contributor Author

l0rd commented Nov 2, 2018

@AndrienkoAleksandr the View menu item is an upstream problem and is tracked/discussed here. As said there I don't have a strong opinion (i.e. I won't spend hours discussing that)

On the other end the second point (automatically pick the first container among the ones created by the user) is really important. Since there is no better option now please use name=='dev' as filter. Another thing that would be cool is if you could make this filter something that is possible to overwrite with an environment variable (e.g. if DEFAULT_CONTAINER_FILTER="..." is set the filter is overwritten by the variable value).

@AndrienkoAleksandr
Copy link
Contributor

Hello, @l0rd . I implemented Add as many "Open Terminal" commands as there are containers. It's looks like:
terminal-command-per-container
Skipping plugin containers will be done later, when we will resolve filtering issue. Label New terminal for containerName of the command is OK?

@slemeur
Copy link
Contributor

slemeur commented Jan 11, 2019 via email

@l0rd
Copy link
Contributor Author

l0rd commented Jan 11, 2019

@AndrienkoAleksandr really cool thank you! I've just used it and it significantly improves the UX.

@AndrienkoAleksandr
Copy link
Contributor

AndrienkoAleksandr commented Feb 18, 2019

All sub-items done, except:
"Open Terminal" and "Open Terminal in specific container" should appear in TheiaViewmenu - theia community against this upstream change:
eclipse-theia/theia#3356 (comment)
eclipse-theia/theia#3359 (comment)
eclipse-theia/theia#4002
And discussion here https://spectrum.chat/theia?thread=421d2943-2386-49dd-ae42-e005e55d6a36

And vscode has a plan in 2019 year road map to cover few new features and change user interface - it become closer to Theia.
See more:
Tabs for integrated terminal -> https://github.com/Microsoft/vscode/wiki/Roadmap#ux -> Use tabs instead of the terminal dropdown
Allow for floating windows -> https://github.com/Microsoft/vscode/wiki/Roadmap#workbench -> Support for detachable workbench parts -> after that I suppose View menu will be changed.

So, seems, it's no make sense to try handle this item, this item related to the current ui concept, but not feature concept.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A feature request - must adhere to the feature request template.
Projects
None yet
Development

No branches or pull requests

4 participants