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

Unfriendly warning when terminals cannot be split #59309

Closed
jrieken opened this issue Sep 25, 2018 · 5 comments · Fixed by #59413
Closed

Unfriendly warning when terminals cannot be split #59309

jrieken opened this issue Sep 25, 2018 · 5 comments · Fixed by #59413
Assignees
Labels
terminal Integrated terminal issues

Comments

@jrieken
Copy link
Member

jrieken commented Sep 25, 2018

re #59266

  • Keep pressing the split button
  • At one point the warning shows

I think we can do better than that... Why don't we disable the action instead of showing the warning? Also, why is this a warning and not an information message?

screen shot 2018-09-25 at 11 10 18

@bpasero
Copy link
Member

bpasero commented Sep 25, 2018

Yeah also found this a bit weird. Fyi we do not prevent splitting of editors in the grid widget and have not heard complaints so far. So I am not sure why we need it here (cc @Tyriar).

@alexr00
Copy link
Member

alexr00 commented Sep 25, 2018

We had one complaint about infinite splitting which is why I added this. I agree that behavior should generally be consistent between editor and terminal for splitting but at the same time allowing the user to put their terminal into a broken state isn't great either.

I was concerned that simply disabling the split operation wouldn't make why it's disabled clear, but it is more intrusive than just disabling it!

This issue has already gotten more thumbs ups than the original (#45678), so clearly something needs to be done 😄 @Tyriar, let me know what you think!

@bpasero
Copy link
Member

bpasero commented Sep 25, 2018

I think disabling the split action when we think it should be prevented is the best solution.

@Tyriar
Copy link
Member

Tyriar commented Sep 25, 2018

@alexr00 I guess we could watch for when the panel resizes and disable/enable if there's room.

@alexr00
Copy link
Member

alexr00 commented Sep 26, 2018

I've created PR for changing the warn to info. The disable/enable for split is more complicated so it would be better to do during the next milestone. Issue #59414 tracks that!

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 10, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
terminal Integrated terminal issues
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants