closeTab warning (When more then one pane is open)

[Chips1234]

Summary of the Pull Request

Presents a dialogue when more then one pane is open.

References

Connected to #5301

PR Checklist

• Closes closeTab should present a confirmation dialog #5301
• CLA signed. If not, go over here and sign the CLA
• No docs!
• I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

I can't seem to figure out the if statement (_CloseFocusTabPane) so if anyone can help, that would be great!

[Chips1234]

Again, I can't figure out why there are unrelated commits.

[carlos-zamora]

Here's a few comments to help you out 😊

[carlos-zamora]

oops. accidentally approved. ugh

[zadjii-msft]

Largely, I agree with everything that @carlos-zamora mentioned.

[carlos-zamora]

Alright, a few things are going on here:

• you accidentally copied the method header for _GetActiveControl() and replaced the one for _CloseFocusedTabPane()
• You haven't exposed Tab::_GetLeafPaneCount() as public yet
• Your _CloseTabWarningPrimaryButtonOnClick() calls _CloseFocusedTab() instead of _CloseFocusedTabPane()

If you fix those three things, I think you should be in a position where this works (functionally speaking). There's still some more code-health related things that would be nice to fix but they'll be found during a code review. Let's just focus on getting this working first 😊

Idk if you'll have to expose _GetLeafPaneCount() in the idl file. But if you start noticing that you've exposed it as public in Tab and you still can't call it from TerminalPage, that might be a problem.

[Chips1234]

Okay thanks. I’ll look into it.

[DHowett]

Sure! The click on the Close button is handled in TerminalPage::_OnTabCloseRequested. That function is called when the user wants to close a tab with the [x].

[Chips1234]

Alright. Thanks. From: Dustin L. Howett<mailto:[email protected]> Sent: May 19, 2020 20:31 To: microsoft/terminal<mailto:[email protected]> Cc: Eric Tian<mailto:[email protected]>; Author<mailto:[email protected]> Subject: Re: [microsoft/terminal] closeTab warning (When more then one pane is open) (#5874) Sure! The click on the Close button is handled in TerminalPage::_OnTabCloseRequested. That function is called when the user wants to close a tab with the [x].

[Chips1234]

Can anyone review this? @zadjii-msft @carlos-zamora? Thx!

[Chips1234]

Hello?

[zadjii-msft]

I'm blocking this based off of two concerns:

1. Primarily, there need to be a couple code fixes here.
2. I think before this merges I want to have a resolution of how a setting to disable this would work. I've discussed this a bit more in Prompt before closing pane with multiple client apps #6549 (comment), but not really taken to any conclusions. I don't think that you need to be the person to implement all the appropriate settings, but I want to make sure that we've got our eye on that before we merge this.

[Chips1234]

2. I think before this merges I want to have a resolution of how a setting to disable this would work. I've discussed this a bit more in #6549 (comment), but not really taken to any conclusions. I don't think that you need to be the person to implement all the appropriate settings, but I want to make sure that we've got our eye on that before we merge this.

Definitely

[github-actions]

New misspellings found, please review:

• mutiple

To accept these changes, run the following commands

perl -e '
my @expect_files=qw('".github/actions/spell-check/whitelist/alphabet.txt
.github/actions/spell-check/whitelist/web.txt
.github/actions/spell-check/whitelist/whitelist.txt"');
@ARGV=@expect_files;
my @stale=qw('"bitfield Emojis HREF memcpying OUTPATHROOT textblock usr vpack "');
my $re=join "|", @stale;
my $suffix=".".time();
my $previous="";
sub maybe_unlink { unlink($_[0]) if $_[0]; }
while (<>) {
  if ($ARGV ne $old_argv) { maybe_unlink($previous); $previous="$ARGV$suffix"; rename($ARGV, $previous); open(ARGV_OUT, ">$ARGV"); select(ARGV_OUT); $old_argv = $ARGV; }
  next if /^($re)(?:$| .*)/; print;
}; maybe_unlink($previous);'
perl -e '
my $new_expect_file=".github/actions/spell-check/whitelist/4af3c8f4e9a27b86ef7126b1d30138f37487934a.txt";
open FILE, q{<}, $new_expect_file; chomp(my @words = <FILE>); close FILE;
my @add=qw('"mutiple "');
my %items; @items{@words} = @words x (1); @items{@add} = @add x (1);
@words = sort {lc($a) cmp lc($b)} keys %items;
open FILE, q{>}, $new_expect_file; for my $word (@words) { print FILE "$word\n" if $word =~ /\w/; };
close FILE;'
git add .github/actions/spell-check/whitelist || echo '... you want to ensure .github/actions/spell-check/whitelist/4af3c8f4e9a27b86ef7126b1d30138f37487934a.txt is added to your repository...'

✏️ Contributor please read this

• If the items listed above are names, please add them to .github/actions/spell-check/dictionary/names.txt.
• If they're APIs, you can add them to a file in .github/actions/spell-check/dictionary/.
• If they're just things you're using, please add them to an appropriate file in .github/actions/spell-check/whitelist/.
• If you need to use a specific token in one place and it shouldn't generally be used, you can
  add an item in an appropriate file in .github/actions/spell-check/patterns/.

See the README.md in each directory for more information.

⚠️ Reviewers

At present, the action that triggered this message will not show its ❌ in this PR unless the branch is within this repository.
Thus, you should make sure that this comment has been addressed before encouraging the merge bot to merge this PR.

[Chips1234]

@zadjii-msft @carlos-zamora Okay, this is ready for review. Thanks

[Chips1234]

@carlos-zamora @zadjii-msft Alright. I think we're all set.

[Chips1234]

Since the commits are a mess and I don't like it, and because it's inactive, I'll close this pr and make a new one later.

[Chips1234]

This has now moved to #7077