Bind Ctrl+Numpad Plus,Minus to the font size controls #9753

hessedoneen · 2021-04-08T23:13:24Z

"ctrl+numpad_plus" command now increases font size and
"ctrl+numpad_minus" command now decreases font size.

Before this only "ctrl+=" and "ctrl+-" controlled font size. Increase in
font size follows previous convention where zooms in arbitrarily large,
but decrease in font size is capped.

Validation Steps Performed

I first ran "ctrl+=" and "ctrl+-" in my terminal to verify its behavior,
then compared that against "ctrl+numpad_plus" and "ctrl+"numpad_minus".
Both increased and decreased the font size by the same amount, and both
appeared to have a cap for how small they could get, but did not appear
to have a cap for how big they could get.

Closes #7518

zadjii-msft · 2021-04-09T10:44:28Z

Yep, I'm pretty sure that's all that needs to be done here. Have you been able to build the Terminal and verify that the numpad keys now work for changing the font size?

hessedoneen · 2021-04-12T01:23:21Z

I am about to write an issue request for testing clarification. I tried to run them with what I saw in:

But they were not passing on my local computer (even before I made any changes). Since this one has passed all your checks, and because it wasn't really any new logic, I am going to make this a full pull request now. I'll definitely be in the issues though asking for some testing help.

zadjii-msft · 2021-04-12T10:05:36Z

Which specific tests didn't pass locally? There's a couple known test failures right now, but this PR certainly shouldn't have caused them.

ghost · 2021-04-12T13:12:27Z

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.

DHowett · 2021-04-12T15:07:21Z

Notes from body:

I think it might be best to put a cap on how large the font can get. Eventually it gets so big you're just zooming in on black space. May be unnecessary considering there seems to be a cap on how small the font can get.

DHowett · 2021-04-12T15:08:08Z

@hessedoneen thanks so much for working on this! 😄 I changed the title and the description of your pull request so that they would show up in our commit history with a more descriptive title.

"ctrl+numpad_plus" command now increases font size and "ctrl+numpad_minus" command now decreases font size. Before this only "ctrl+=" and "ctrl+-" controlled font size. Increase in font size follows previous convention where zooms in arbitrarily large, but decrease in font size is capped. ## Validation Steps Performed I first ran "ctrl+=" and "ctrl+-" in my terminal to verify its behavior, then compared that against "ctrl+numpad_plus" and "ctrl+"numpad_minus". Both increased and decreased the font size by the same amount, and both appeared to have a cap for how small they could get, but did not appear to have a cap for how big they could get. Closes #7518 (cherry picked from commit 9a276c6)

hessedoneen · 2021-04-13T01:24:43Z

Thank you so much! I'll take note and try to make future pulls similar in conciseness / style :)
I was running the tests just now to get all the ones which didn't pass, and I ended up running it in powershell instead and it went a lot better for some reason?

The output from powershell says everything passed, but just has a couple errors:

Invoke-OpenConsoleTests -AllTests
...
Summary: Total=329, Passed=329, Failed=0, Blocked=0, Not Run=0, Skipped=0
Start-Process : This command cannot be run due to the error: The system cannot find the file specified.
At C:\Users\hesse\481\p6-open-source\terminal\tools\OpenConsole.psm1:132 char:5
+     Start-Process $OpenConsolePath -Wait -ArgumentList "powershell.ex ...
+     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidOperation: (:) [Start-Process], InvalidOperationException
    + FullyQualifiedErrorId : InvalidOperationException,Microsoft.PowerShell.Commands.StartProcessCommand

Start-Process : This command cannot be run due to the error: The system cannot find the file specified.
At C:\Users\hesse\481\p6-open-source\terminal\tools\OpenConsole.psm1:132 char:5
+     Start-Process $OpenConsolePath -Wait -ArgumentList "powershell.ex ...
+     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidOperation: (:) [Start-Process], InvalidOperationException
    + FullyQualifiedErrorId : InvalidOperationException,Microsoft.PowerShell.Commands.StartProcessCommand

But there are a lot of failures for my cmd prompt:

runut.cmd
...
Summary of Errors Outside of Tests:
    Error: TAEF: [HRESULT: 0x80070005] A failure occurred while preparing to run tests in 'TerminalApp.LocalTests.dll'. (Failed to load "C:\Users\hesse\481\p6-open-source\terminal\bin\x64\Debug\TestHostApp\TerminalApp.LocalTests.dll" or one of its dependencies.)
    Error: TAEF: [HRESULT: 0x80070005] A failure occurred while preparing to run tests in 'SettingsModel.LocalTests.dll'. (Failed to load "C:\Users\hesse\481\p6-open-source\terminal\bin\x64\Debug\TestHostApp\SettingsModel.LocalTests.dll" or one of its dependencies.)

Summary of Non-passing Tests (showing 20 of 150):
    ApiRoutinesTests::ApiGetConsoleTitleA [Failed]
    ApiRoutinesTests::ApiGetConsoleOriginalTitleA [Failed]
    TerminalAppLocalTests::CommandlineTest::ParseSimpleCommandline [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseTrickyCommandlines [Blocked]
    TerminalAppLocalTests::CommandlineTest::TestEscapeDelimiters [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseSimpleHelp#metadataSet0 [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseSimpleHelp#metadataSet1 [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseSimpleHelp#metadataSet2 [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseSimpleHelp#metadataSet3 [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseBadOptions#metadataSet0 [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseBadOptions#metadataSet1 [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseBadOptions#metadataSet2 [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseSubcommandHelp#metadataSet0 [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseSubcommandHelp#metadataSet1 [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseSubcommandHelp#metadataSet2 [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseSubcommandHelp#metadataSet3 [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseBasicCommandlineIntoArgs [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseNewTabCommand#metadataSet0 [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseNewTabCommand#metadataSet1 [Blocked]
    TerminalAppLocalTests::CommandlineTest::ParseSplitPaneIntoArgs#metadataSet0 [Blocked]

Summary: Total=7098, Passed=6948, Failed=2, Blocked=144, Not Run=0, Skipped=4

runft.cmd:
...
Summary of Non-passing Tests:
    DimensionsTests::TestGetLargestConsoleWindowSize [Failed]
    InputTests::TestMouseWheelReadConsoleMouseInput [Skipped]
    InputTests::TestMouseHorizWheelReadConsoleMouseInput [Skipped]
    InputTests::TestMouseWheelReadConsoleNoMouseInput [Skipped]
    InputTests::TestMouseHorizWheelReadConsoleNoMouseInput [Skipped]
    InputTests::TestMouseWheelReadConsoleInputQuickEdit [Skipped]
    InputTests::TestMouseHorizWheelReadConsoleInputQuickEdit [Skipped]

Summary: Total=406, Passed=399, Failed=1, Blocked=0, Not Run=0, Skipped=6

runuia.cmd
...
Summary of Non-passing Tests:
    Conhost.UIA.Tests.AccessibilityTests.CanAccessAccessibilityTree [Failed]
    Conhost.UIA.Tests.AccessibilityTests.CanCloneTextRangeProvider [Failed]
    Conhost.UIA.Tests.KeyPressTests.VerifyCtrlZCmd [Failed]
    Conhost.UIA.Tests.KeyPressTests.VerifyCtrlHCmd [Failed]
    Conhost.UIA.Tests.KeyPressTests.VerifyCtrlCCmd [Failed]
    Conhost.UIA.Tests.MiscTests.DotNetSetWindowPosition [Failed]
    Conhost.UIA.Tests.MouseWheelTests.TestMouseWheel [Failed]
    Conhost.UIA.Tests.SelectionApiTests.TestCtrlHomeEnd [Failed]
    Conhost.UIA.Tests.SelectionApiTests.TestKeyboardSelection [Failed]
    Conhost.UIA.Tests.SelectionApiTests.TestMouseSelection [Failed]
    Conhost.UIA.Tests.VirtualTerminalTests.RunVtAppTester [Failed]

Summary: Total=12, Passed=1, Failed=11, Blocked=0, Not Run=0, Skipped=0

Is this normal? Which one should I run against for future pull requests do you think?
Thank you so much for your help. This is super exciting, and I appreciate all your support :)

zadjii-msft · 2021-04-13T10:40:10Z

Alright so:

I'm not sure about this powershell error. Never seen anything like that before, but I use the cmd version of the tools so I'm not a lot of help here.
The "local" tests are notoriously finicky. Someone else on the team was hitting a bug just like that recently. You might have a better chance building the TestHostApp project first, but if that doesn't work, don't sweat it. Those don't run in the CI, so we've just got to run them locally every so often to make sure nothing regresses 😂
As far as the feature (runft) and UIA (runuia) tests go - those are also really touchy. you've got to immediately stop using your PC as soon as you start those tests, because they'll drive your mouse, so anything you do will mess with the test. They do run in CI, so I wouldn't be too worried about running them before a PR.

hessedoneen · 2021-04-13T18:30:14Z

This is super helpful feedback :)
I will build TestHostApp first from now on and see if that makes a difference. I look forward to keeping working!

ghost · 2021-04-14T17:09:03Z

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

Handy links:

hessedoneen added 2 commits April 8, 2021 19:05

adding in comment so can draft pull request

555f1d0

numpad zoom key-binding added

7377510

ghost added Area-Settings Issues related to settings and customizability, for console or terminal Issue-Task It's a feature request, but it doesn't really need a major design. Product-Terminal The new Windows Terminal. labels Apr 9, 2021

hessedoneen marked this pull request as ready for review April 12, 2021 01:08

zadjii-msft approved these changes Apr 12, 2021

View reviewed changes

zadjii-msft added AutoMerge Marked for automatic merge by the bot when requirements are met Needs-Second It's a PR that needs another sign-off labels Apr 12, 2021

ghost requested review from miniksa, carlos-zamora, DHowett, leonMSFT and PankajBhojwani April 12, 2021 13:12

DHowett changed the title ~~Draft Pull Request for Issue-7518~~ Bind Ctrl+Numpad Plus,Minus to the font size controls Apr 12, 2021

DHowett approved these changes Apr 12, 2021

View reviewed changes

ghost requested a review from DHowett April 12, 2021 15:07

ghost merged commit 9a276c6 into microsoft:main Apr 12, 2021

DHowett added the zPreview-Service-Queued-1.13 A floating label that tracks the current Preview version for servicing purposes. label Apr 12, 2021

DHowett removed the zPreview-Service-Queued-1.13 A floating label that tracks the current Preview version for servicing purposes. label Apr 13, 2021

hessedoneen deleted the issue-7518 branch April 13, 2021 18:35

ghost mentioned this pull request Apr 14, 2021

Font size can't be changed while using + and - from numeric keypad #7518

Closed

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bind Ctrl+Numpad Plus,Minus to the font size controls #9753

Bind Ctrl+Numpad Plus,Minus to the font size controls #9753

hessedoneen commented Apr 8, 2021 •

edited by DHowett

Loading

zadjii-msft commented Apr 9, 2021

hessedoneen commented Apr 12, 2021

zadjii-msft commented Apr 12, 2021

ghost commented Apr 12, 2021

DHowett commented Apr 12, 2021

DHowett commented Apr 12, 2021

hessedoneen commented Apr 13, 2021

zadjii-msft commented Apr 13, 2021

hessedoneen commented Apr 13, 2021

ghost commented Apr 14, 2021

Bind Ctrl+Numpad Plus,Minus to the font size controls #9753

Bind Ctrl+Numpad Plus,Minus to the font size controls #9753

Conversation

hessedoneen commented Apr 8, 2021 • edited by DHowett Loading

Validation Steps Performed

zadjii-msft commented Apr 9, 2021

hessedoneen commented Apr 12, 2021

zadjii-msft commented Apr 12, 2021

ghost commented Apr 12, 2021

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.

DHowett commented Apr 12, 2021

DHowett commented Apr 12, 2021

hessedoneen commented Apr 13, 2021

zadjii-msft commented Apr 13, 2021

hessedoneen commented Apr 13, 2021

ghost commented Apr 14, 2021

hessedoneen commented Apr 8, 2021 •

edited by DHowett

Loading

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.