From 4f3693e32284c1f052317224103e60a99969e99b Mon Sep 17 00:00:00 2001
From: Chenlei Hu <huchenlei@proton.me>
Date: Wed, 20 Nov 2024 15:01:04 -0500
Subject: [PATCH] Reland 'Bind Ctrl+s to Comfy.SaveWorkflow' (#1618)

---
 src/stores/coreKeybindings.ts                     | 13 ++++++++++++-
 src/stores/keybindingStore.ts                     | 13 +++++++++++--
 tests-ui/tests/slow/store/keybindingStore.test.ts |  4 ++--
 3 files changed, 25 insertions(+), 5 deletions(-)

diff --git a/src/stores/coreKeybindings.ts b/src/stores/coreKeybindings.ts
index 089d1343fb..8e3066bb8c 100644
--- a/src/stores/coreKeybindings.ts
+++ b/src/stores/coreKeybindings.ts
@@ -59,7 +59,7 @@ export const CORE_KEYBINDINGS: Keybinding[] = [
       key: 's',
       ctrl: true
     },
-    commandId: 'Comfy.ExportWorkflow'
+    commandId: 'Comfy.SaveWorkflow'
   },
   {
     combo: {
@@ -182,3 +182,14 @@ export const CORE_KEYBINDINGS: Keybinding[] = [
     commandId: 'Workspace.ToggleFocusMode'
   }
 ]
+
+export const DEPRECATED_KEYBINDINGS: Keybinding[] = [
+  // Ctrl+S is now used for saving the workflow after v1.4.6.
+  {
+    combo: {
+      key: 's',
+      ctrl: true
+    },
+    commandId: 'Comfy.ExportWorkflow'
+  }
+]
diff --git a/src/stores/keybindingStore.ts b/src/stores/keybindingStore.ts
index c82cf3369d..103fba7e68 100644
--- a/src/stores/keybindingStore.ts
+++ b/src/stores/keybindingStore.ts
@@ -2,7 +2,7 @@ import { defineStore } from 'pinia'
 import { computed, Ref, ref, toRaw } from 'vue'
 import { Keybinding, KeyCombo } from '@/types/keyBindingTypes'
 import { useSettingStore } from './settingStore'
-import { CORE_KEYBINDINGS } from './coreKeybindings'
+import { CORE_KEYBINDINGS, DEPRECATED_KEYBINDINGS } from './coreKeybindings'
 import type { ComfyExtension } from '@/types/comfy'
 
 export class KeybindingImpl implements Keybinding {
@@ -215,6 +215,9 @@ export const useKeybindingStore = defineStore('keybinding', () => {
     addKeybinding(userKeybindings, keybinding, { existOk: true })
   }
 
+  const deprecatedKeybindings = DEPRECATED_KEYBINDINGS.map(
+    (k) => new KeybindingImpl(k)
+  )
   function unsetKeybinding(keybinding: KeybindingImpl) {
     const serializedCombo = keybinding.combo.serialize()
     if (!(serializedCombo in keybindingByKeyCombo.value)) {
@@ -231,7 +234,13 @@ export const useKeybindingStore = defineStore('keybinding', () => {
       return
     }
 
-    throw new Error(`NOT_REACHED`)
+    if (deprecatedKeybindings.some((k) => k.equals(keybinding))) {
+      return
+    }
+
+    console.warn(
+      `Trying to unset non-exist keybinding: ${JSON.stringify(keybinding)}`
+    )
   }
 
   /**
diff --git a/tests-ui/tests/slow/store/keybindingStore.test.ts b/tests-ui/tests/slow/store/keybindingStore.test.ts
index f34985464f..245c25ef1f 100644
--- a/tests-ui/tests/slow/store/keybindingStore.test.ts
+++ b/tests-ui/tests/slow/store/keybindingStore.test.ts
@@ -126,14 +126,14 @@ describe('useKeybindingStore', () => {
     expect(store.getKeybinding(keybinding2.combo)).toEqual(keybinding2)
   })
 
-  it('should throw an error when unsetting non-existent keybindings', () => {
+  it('should not throw an error when unsetting non-existent keybindings', () => {
     const store = useKeybindingStore()
     const keybinding = new KeybindingImpl({
       commandId: 'test.command',
       combo: { key: 'H', alt: true, shift: true }
     })
 
-    expect(() => store.unsetKeybinding(keybinding)).toThrow()
+    expect(() => store.unsetKeybinding(keybinding)).not.toThrow()
   })
 
   it('should remove unset keybinding when adding back a default keybinding', () => {