Skip to content

Ignore newTab actions with a profile index greater than the number of profiles#11621

Merged
3 commits merged intomainfrom
dev/migrie/b/11114-proposal
May 5, 2022
Merged

Ignore newTab actions with a profile index greater than the number of profiles#11621
3 commits merged intomainfrom
dev/migrie/b/11114-proposal

Conversation

@zadjii-msft
Copy link
Member

Summary of the Pull Request

As discussed in team sync. Ignore newTab actions with a profile index greater than the number of profiles.

PR Checklist

@ghost ghost added Area-Settings Issues related to settings and customizability, for console or terminal Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-3 A description (P3) Product-Terminal The new Windows Terminal. labels Oct 26, 2021
@DHowett DHowett added the Needs-Second It's a PR that needs another sign-off label Oct 26, 2021
@DHowett
Copy link
Member

DHowett commented Oct 26, 2021

Sure, let's do it!

{
if (index >= _settings->ActiveProfiles().Size())
{
args.Handled(false);

Choose a reason for hiding this comment

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

What's the effect of false here? I would greatly prefer that the set of key combinations passed to the conpty not depend on how many profiles are defined.

@zadjii-msft zadjii-msft self-assigned this Jan 4, 2022
@zadjii-msft zadjii-msft added the AutoMerge Marked for automatic merge by the bot when requirements are met label May 5, 2022
@ghost
Copy link

ghost commented May 5, 2022

Hello @zadjii-msft!

Because this pull request has the AutoMerge label, I will be glad to assist with helping to merge this pull request once all check-in policies pass.

p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (@msftbot) and give me an instruction to get started! Learn more here.

@zadjii-msft
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ghost ghost merged commit ce850ca into main May 5, 2022
@ghost ghost deleted the dev/migrie/b/11114-proposal branch May 5, 2022 15:23
carlos-zamora pushed a commit that referenced this pull request May 12, 2022
…of profiles (#11621)

## Summary of the Pull Request

As discussed in team sync. Ignore `newTab` actions with a profile index greater than the number of profiles.

## PR Checklist
* [x] Closes #11114
* [x] I work here
* [ ] Tests added/passed
* [ ] Requires documentation to be updated - maybe❓

(cherry picked from commit ce850ca)
Service-Card-Id: 81744839
Service-Version: 1.13
@ghost
Copy link

ghost commented May 24, 2022

🎉Windows Terminal v1.13.1143 has been released which incorporates this pull request.:tada:

Handy links:

@ghost
Copy link

ghost commented May 24, 2022

🎉Windows Terminal Preview v1.14.143 has been released which incorporates this pull request.:tada:

Handy links:

This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Settings Issues related to settings and customizability, for console or terminal AutoMerge Marked for automatic merge by the bot when requirements are met Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Second It's a PR that needs another sign-off Priority-3 A description (P3) Product-Terminal The new Windows Terminal.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

newTab with out-of-range profile index uses the default profile

5 participants