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

Creating a terminal and sending text to it should not show it #11384

Closed
jrieken opened this issue Sep 1, 2016 · 11 comments
Closed

Creating a terminal and sending text to it should not show it #11384

jrieken opened this issue Sep 1, 2016 · 11 comments
Assignees
Labels
api bug Issue identified by VS Code Team member as probable bug important Issue identified as high-priority terminal Integrated terminal issues verified Verification succeeded

Comments

@jrieken
Copy link
Member

jrieken commented Sep 1, 2016

Creating a new terminal immediately shows it which

  • gives me no way to create a terminal in background
  • is different from createOutputChannel or createStatusBarItem
@jrieken jrieken added the bug Issue identified by VS Code Team member as probable bug label Sep 1, 2016
@jrieken jrieken added this to the August 2016 milestone Sep 1, 2016
@jrieken
Copy link
Member Author

jrieken commented Sep 1, 2016

Pushing for August since no one has yet programmed against the API and changes after that won't be possible

@Tyriar Tyriar added the terminal Integrated terminal issues label Sep 1, 2016
@Tyriar
Copy link
Member

Tyriar commented Sep 1, 2016

This is actually a non-trivial change and is related to #11275. A createInBackground?: boolean property can be added to vscode.TerminalConfiguration to keep this backwards compatible.

@Tyriar
Copy link
Member

Tyriar commented Sep 1, 2016

Are you suggesting this should create in background by default? Seems to me that the majority of use cases would want it in the foreground?

@jrieken
Copy link
Member Author

jrieken commented Sep 6, 2016

Are you suggesting this should create in background by default?

For the sake of being consistent with other API (createOutputChannel, createStatusBarItem) "yes". Also it won't require the proposed createInBackground-flag thus keeping it more simple.

@jrieken
Copy link
Member Author

jrieken commented Sep 7, 2016

@Tyriar I would suggest for August to internally call hide after creation. That would align the behaviour, be an easy fix, and the flickering would be along the lines of #11275

@jrieken jrieken added important Issue identified as high-priority api labels Sep 7, 2016
@jrieken
Copy link
Member Author

jrieken commented Sep 7, 2016

Alternatively we release-note this behaviour for August - the API is correct but wrongly implemented. That would mean consumers for 1.5 have to be prepared for a behavioural change with 1.6

@Tyriar Tyriar modified the milestones: August 2016, September 2016 Sep 7, 2016
@Tyriar
Copy link
Member

Tyriar commented Sep 7, 2016

@jrieken while calling hide immediately works for createTerminal, it does not for sendText which also forces the display of the terminal. Changing the latter is a more high risk change, I'm going to make a note in vscode.d.ts indicating that it will be changed as the release notes defer to the docs.

@Tyriar
Copy link
Member

Tyriar commented Sep 7, 2016

Set the PR to August, moving this to September for the real fix.

@Tyriar Tyriar modified the milestones: September 2016, August 2016 Sep 7, 2016
jrieken pushed a commit that referenced this issue Sep 8, 2016
jrieken pushed a commit that referenced this issue Sep 8, 2016
@Tyriar Tyriar changed the title creating a terminal should not show it Creating a terminal and sending text to it should not show it Sep 8, 2016
@Tyriar
Copy link
Member

Tyriar commented Sep 14, 2016

This was done via #11839 and subsequent fixes with one caveat; if the terminal panel is not initialized, the output will not be retained. So the process will run fine, just output will not be kept until Terminal.show is invoked at least once. The upstream issue tracking this is xtermjs/xterm.js#266

@Tyriar Tyriar closed this as completed Sep 14, 2016
@jrieken
Copy link
Member Author

jrieken commented Sep 19, 2016

@Tyriar We still have the doc comments about changes in 1.6 in vscode.d.ts

@Tyriar Tyriar reopened this Sep 19, 2016
@Tyriar Tyriar closed this as completed in dbea7fb Sep 19, 2016
@Tyriar
Copy link
Member

Tyriar commented Sep 19, 2016

Done, I just removed it and didn't elaborate on it not showing the terminal as that's consistent with createOutputChannel.

@bpasero bpasero added the verified Verification succeeded label Sep 30, 2016
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api bug Issue identified by VS Code Team member as probable bug important Issue identified as high-priority terminal Integrated terminal issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

3 participants