From 05c712c8700c35835497bb92fb740614657d6a85 Mon Sep 17 00:00:00 2001 From: KavishaHaswani <141186389+KavishaHaswani@users.noreply.github.com> Date: Sat, 4 Oct 2025 10:21:16 +0530 Subject: [PATCH 1/7] Fix initial position parsing from -pos tag (#4247) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This update modifies how the -pos tag is parsed when setting the initial window position. Specifically: - A single value like "number" now sets both x and y positions to that value — similar to how padding behaves. - A value like "number," (with a trailing comma) continues to set only x, leaving y unchanged. Changes: - Updated parsing logic for -pos tag - Added unit test to verify single-value behavior - Confirmed all existing tests pass --- .../spec.md | 1 + .../LocalTests_TerminalApp/SettingsTests.cpp | 49 ++++++++++++++++++- .../ModelSerializationHelpers.h | 4 ++ 3 files changed, 53 insertions(+), 1 deletion(-) diff --git a/doc/specs/#1043 - Set the initial position of the Terminal/spec.md b/doc/specs/#1043 - Set the initial position of the Terminal/spec.md index a6dcc7fd7b6..9d7966a491d 100644 --- a/doc/specs/#1043 - Set the initial position of the Terminal/spec.md +++ b/doc/specs/#1043 - Set the initial position of the Terminal/spec.md @@ -27,6 +27,7 @@ Two new properties should be added in the json settings file: 2. Both X value and Y values are optional. If any one of them is missing, or the value is invalid, system default value will be used. Examples: + "1000" equals (1000, 1000) ", 1000" equals (default, 1000) "1000, " equals (1000, default) "," equals (default, default) diff --git a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp index 67b6a174666..efa760a20db 100644 --- a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp @@ -73,6 +73,8 @@ namespace TerminalAppLocalTests TEST_METHOD(TestElevateArg); + TEST_METHOD(TestInitialPositionParsing); + TEST_CLASS_SETUP(ClassSetup) { return true; @@ -1619,4 +1621,49 @@ namespace TerminalAppLocalTests } } -} + void SettingsTests::TestInitialPositionParsing() + { + { + const auto pos = LaunchPositionFromString("50"); + VERIFY_IS_TRUE(pos.X.has_value()); + VERIFY_IS_TRUE(pos.Y.has_value()); + VERIFY_ARE_EQUAL(50, pos.X.Value()); + VERIFY_ARE_EQUAL(50, pos.Y.Value()); + } + + { + const auto pos = LaunchPositionFromString("100,"); + VERIFY_IS_TRUE(pos.X.has_value()); + VERIFY_ARE_EQUAL(100, pos.X.Value()); + VERIFY_IS_FALSE(pos.Y.has_value()); + } + + { + const auto pos = LaunchPositionFromString(",100"); + VERIFY_IS_FALSE(pos.X.has_value()); + VERIFY_IS_TRUE(pos.Y.has_value()); + VERIFY_ARE_EQUAL(100, pos.Y.Value()); + } + + { + const auto pos = LaunchPositionFromString("50,50"); + VERIFY_IS_TRUE(pos.X.has_value()); + VERIFY_ARE_EQUAL(50, pos.X.Value()); + VERIFY_IS_TRUE(pos.Y.has_value()); + VERIFY_ARE_EQUAL(50, pos.Y.Value()); + } + + { + const auto pos = LaunchPositionFromString("abc,100"); + VERIFY_IS_FALSE(pos.X.has_value()); + VERIFY_IS_TRUE(pos.Y.has_value()); + VERIFY_ARE_EQUAL(100, pos.Y.Value()); + } + + { + const auto pos = LaunchPositionFromString("abc"); + VERIFY_IS_FALSE(pos.X.has_value()); + VERIFY_IS_FALSE(pos.Y.has_value()); + } + } +} \ No newline at end of file diff --git a/src/cascadia/TerminalSettingsModel/ModelSerializationHelpers.h b/src/cascadia/TerminalSettingsModel/ModelSerializationHelpers.h index d8c9201f662..961d523399a 100644 --- a/src/cascadia/TerminalSettingsModel/ModelSerializationHelpers.h +++ b/src/cascadia/TerminalSettingsModel/ModelSerializationHelpers.h @@ -67,6 +67,10 @@ _TIL_INLINEPREFIX ::winrt::Microsoft::Terminal::Settings::Model::LaunchPosition string, [&initialPosition](int32_t left) { initialPosition.X = left; }, [&initialPosition](int32_t right) { initialPosition.Y = right; }); + if (initialPosition.X && !initialPosition.Y && string.find(',') == std::string::npos) + { + initialPosition.Y = initialPosition.X; + } return initialPosition; } From d1cf6fdea2e5748c36c3cb46edab98bcd808d37e Mon Sep 17 00:00:00 2001 From: KavishaHaswani <141186389+KavishaHaswani@users.noreply.github.com> Date: Tue, 7 Oct 2025 01:30:22 +0530 Subject: [PATCH 2/7] Fix missing include and type mismatch in LaunchPosition tests --- .../LocalTests_TerminalApp/SettingsTests.cpp | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp index efa760a20db..42a39cb6256 100644 --- a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp @@ -6,6 +6,7 @@ #include "../TerminalApp/TerminalPage.h" #include "../UnitTests_SettingsModel/TestUtils.h" #include "../TerminalSettingsAppAdapterLib/TerminalSettings.h" +#include "../TerminalSettingsModel/ModelSerializationHelpers.h" using namespace Microsoft::Console; using namespace WEX::Logging; @@ -1627,14 +1628,14 @@ namespace TerminalAppLocalTests const auto pos = LaunchPositionFromString("50"); VERIFY_IS_TRUE(pos.X.has_value()); VERIFY_IS_TRUE(pos.Y.has_value()); - VERIFY_ARE_EQUAL(50, pos.X.Value()); - VERIFY_ARE_EQUAL(50, pos.Y.Value()); + VERIFY_ARE_EQUAL(50, static_castpos.X.Value()); + VERIFY_ARE_EQUAL(50, static_castpos.Y.Value()); } { const auto pos = LaunchPositionFromString("100,"); VERIFY_IS_TRUE(pos.X.has_value()); - VERIFY_ARE_EQUAL(100, pos.X.Value()); + VERIFY_ARE_EQUAL(100, static_castpos.X.Value()); VERIFY_IS_FALSE(pos.Y.has_value()); } @@ -1642,22 +1643,22 @@ namespace TerminalAppLocalTests const auto pos = LaunchPositionFromString(",100"); VERIFY_IS_FALSE(pos.X.has_value()); VERIFY_IS_TRUE(pos.Y.has_value()); - VERIFY_ARE_EQUAL(100, pos.Y.Value()); + VERIFY_ARE_EQUAL(100, static_castpos.Y.Value()); } { const auto pos = LaunchPositionFromString("50,50"); VERIFY_IS_TRUE(pos.X.has_value()); - VERIFY_ARE_EQUAL(50, pos.X.Value()); + VERIFY_ARE_EQUAL(50, static_castpos.X.Value()); VERIFY_IS_TRUE(pos.Y.has_value()); - VERIFY_ARE_EQUAL(50, pos.Y.Value()); + VERIFY_ARE_EQUAL(50, static_castpos.Y.Value()); } { const auto pos = LaunchPositionFromString("abc,100"); VERIFY_IS_FALSE(pos.X.has_value()); VERIFY_IS_TRUE(pos.Y.has_value()); - VERIFY_ARE_EQUAL(100, pos.Y.Value()); + VERIFY_ARE_EQUAL(100, static_castpos.Y.Value()); } { From c4549fa2d52c994971ba97063729b6bd2f1502ff Mon Sep 17 00:00:00 2001 From: KavishaHaswani <141186389+KavishaHaswani@users.noreply.github.com> Date: Wed, 8 Oct 2025 05:26:59 +0530 Subject: [PATCH 3/7] Fix test logic for -pos parsing; passes locally --- .../LocalTests_TerminalApp/SettingsTests.cpp | 49 ------------------- .../TerminalSettingsTests.cpp | 47 ++++++++++++++++++ 2 files changed, 47 insertions(+), 49 deletions(-) diff --git a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp index 42a39cb6256..dc3845e930d 100644 --- a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp @@ -6,7 +6,6 @@ #include "../TerminalApp/TerminalPage.h" #include "../UnitTests_SettingsModel/TestUtils.h" #include "../TerminalSettingsAppAdapterLib/TerminalSettings.h" -#include "../TerminalSettingsModel/ModelSerializationHelpers.h" using namespace Microsoft::Console; using namespace WEX::Logging; @@ -74,8 +73,6 @@ namespace TerminalAppLocalTests TEST_METHOD(TestElevateArg); - TEST_METHOD(TestInitialPositionParsing); - TEST_CLASS_SETUP(ClassSetup) { return true; @@ -1621,50 +1618,4 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(true, termSettings->Elevate()); } } - - void SettingsTests::TestInitialPositionParsing() - { - { - const auto pos = LaunchPositionFromString("50"); - VERIFY_IS_TRUE(pos.X.has_value()); - VERIFY_IS_TRUE(pos.Y.has_value()); - VERIFY_ARE_EQUAL(50, static_castpos.X.Value()); - VERIFY_ARE_EQUAL(50, static_castpos.Y.Value()); - } - - { - const auto pos = LaunchPositionFromString("100,"); - VERIFY_IS_TRUE(pos.X.has_value()); - VERIFY_ARE_EQUAL(100, static_castpos.X.Value()); - VERIFY_IS_FALSE(pos.Y.has_value()); - } - - { - const auto pos = LaunchPositionFromString(",100"); - VERIFY_IS_FALSE(pos.X.has_value()); - VERIFY_IS_TRUE(pos.Y.has_value()); - VERIFY_ARE_EQUAL(100, static_castpos.Y.Value()); - } - - { - const auto pos = LaunchPositionFromString("50,50"); - VERIFY_IS_TRUE(pos.X.has_value()); - VERIFY_ARE_EQUAL(50, static_castpos.X.Value()); - VERIFY_IS_TRUE(pos.Y.has_value()); - VERIFY_ARE_EQUAL(50, static_castpos.Y.Value()); - } - - { - const auto pos = LaunchPositionFromString("abc,100"); - VERIFY_IS_FALSE(pos.X.has_value()); - VERIFY_IS_TRUE(pos.Y.has_value()); - VERIFY_ARE_EQUAL(100, static_castpos.Y.Value()); - } - - { - const auto pos = LaunchPositionFromString("abc"); - VERIFY_IS_FALSE(pos.X.has_value()); - VERIFY_IS_FALSE(pos.Y.has_value()); - } - } } \ No newline at end of file diff --git a/src/cascadia/UnitTests_SettingsModel/TerminalSettingsTests.cpp b/src/cascadia/UnitTests_SettingsModel/TerminalSettingsTests.cpp index 36dfd21eae6..90aba8669d8 100644 --- a/src/cascadia/UnitTests_SettingsModel/TerminalSettingsTests.cpp +++ b/src/cascadia/UnitTests_SettingsModel/TerminalSettingsTests.cpp @@ -7,6 +7,7 @@ #include #include "../TerminalSettingsModel/CascadiaSettings.h" +#include "../TerminalSettingsModel/ModelSerializationHelpers.h" #include "TestUtils.h" using namespace Microsoft::Console; @@ -52,6 +53,7 @@ namespace SettingsModelUnitTests TEST_METHOD(MakeSettingsForDefaultProfileThatDoesntExist); TEST_METHOD(TestLayerProfileOnColorScheme); TEST_METHOD(TestCommandlineToTitlePromotion); + TEST_METHOD(TestInitialPositionParsing); }; // CascadiaSettings::_normalizeCommandLine abuses some aspects from CommandLineToArgvW @@ -891,4 +893,49 @@ namespace SettingsModelUnitTests VERIFY_ARE_EQUAL(L"", settingsStruct.DefaultSettings()->StartingTitle()); } } + void TerminalSettingsTests::TestInitialPositionParsing() + { + { + const auto pos = LaunchPositionFromString("50"); + VERIFY_IS_TRUE(pos.X.Value()); + VERIFY_IS_TRUE(pos.Y.Value()); + VERIFY_ARE_EQUAL(50, static_cast(pos.X.Value())); + VERIFY_ARE_EQUAL(50, static_cast(pos.Y.Value())); + } + + { + const auto pos = LaunchPositionFromString("100,"); + VERIFY_IS_TRUE(pos.X.Value()); + VERIFY_ARE_EQUAL(100, static_cast(pos.X.Value())); + VERIFY_IS_TRUE(pos.Y == nullptr); + } + + { + const auto pos = LaunchPositionFromString(",100"); + VERIFY_IS_TRUE(pos.X == nullptr); + VERIFY_IS_TRUE(pos.Y.Value()); + VERIFY_ARE_EQUAL(100, static_cast(pos.Y.Value())); + } + + { + const auto pos = LaunchPositionFromString("50,50"); + VERIFY_IS_TRUE(pos.X.Value()); + VERIFY_ARE_EQUAL(50, static_cast(pos.X.Value())); + VERIFY_IS_TRUE(pos.Y.Value()); + VERIFY_ARE_EQUAL(50, static_cast(pos.Y.Value())); + } + + { + const auto pos = LaunchPositionFromString("abc,100"); + VERIFY_IS_TRUE(pos.X == nullptr); + VERIFY_IS_TRUE(pos.Y.Value()); + VERIFY_ARE_EQUAL(100, static_cast(pos.Y.Value())); + } + + { + const auto pos = LaunchPositionFromString("abc"); + VERIFY_IS_TRUE(pos.X == nullptr); + VERIFY_IS_TRUE(pos.Y == nullptr); + } + } } From 4909abe8d611958fd48b6654e0acbb089bfa2b33 Mon Sep 17 00:00:00 2001 From: KavishaHaswani <141186389+KavishaHaswani@users.noreply.github.com> Date: Wed, 5 Nov 2025 13:16:01 +0530 Subject: [PATCH 4/7] fix: apply LaunchPositionFromString updates to ParseCommaSeparatedPair --- src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp | 1 + .../TerminalSettingsModel/ModelSerializationHelpers.h | 9 +++------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp index dc3845e930d..3ae413c7091 100644 --- a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp @@ -1618,4 +1618,5 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(true, termSettings->Elevate()); } } + } \ No newline at end of file diff --git a/src/cascadia/TerminalSettingsModel/ModelSerializationHelpers.h b/src/cascadia/TerminalSettingsModel/ModelSerializationHelpers.h index 961d523399a..6531d839e76 100644 --- a/src/cascadia/TerminalSettingsModel/ModelSerializationHelpers.h +++ b/src/cascadia/TerminalSettingsModel/ModelSerializationHelpers.h @@ -16,10 +16,10 @@ Module Name: // - Helper for converting a pair of comma separated, potentially absent integer values // into the corresponding left and right values. The leftValue and rightValue functions // will be called back with the associated parsed integer value, assuming it's present. -// (100, 100): leftValue and rightValue functions both called back with 100. +// (100, 100), (100): leftValue and rightValue functions both called back with 100. // (100, ), (100, abc): leftValue function called back with 100 // (, 100), (abc, 100): rightValue function called back with 100 -// (,): no function called back +// (,), (abc): no function called back // (100, 100, 100): we only read the first two values, this is equivalent to (100, 100) // Arguments: // - string: The string to parse @@ -45,6 +45,7 @@ _TIL_INLINEPREFIX void ParseCommaSeparatedPair(const std::string& string, if (index == 0) { leftValue(value); + if (string.find(',') == std::string::npos) {rightValue(value);} } if (index == 1) @@ -67,10 +68,6 @@ _TIL_INLINEPREFIX ::winrt::Microsoft::Terminal::Settings::Model::LaunchPosition string, [&initialPosition](int32_t left) { initialPosition.X = left; }, [&initialPosition](int32_t right) { initialPosition.Y = right; }); - if (initialPosition.X && !initialPosition.Y && string.find(',') == std::string::npos) - { - initialPosition.Y = initialPosition.X; - } return initialPosition; } From 6c588f71726f7c7870db9c3940e7acd9a53c083b Mon Sep 17 00:00:00 2001 From: KavishaHaswani <141186389+KavishaHaswani@users.noreply.github.com> Date: Wed, 5 Nov 2025 13:21:47 +0530 Subject: [PATCH 5/7] style: revert whitespace changes in SettingsTests.cpp --- src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp index 3ae413c7091..ad868e662f9 100644 --- a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp @@ -1618,5 +1618,5 @@ namespace TerminalAppLocalTests VERIFY_ARE_EQUAL(true, termSettings->Elevate()); } } - + } \ No newline at end of file From c67a40a11421dd455e99f1498ebbcf3ab087b9bd Mon Sep 17 00:00:00 2001 From: KavishaHaswani <141186389+KavishaHaswani@users.noreply.github.com> Date: Wed, 5 Nov 2025 13:29:37 +0530 Subject: [PATCH 6/7] style: revert whitespace changes in SettingsTests.cpp --- src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp index ad868e662f9..154920b1056 100644 --- a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp @@ -1619,4 +1619,4 @@ namespace TerminalAppLocalTests } } -} \ No newline at end of file +} \ No newline at end of file From bc293a0b462539a1192ce50c5a3ba1a8dcb86773 Mon Sep 17 00:00:00 2001 From: KavishaHaswani <141186389+KavishaHaswani@users.noreply.github.com> Date: Wed, 5 Nov 2025 13:36:32 +0530 Subject: [PATCH 7/7] style: revert whitespace change in SettingsTests.cpp --- src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp index 154920b1056..dfc1c8afb91 100644 --- a/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp +++ b/src/cascadia/LocalTests_TerminalApp/SettingsTests.cpp @@ -1619,4 +1619,4 @@ namespace TerminalAppLocalTests } } -} \ No newline at end of file +}