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.

Sign up for GitHub

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

Jump to bottom

ShellTerminalProfile modify method broken #12197

Closed
colin-grant-work opened this issue Feb 17, 2023 · 4 comments · Fixed by #12199
Closed

ShellTerminalProfile modify method broken #12197

colin-grant-work opened this issue Feb 17, 2023 · 4 comments · Fixed by #12199
Assignees
@tsmaeder
Labels
bug bugs found in the application terminal issues related to the terminal

Comments

@colin-grant-work
Copy link
Contributor

colin-grant-work commented Feb 17, 2023 •
edited
Loading

The type accepted by modify is basically used as a constructor argument for a new ShellTerminalProfile, but the types accepted by the modify method and the the constuctor of ShellTerminalProfile have almost nothing in common. The clearest manifestation of the bugs this causes is that opening a terminal in a multi-root workspace ignores the user's selection of root.

Originally posted by @colin-grant-work in #12066 (comment)
@colin-grant-work colin-grant-work added bug bugs found in the application terminal issues related to the terminal labels Feb 17, 2023
@colin-grant-work
Copy link
Contributor Author

@tsmaeder, @JonasHelming, this may be worth considering for a hot fix for the community release.

@tsmaeder tsmaeder self-assigned this Feb 20, 2023
@tsmaeder
Copy link
Contributor

tsmaeder commented Feb 20, 2023 •
edited
Loading

@colin-grant-work ignoring the selected workspace root is the only manifestation of the bug I can see in the Theia code base. Not sure that warrants a bugfix release. Let's discuss severity.

tsmaeder added a commit to tsmaeder/theia that referenced this issue Feb 20, 2023
@tsmaeder
Correctly pass selected workspace root to terminal profile
1c11c52 
Fixes eclipse-theia#12197

Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
@tsmaeder tsmaeder mentioned this issue Feb 20, 2023
Merged
1 task
@colin-grant-work
Copy link
Contributor Author

colin-grant-work commented Feb 20, 2023 •
edited
Loading

@tsmaeder, I would agree it isn't very severe, but it's a regression that is obvious to users and occurs in a not-uncommon scenario. In my view, that's exactly the sort of thing we want to avoid in the community releases.

@JonasHelming
Copy link
Contributor

It is on the agenda for tomorrow: https://github.com/eclipse-theia/theia/wiki/Dev-Meetings

tsmaeder added a commit that referenced this issue Feb 21, 2023
@tsmaeder
Correctly pass selected workspace root to terminal profile (#12199)
5611541 
Fixes #12197

Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
@tsmaeder tsmaeder mentioned this issue Feb 23, 2023
Closed
tsmaeder added a commit to tsmaeder/theia that referenced this issue Feb 24, 2023
@tsmaeder
Correctly pass selected workspace root to terminal profile
5da2c2c 
Fixes eclipse-theia#12197

Contributed on behalf of STMicroelectronics

Signed-off-by: Thomas Mäder <t.s.maeder@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@tsmaeder tsmaeder

Labels
bug bugs found in the application terminal issues related to the terminal
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Correctly pass selected workspace root to terminal profile tsmaeder/theia
3 participants
@JonasHelming @tsmaeder @colin-grant-work