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

Make sure we check to profile shortcuts with an index >9 #7344

Merged
merged 2 commits into from
Aug 20, 2020

Conversation

MichelleTanPY
Copy link
Contributor

Summary of the Pull Request

Update profile shortcut based on profiles defined.

  1. Removed hard-coded value (9).

PR Checklist

Validation Steps Performed

profiles
Added more profiles and able to create new tab.
Note: This binding doesn't work tho { "command": { "action": "newTab", "index": 10 }, "keys": "ctrl+shift+0" }

@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-2 A description (P2) labels Aug 19, 2020
Comment on lines 465 to 466
// add the keyboard shortcuts based on the number of profiles defined
if (profileIndex < profileCount)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// add the keyboard shortcuts based on the number of profiles defined
if (profileIndex < profileCount)

Since we're in a loop from 0 to profileCount, we're just guaranteed that this is true. We can simplify this code by removing the check. If you want, you can keep the {} for this block to keep it logically separated from the rest of the code, however.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok sure, make sense. I've removed that condition in the loop and updated the indentation.

@ghost ghost added Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something and removed Needs-Author-Feedback The original author of the issue/PR needs to come back and respond to something labels Aug 20, 2020
1. Remove unnessary conditional check in loop.
2. Update indentation and code comments.
@DHowett
Copy link
Member

DHowett commented Aug 20, 2020

This is lovely. Thanks!

@DHowett DHowett changed the title Update profile shortcut based on profiles defined. Make sure we check to profile shortcuts with an index >9 Aug 20, 2020
@DHowett DHowett merged commit 6f991d3 into microsoft:master Aug 20, 2020
DHowett pushed a commit that referenced this pull request Aug 24, 2020
@ghost
Copy link

ghost commented Aug 26, 2020

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

Handy links:

@ghost
Copy link

ghost commented Aug 26, 2020

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

Handy links:

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 Issue-Bug It either shouldn't be doing this or needs an investigation. Priority-2 A description (P2)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visual Error - shortcut CTRL+SHIFT+[dash,equals]
3 participants