Skip to content

Conversation

Lixire
Copy link
Contributor

@Lixire Lixire commented Aug 8, 2017

Fixes an issue where if you delete a terminal that is not the last one and you also have the active terminal as the last one, the index doesn't get updated and the terminal panel title is blank.

Steps to reproduce:

  1. Create three terminals and focus the last one
  2. Delete the second one through the quick picker
  3. Observe that the panel title is blank

@Lixire Lixire added terminal General terminal issues that don't fall under another label quick-pick Quick-pick widget issues labels Aug 8, 2017
@Lixire Lixire added this to the August 2017 milestone Aug 8, 2017
@Lixire Lixire self-assigned this Aug 8, 2017
@Lixire Lixire requested a review from roblourens August 8, 2017 23:01
@@ -95,6 +95,9 @@ export class QuickKillTerminalAction extends Action {
if (terminal) {
terminal.dispose();
}
if (this.terminalService.activeTerminalInstanceIndex !== terminalIndex) {
this.terminalService.setActiveInstanceByIndex(Math.min(terminalIndex, this.terminalService.terminalInstances.length - 1));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it a problem if there's only one terminal, and you delete it? Does it call this with -1?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works as expected, but we probably should check for that case

@roblourens
Copy link
Member

I just realized that I get a green square for reviewing a PR, but not for merging it.

@roblourens roblourens merged commit bac7689 into master Aug 9, 2017
@Lixire Lixire mentioned this pull request Aug 10, 2017
6 tasks
@Lixire Lixire deleted the amqi/terminal-quickopen branch August 16, 2017 17:41
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
quick-pick Quick-pick widget issues terminal General terminal issues that don't fall under another label
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants