From 5af8fdede65badbac6e4e75ed2b19a83e05c1079 Mon Sep 17 00:00:00 2001 From: Hammad Majid Date: Fri, 2 Oct 2026 01:22:03 +0500 Subject: [PATCH 1/4] fix(action): a user binding displaces the built-in on its key (#63) --- internal/action/action.go | 17 ++++++++++++++++- internal/action/action_test.go | 26 +++++++++++++------------- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/internal/action/action.go b/internal/action/action.go index f52fce3..7377c31 100644 --- a/internal/action/action.go +++ b/internal/action/action.go @@ -72,6 +72,11 @@ func (e *Error) Unwrap() error { return e.Err } // result: the built-ins in their own order, then the user's other Actions // sorted by name. It fails on the first invalid Action with an *Error. // +// A key a user table sets wins over an Action that only holds it by default, +// which is left unbound as if the user had set its key to "". Two Actions +// whose keys the user set to the same key are an error, as are two that +// both hold it by default. +// // vim says whether the vim key map is on, the only one where a plain // printable key may be bound. func Merge(user map[string]Override, vim bool) ([]Action, error) { @@ -123,7 +128,17 @@ func Merge(user map[string]Override, vim bool) ([]Action, error) { if key == "" { continue } - if prev, ok := owner[key]; ok { + prev, clash := owner[key] + switch { + case !clash: + case user[a.Name].Key != nil && user[prev].Key != nil: + return nil, &Error{a.Name, "key", fmt.Errorf("%q is already bound to %q", key, prev)} + case user[a.Name].Key != nil: + out[index[prev]].Key = "" + case user[prev].Key != nil: + a.Key = "" + continue + default: return nil, &Error{a.Name, "key", fmt.Errorf("%q is already bound to %q", key, prev)} } owner[key] = a.Name diff --git a/internal/action/action_test.go b/internal/action/action_test.go index ccc7c8c..9191dbe 100644 --- a/internal/action/action_test.go +++ b/internal/action/action_test.go @@ -105,13 +105,19 @@ func TestMerge_Errors(t *testing.T) { } } -func TestMerge_ClashWithABuiltinNamesTheUserAction(t *testing.T) { +func TestMerge_UserKeyDisplacesABuiltinHoldingIt(t *testing.T) { withBuiltins(t, Action{Name: "files", Key: "ctrl+o", Run: "xdg-open {path}"}) - _, err := Merge(map[string]Override{"code": {Key: str("ctrl+o"), Run: str("code")}}, false) - var ae *Error - if !errors.As(err, &ae) || ae.Name != "code" { - t.Fatalf("err = %v, want an *Error for code", err) + got, err := Merge(map[string]Override{"code": {Key: str("ctrl+o"), Run: str("code")}}, false) + if err != nil { + t.Fatalf("Merge: %v", err) + } + keys := map[string]string{} + for _, a := range got { + keys[a.Name] = a.Key + } + if keys["code"] != "ctrl+o" || keys["files"] != "" { + t.Errorf("keys = %v, want code on ctrl+o and files unbound", keys) } } @@ -160,14 +166,8 @@ func TestMerge_JumpRebindsAndEnterGoesToAnotherAction(t *testing.T) { } } -func TestMerge_EnterNeedsJumpMovedOrUnboundBeforeAnotherActionTakesIt(t *testing.T) { - code := Override{Key: str("enter"), Run: str("code {path}")} - - if _, err := Merge(map[string]Override{"code": code}, false); err == nil { - t.Error("Merge succeeded with jump still on enter, want a conflict") - } - - got, err := Merge(map[string]Override{"code": code, "jump": {Key: str("")}}, false) +func TestMerge_EnterGoesToAnotherActionAndJumpIsLeftUnbound(t *testing.T) { + got, err := Merge(map[string]Override{"code": {Key: str("enter"), Run: str("code {path}")}}, false) if err != nil { t.Fatalf("Merge: %v", err) } From ade53de589579487e5127b8a9532da487375c15b Mon Sep 17 00:00:00 2001 From: Hammad Majid Date: Fri, 2 Oct 2026 01:22:03 +0500 Subject: [PATCH 2/4] test(action): built-in rebound onto a built-in's key, two user Actions on enter (#63) --- internal/action/action_test.go | 45 ++++++++++++++++++++++++++++++++++ 1 file changed, 45 insertions(+) diff --git a/internal/action/action_test.go b/internal/action/action_test.go index 9191dbe..a90c2e1 100644 --- a/internal/action/action_test.go +++ b/internal/action/action_test.go @@ -121,6 +121,51 @@ func TestMerge_UserKeyDisplacesABuiltinHoldingIt(t *testing.T) { } } +func TestMerge_UserRebindingABuiltinDisplacesAnotherBuiltin(t *testing.T) { + withBuiltins(t, + Action{Name: "jump", Key: "enter", Jump: true}, + Action{Name: "remote", Key: "ctrl+r", Run: "open-remote {path}"}, + ) + + got, err := Merge(map[string]Override{"remote": {Key: str("enter")}}, false) + if err != nil { + t.Fatalf("Merge: %v", err) + } + keys := map[string]string{} + for _, a := range got { + keys[a.Name] = a.Key + } + if keys["remote"] != "enter" || keys["jump"] != "" { + t.Errorf("keys = %v, want remote on enter and jump unbound", keys) + } +} + +func TestMerge_UserRebindingAnEarlierBuiltinDisplacesALaterOne(t *testing.T) { + withBuiltins(t, + Action{Name: "a", Key: "ctrl+a", Run: "x"}, + Action{Name: "b", Key: "ctrl+b", Run: "y"}, + ) + + got, err := Merge(map[string]Override{"a": {Key: str("ctrl+b")}}, false) + if err != nil { + t.Fatalf("Merge: %v", err) + } + if got[0].Key != "ctrl+b" || got[1].Key != "" { + t.Errorf("keys = %q, %q, want a on ctrl+b and b unbound", got[0].Key, got[1].Key) + } +} + +func TestMerge_TwoUserActionsOnEnterIsAnError(t *testing.T) { + _, err := Merge(map[string]Override{ + "code": {Key: str("enter"), Run: str("code {path}")}, + "edit": {Key: str("enter"), Run: str("vi {path}")}, + }, false) + var ae *Error + if !errors.As(err, &ae) || ae.Name != "edit" || ae.Field != "key" { + t.Fatalf("err = %v, want an *Error for edit's key", err) + } +} + func TestMerge_PrintableKeyAllowedWithVim(t *testing.T) { withBuiltins(t) got, err := Merge(map[string]Override{"a": {Key: str("?"), Run: str("x")}}, true) From ef4b1fc07262330d6e3369ac275084b166aaac8f Mon Sep 17 00:00:00 2001 From: Hammad Majid Date: Fri, 2 Oct 2026 01:22:04 +0500 Subject: [PATCH 3/4] docs: remapping Enter alone is enough, a user binding wins (#63) --- README.md | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 8099d14..eecaaa9 100644 --- a/README.md +++ b/README.md @@ -161,7 +161,8 @@ Key names are those the Picker recognises: `ctrl+x`, `alt+x`, `enter`, (`ctrl+n` bound means `↓` is the only way down), but `esc` and `ctrl+c` can never be bound. A plain printable key (`a`, `?`) would steal typing, so it is an error unless `keys.vim = true`, where it applies in list focus. -Two Actions on one key is an error, and `key = ""` leaves an Action +Two Actions you bind to one key is an error (a built-in's default key just +yields to yours), and `key = ""` leaves an Action unbound. Each of these is reported with the line it is on. An `[actions.]` table whose name is a built-in Action overrides only @@ -179,9 +180,12 @@ run = "code {path}" detach = true ``` -`jump` holds `enter` until you move it, so giving `enter` to another Action -means moving `jump` or setting `[actions.jump] key = ""`, which leaves it -unbound. That is allowed; `jump` just has no key. +A key you set wins over a built-in that holds it by default: `jump` holds +`enter` until you give `enter` to another Action, which leaves `jump` +unbound, as if you had set `[actions.jump] key = ""`. So remapping `enter` +alone is enough; there is no need to move `jump` first. The same goes for +rebinding one built-in onto another built-in's key. Two Actions you bind to +the same key are still an error. ### Layout From 0ea7800d30385906074ae73463ac2b14513dafab Mon Sep 17 00:00:00 2001 From: Hammad Majid Date: Fri, 2 Oct 2026 01:22:12 +0500 Subject: [PATCH 4/4] docs: reflow the duplicate-key sentence (#63) --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index eecaaa9..4c40308 100644 --- a/README.md +++ b/README.md @@ -161,9 +161,9 @@ Key names are those the Picker recognises: `ctrl+x`, `alt+x`, `enter`, (`ctrl+n` bound means `↓` is the only way down), but `esc` and `ctrl+c` can never be bound. A plain printable key (`a`, `?`) would steal typing, so it is an error unless `keys.vim = true`, where it applies in list focus. -Two Actions you bind to one key is an error (a built-in's default key just -yields to yours), and `key = ""` leaves an Action -unbound. Each of these is reported with the line it is on. +Two Actions you bind to one key is an error, though a built-in's default key +yields to yours, and `key = ""` leaves an Action unbound. Each of these is +reported with the line it is on. An `[actions.]` table whose name is a built-in Action overrides only the fields it sets. The only built-in Action is `jump`, which Jumps to the