Refactor ShortcutTracker.Match() to respect AllowFallback priority order

Co-authored-by: kovidgoyal <1308621+kovidgoyal@users.noreply.github.com>
Agent-Logs-Url: https://github.com/kovidgoyal/kitty/sessions/85fbf706-4688-4901-9a23-907cebc91da3

tools/config/utils.go

@@ -326,37 +326,66 @@ func ParseMap(val string) (*KeyAction, error) {
 	return &KeyAction{Name: action_name, Args: action_args, Normalized_keys: NormalizeShortcuts(spec), AllowFallback: allow_fallback}, nil
 }
 
+type partialMatch struct {
+	action   *KeyAction
+	priority int
+}
+
 type ShortcutTracker struct {
-	partial_matches      []*KeyAction
+	partial_matches      []partialMatch
 	partial_num_consumed int
 }
 
 func (self *ShortcutTracker) Match(ev *loop.KeyEvent, all_actions []*KeyAction) *KeyAction {
 	if self.partial_num_consumed > 0 {
 		ev.Handled = true
-		self.partial_matches = utils.Filter(self.partial_matches, func(ac *KeyAction) bool {
-			return self.partial_num_consumed < len(ac.Normalized_keys) && ev.MatchesPressOrRepeatWithFallback(ac.Normalized_keys[self.partial_num_consumed], ac.AllowFallback)
-		})
+		new_matches := self.partial_matches[:0]
+		for _, pm := range self.partial_matches {
+			if self.partial_num_consumed >= len(pm.action.Normalized_keys) {
+				continue
+			}
+			p := ev.MatchesPressOrRepeatPriorityWithFallback(pm.action.Normalized_keys[self.partial_num_consumed], pm.action.AllowFallback)
+			if p >= 0 {
+				if p > pm.priority {
+					pm.priority = p
+				}
+				new_matches = append(new_matches, pm)
+			}
+		}
+		self.partial_matches = new_matches
 		if len(self.partial_matches) == 0 {
 			self.partial_num_consumed = 0
 			return nil
 		}
 	} else {
-		self.partial_matches = utils.Filter(all_actions, func(ac *KeyAction) bool {
-			return ev.MatchesPressOrRepeatWithFallback(ac.Normalized_keys[0], ac.AllowFallback)
-		})
+		new_matches := self.partial_matches[:0]
+		for _, ac := range all_actions {
+			p := ev.MatchesPressOrRepeatPriorityWithFallback(ac.Normalized_keys[0], ac.AllowFallback)
+			if p >= 0 {
+				new_matches = append(new_matches, partialMatch{action: ac, priority: p})
+			}
+		}
+		self.partial_matches = new_matches
 		if len(self.partial_matches) == 0 {
 			return nil
 		}
 		ev.Handled = true
 	}
 	self.partial_num_consumed++
-	for _, x := range self.partial_matches {
-		if self.partial_num_consumed >= len(x.Normalized_keys) {
-			self.partial_num_consumed = 0
-			return x
+	var best *partialMatch
+	for i := range self.partial_matches {
+		pm := &self.partial_matches[i]
+		if self.partial_num_consumed >= len(pm.action.Normalized_keys) {
+			if best == nil || pm.priority < best.priority {
+				best = pm
+			}
 		}
 	}
+	if best != nil {
+		self.partial_num_consumed = 0
+		self.partial_matches = nil
+		return best.action
+	}
 	return nil
 }
 

tools/config/utils_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 
 	"github.com/google/go-cmp/cmp"
+	"github.com/kovidgoyal/kitty/tools/tui/loop"
 )
 
 var _ = fmt.Print
@@ -176,3 +177,84 @@ func TestNormalizeShortcuts(t *testing.T) {
 		}
 	}
 }
+
+func TestShortcutTrackerMatchPriority(t *testing.T) {
+	// Helper to create a KeyAction with a given spec and AllowFallback.
+	makeAction := func(name, spec, allowFallback string) *KeyAction {
+		return &KeyAction{Name: name, Normalized_keys: NormalizeShortcuts(spec), AllowFallback: allowFallback}
+	}
+	// Helper to simulate a key press event.
+	makeEv := func(key, shiftedKey, alternateKey string, mods loop.KeyModifiers) *loop.KeyEvent {
+		return &loop.KeyEvent{Type: loop.PRESS, Key: key, ShiftedKey: shiftedKey, AlternateKey: alternateKey, Mods: mods}
+	}
+
+	// Scenario 1: shifted key event — "shifted,ascii" shortcut wins over "ascii,shifted"
+	actions := []*KeyAction{
+		makeAction("ascii_shifted", "a", "ascii,shifted"),
+		makeAction("shifted_ascii", "a", "shifted,ascii"),
+	}
+	// Shift+A with ShiftedKey="a": matches via shifted fallback for both
+	tracker := ShortcutTracker{}
+	ev := makeEv("A", "a", "", loop.SHIFT)
+	result := tracker.Match(ev, actions)
+	if result == nil || result.Name != "shifted_ascii" {
+		name := "<nil>"
+		if result != nil {
+			name = result.Name
+		}
+		t.Fatalf("shifted key: expected 'shifted_ascii' (shifted first), got %q", name)
+	}
+
+	// Scenario 2: alternate (non-ASCII) key event — "ascii,shifted" shortcut wins over "shifted,ascii"
+	actions2 := []*KeyAction{
+		makeAction("shifted_ascii", "ctrl+c", "shifted,ascii"),
+		makeAction("ascii_shifted", "ctrl+c", "ascii,shifted"),
+	}
+	// Cyrillic "с" with AlternateKey="c": matches via ascii fallback for both
+	tracker2 := ShortcutTracker{}
+	ev2 := makeEv("с", "", "c", loop.CTRL)
+	result2 := tracker2.Match(ev2, actions2)
+	if result2 == nil || result2.Name != "ascii_shifted" {
+		name := "<nil>"
+		if result2 != nil {
+			name = result2.Name
+		}
+		t.Fatalf("ascii key: expected 'ascii_shifted' (ascii first), got %q", name)
+	}
+
+	// Scenario 3: direct match wins over any fallback match
+	// Event: Cyrillic "с" with ctrl + AlternateKey="c"; two shortcuts: one direct match for Cyrillic key,
+	// one matching via ascii fallback.
+	actions3 := []*KeyAction{
+		makeAction("fallback", "ctrl+c", "ascii"),
+		makeAction("direct", "ctrl+с", ""),
+	}
+	tracker3 := ShortcutTracker{}
+	ev3 := makeEv("с", "", "c", loop.CTRL)
+	result3 := tracker3.Match(ev3, actions3)
+	if result3 == nil || result3.Name != "direct" {
+		name := "<nil>"
+		if result3 != nil {
+			name = result3.Name
+		}
+		t.Fatalf("direct match: expected 'direct', got %q", name)
+	}
+
+	// Scenario 4: single-type AllowFallback has same priority as first position in two-type AllowFallback
+	// "shifted" only vs "shifted,ascii" — when matching via shifted key, both have priority 1, so first in list wins
+	actions4 := []*KeyAction{
+		makeAction("shifted_only", "a", "shifted"),
+		makeAction("shifted_ascii", "a", "shifted,ascii"),
+	}
+	tracker4 := ShortcutTracker{}
+	ev4 := makeEv("A", "a", "", loop.SHIFT)
+	result4 := tracker4.Match(ev4, actions4)
+	// Both have priority 1 (shifted at position 0); first encountered wins
+	if result4 == nil || result4.Name != "shifted_only" {
+		name := "<nil>"
+		if result4 != nil {
+			name = result4.Name
+		}
+		t.Fatalf("single vs two-type (shifted): expected 'shifted_only', got %q", name)
+	}
+}