From 19b5cda3c5168f462ec678d0dc56f6195d8d54ca Mon Sep 17 00:00:00 2001
From: Pankaj Bhojwani <pankaj.d.bhoj@gmail.com>
Date: Wed, 21 Aug 2024 13:27:46 -0700
Subject: [PATCH 1/2] fix

---
 .../ActionsViewModel.cpp                      | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/cascadia/TerminalSettingsEditor/ActionsViewModel.cpp b/src/cascadia/TerminalSettingsEditor/ActionsViewModel.cpp
index 1d59f7a6c7c..0c50889f29c 100644
--- a/src/cascadia/TerminalSettingsEditor/ActionsViewModel.cpp
+++ b/src/cascadia/TerminalSettingsEditor/ActionsViewModel.cpp
@@ -278,9 +278,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         // Check for this special case:
         //  we're changing the key chord,
         //  but the new key chord is already in use
+        const auto& conflictingCmd{ _Settings.ActionMap().GetActionByKeyChord(args.NewKeys()) };
         if (isNewAction || args.OldKeys().Modifiers() != args.NewKeys().Modifiers() || args.OldKeys().Vkey() != args.NewKeys().Vkey())
         {
-            const auto& conflictingCmd{ _Settings.ActionMap().GetActionByKeyChord(args.NewKeys()) };
             if (conflictingCmd)
             {
                 // We're about to overwrite another key chord.
@@ -324,13 +324,18 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             }
         }
 
-        // update settings model and view model
-        applyChangesToSettingsModel();
+        // if there was a conflictingCmd, the flyout we created will handle whether changes need to be propagated
+        // otherwise, go ahead and apply the changes
+        if (!conflictingCmd)
+        {
+            // update settings model and view model
+            applyChangesToSettingsModel();
 
-        // We NEED to toggle the edit mode here,
-        // so that if nothing changed, we still exit
-        // edit mode.
-        senderVM.ToggleEditMode();
+            // We NEED to toggle the edit mode here,
+            // so that if nothing changed, we still exit
+            // edit mode.
+            senderVM.ToggleEditMode();
+        }
     }
 
     void ActionsViewModel::_KeyBindingViewModelDeleteNewlyAddedKeyBindingHandler(const Editor::KeyBindingViewModel& senderVM, const IInspectable& /*args*/)

From ec74a4d19ab26cacfebeae07413e1f4d70c205e5 Mon Sep 17 00:00:00 2001
From: Pankaj Bhojwani <pankaj.d.bhoj@gmail.com>
Date: Wed, 21 Aug 2024 13:37:18 -0700
Subject: [PATCH 2/2] separate bool

---
 src/cascadia/TerminalSettingsEditor/ActionsViewModel.cpp | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/cascadia/TerminalSettingsEditor/ActionsViewModel.cpp b/src/cascadia/TerminalSettingsEditor/ActionsViewModel.cpp
index 0c50889f29c..a8f22f0212f 100644
--- a/src/cascadia/TerminalSettingsEditor/ActionsViewModel.cpp
+++ b/src/cascadia/TerminalSettingsEditor/ActionsViewModel.cpp
@@ -278,11 +278,13 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
         // Check for this special case:
         //  we're changing the key chord,
         //  but the new key chord is already in use
-        const auto& conflictingCmd{ _Settings.ActionMap().GetActionByKeyChord(args.NewKeys()) };
+        bool conflictFound{ false };
         if (isNewAction || args.OldKeys().Modifiers() != args.NewKeys().Modifiers() || args.OldKeys().Vkey() != args.NewKeys().Vkey())
         {
+            const auto& conflictingCmd{ _Settings.ActionMap().GetActionByKeyChord(args.NewKeys()) };
             if (conflictingCmd)
             {
+                conflictFound = true;
                 // We're about to overwrite another key chord.
                 // Display a confirmation dialog.
                 TextBlock errorMessageTB{};
@@ -324,9 +326,9 @@ namespace winrt::Microsoft::Terminal::Settings::Editor::implementation
             }
         }
 
-        // if there was a conflictingCmd, the flyout we created will handle whether changes need to be propagated
+        // if there was a conflict, the flyout we created will handle whether changes need to be propagated
         // otherwise, go ahead and apply the changes
-        if (!conflictingCmd)
+        if (!conflictFound)
         {
             // update settings model and view model
             applyChangesToSettingsModel();