From 3a29862bcec6ee042e75908641fc799c68fdf833 Mon Sep 17 00:00:00 2001 From: Robert Deaton Date: Mon, 27 Apr 2026 12:48:35 -0700 Subject: [PATCH] feat(bookmark): prefill bookmark name via open_set_bookmark value arg (#652) * feat(bookmark): allow prefilling name via open_set_bookmark value arg Adds an optional `value` arg to the `revisions.open_set_bookmark` action so Lua scripts can prefill the bookmark input dynamically (e.g. with a date-stamped prefix). Introduces `$string?(name)` annotation syntax for optional string args mirroring the existing `$bool()` behavior. * chore: go fmt --------- Co-authored-by: ibrahim dursun --- cmd/genactions/main.go | 21 ++++++++++++++++--- internal/config/default/types.lua | 2 +- internal/ui/actionmeta/builtins_gen.go | 3 +++ internal/ui/actions/catalog_gen.go | 2 +- internal/ui/intents/ui_intents.go | 6 ++++-- .../ui/operations/bookmark/set_bookmark.go | 5 +++-- .../operations/bookmark/set_bookmark_test.go | 17 ++++++++++++++- internal/ui/revisions/revisions.go | 6 +++--- internal/ui/ui_test.go | 2 +- 9 files changed, 50 insertions(+), 14 deletions(-) diff --git a/cmd/genactions/main.go b/cmd/genactions/main.go index 260206c..051400f 100644 --- a/cmd/genactions/main.go +++ b/cmd/genactions/main.go @@ -809,10 +809,10 @@ func validateSetValueType(fieldType, value string, enums map[string][]enumValueM if strings.HasPrefix(value, "\"") && strings.HasSuffix(value, "\"") { return nil } - if isStringArg(value) { + if isStringArg(value) || isOptionalStringArg(value) { return nil } - return fmt.Errorf("expected quoted string or $string(...), got %q", value) + return fmt.Errorf("expected quoted string, $string(...), or $string?(...), got %q", value) default: if argName, ok := parseEnumArg(value); ok { if len(enums[fieldType]) == 0 { @@ -852,6 +852,10 @@ func isStringArg(value string) bool { return strings.HasPrefix(value, "$string(") && strings.HasSuffix(value, ")") } +func isOptionalStringArg(value string) bool { + return strings.HasPrefix(value, "$string?(") && strings.HasSuffix(value, ")") +} + func parseArgRef(fieldType, value string, enums map[string][]enumValueMeta) (name string, typ string, required bool, ok bool) { if isBoolArg(value) { name := strings.TrimSuffix(strings.TrimPrefix(value, "$bool("), ")") @@ -866,6 +870,13 @@ func parseArgRef(fieldType, value string, enums map[string][]enumValueMeta) (nam } return argName, enumSchemaForType(fieldType, enums), true, true } + if isOptionalStringArg(value) { + name := strings.TrimSuffix(strings.TrimPrefix(value, "$string?("), ")") + if name == "" { + return "", "", false, false + } + return name, "string", false, true + } if isStringArg(value) { name := strings.TrimSuffix(strings.TrimPrefix(value, "$string("), ")") if name == "" { @@ -980,6 +991,10 @@ func renderValue(fieldType, value string, enums map[string][]enumValueMeta) stri name := strings.TrimSuffix(strings.TrimPrefix(value, "$string("), ")") return fmt.Sprintf("actionargs.StringArg(args, %q, \"\")", name) } + if isOptionalStringArg(value) { + name := strings.TrimSuffix(strings.TrimPrefix(value, "$string?("), ")") + return fmt.Sprintf("actionargs.StringArg(args, %q, \"\")", name) + } if argName, ok := parseEnumArg(value); ok && len(enums[fieldType]) > 0 { return fmt.Sprintf("enumArg%s(args, %q)", toCamel(fieldType), argName) } @@ -1004,7 +1019,7 @@ func rulesUseActionArgs(rules []bindRule) bool { if strings.HasPrefix(value, "$bool(") && strings.HasSuffix(value, ")") { return true } - if isStringArg(value) { + if isStringArg(value) || isOptionalStringArg(value) { return true } } diff --git a/internal/config/default/types.lua b/internal/config/default/types.lua index a121efe..e7bf282 100644 --- a/internal/config/default/types.lua +++ b/internal/config/default/types.lua @@ -231,7 +231,7 @@ function wait_refresh() end ---@field open_inline_describe fun() ---@field open_rebase fun() ---@field open_revert fun() ----@field open_set_bookmark fun() +---@field open_set_bookmark fun(args: {value?: string}) ---@field open_set_parents fun() ---@field open_squash fun() ---@field page_down fun() diff --git a/internal/ui/actionmeta/builtins_gen.go b/internal/ui/actionmeta/builtins_gen.go index 2823de9..9e24db1 100644 --- a/internal/ui/actionmeta/builtins_gen.go +++ b/internal/ui/actionmeta/builtins_gen.go @@ -303,6 +303,9 @@ var builtInActionArgSchemas = map[string]map[string]string{ "revisions.inline_describe.accept": { "force": "bool", }, + "revisions.open_set_bookmark": { + "value": "string", + }, "revisions.rebase.apply": { "force": "bool", }, diff --git a/internal/ui/actions/catalog_gen.go b/internal/ui/actions/catalog_gen.go index 31687d7..33f5826 100644 --- a/internal/ui/actions/catalog_gen.go +++ b/internal/ui/actions/catalog_gen.go @@ -307,7 +307,7 @@ func ResolveIntent(scope string, action keybindings.Action, args map[string]any) case keybindings.Action("revisions.open_revert"): return intents.OpenRevert{}, true case keybindings.Action("revisions.open_set_bookmark"): - return intents.OpenSetBookmark{}, true + return intents.OpenSetBookmark{Value: actionargs.StringArg(args, "value", "")}, true case keybindings.Action("revisions.open_set_parents"): return intents.OpenSetParents{}, true case keybindings.Action("revisions.open_squash"): diff --git a/internal/ui/intents/ui_intents.go b/internal/ui/intents/ui_intents.go index 29ba064..6aae67d 100644 --- a/internal/ui/intents/ui_intents.go +++ b/internal/ui/intents/ui_intents.go @@ -77,8 +77,10 @@ type OpenGit struct{} func (OpenGit) isIntent() {} -//jjui:bind scope=revisions action=open_set_bookmark -type OpenSetBookmark struct{} +//jjui:bind scope=revisions action=open_set_bookmark set=Value:$string?(value) +type OpenSetBookmark struct { + Value string +} func (OpenSetBookmark) isIntent() {} diff --git a/internal/ui/operations/bookmark/set_bookmark.go b/internal/ui/operations/bookmark/set_bookmark.go index 76ced29..a112e0a 100644 --- a/internal/ui/operations/bookmark/set_bookmark.go +++ b/internal/ui/operations/bookmark/set_bookmark.go @@ -107,13 +107,14 @@ func (s *SetBookmarkOperation) Name() string { return "set bookmark" } -func NewSetBookmarkOperation(context *context.MainContext, changeId string) *SetBookmarkOperation { +func NewSetBookmarkOperation(context *context.MainContext, changeId string, initialValue string) *SetBookmarkOperation { t := textinput.New() t.ShowSuggestions = true t.CharLimit = 120 t.Prompt = "" - t.SetValue("") + t.SetValue(initialValue) + t.CursorEnd() t.Focus() op := &SetBookmarkOperation{ diff --git a/internal/ui/operations/bookmark/set_bookmark_test.go b/internal/ui/operations/bookmark/set_bookmark_test.go index 2b941d2..3a1135c 100644 --- a/internal/ui/operations/bookmark/set_bookmark_test.go +++ b/internal/ui/operations/bookmark/set_bookmark_test.go @@ -15,8 +15,23 @@ func TestSetBookmarkModel_Update(t *testing.T) { commandRunner.Expect(jj.BookmarkSet("revision", "name")) defer commandRunner.Verify() - op := NewSetBookmarkOperation(test.NewTestContext(commandRunner), "revision") + op := NewSetBookmarkOperation(test.NewTestContext(commandRunner), "revision", "") test.SimulateModel(op, op.Init()) test.SimulateModel(op, test.Type("name")) test.SimulateModel(op, func() tea.Msg { return intents.Apply{} }) } + +func TestSetBookmarkModel_Prefill(t *testing.T) { + commandRunner := test.NewTestCommandRunner(t) + commandRunner.Expect(jj.BookmarkListMovable("revision")) + commandRunner.Expect(jj.BookmarkSet("revision", "rdeaton/20260425/feature")) + defer commandRunner.Verify() + + op := NewSetBookmarkOperation(test.NewTestContext(commandRunner), "revision", "rdeaton/20260425/") + test.SimulateModel(op, op.Init()) + if got := op.name.Value(); got != "rdeaton/20260425/" { + t.Fatalf("expected prefilled value %q, got %q", "rdeaton/20260425/", got) + } + test.SimulateModel(op, test.Type("feature")) + test.SimulateModel(op, func() tea.Msg { return intents.Apply{} }) +} diff --git a/internal/ui/revisions/revisions.go b/internal/ui/revisions/revisions.go index 4a3c987..6119eea 100644 --- a/internal/ui/revisions/revisions.go +++ b/internal/ui/revisions/revisions.go @@ -643,7 +643,7 @@ func (m *Model) HandleIntent(intent intents.Intent) (tea.Cmd, bool) { case intents.OpenSetParents: return m.startSetParents(intent), true case intents.OpenSetBookmark: - return m.startBookmarkSet(), true + return m.startBookmarkSet(intent), true case intents.RevisionsToggleSelect: commit := m.SelectedRevision() if commit == nil { @@ -681,12 +681,12 @@ func (m *Model) HandleIntent(intent intents.Intent) (tea.Cmd, bool) { return nil, false } -func (m *Model) startBookmarkSet() tea.Cmd { +func (m *Model) startBookmarkSet(intent intents.OpenSetBookmark) tea.Cmd { rev := m.SelectedRevision() if rev == nil { return nil } - return m.setBaseOperation(bookmark.NewSetBookmarkOperation(m.context, rev.GetChangeId())) + return m.setBaseOperation(bookmark.NewSetBookmarkOperation(m.context, rev.GetChangeId(), intent.Value)) } func (m *Model) refresh(intent intents.Refresh) tea.Cmd { diff --git a/internal/ui/ui_test.go b/internal/ui/ui_test.go index eb17479..968b2db 100644 --- a/internal/ui/ui_test.go +++ b/internal/ui/ui_test.go @@ -1327,7 +1327,7 @@ func Test_Update_SetBookmarkTypingDoesNotTogglePreview(t *testing.T) { model := NewUI(ctx) model.previewModel.SetVisible(true) - op := bookmark.NewSetBookmarkOperation(ctx, "abc123") + op := bookmark.NewSetBookmarkOperation(ctx, "abc123", "") test.SimulateModel(op, op.Init()) model.Update(common.RestoreOperationMsg{Operation: op}) require.False(t, model.revisions.InNormalMode(), "set bookmark operation should be active")