From 897225908b2ab635ea4e2f10867b7188ef4b84af Mon Sep 17 00:00:00 2001 From: George Tsiolis Date: Mon, 23 Mar 2026 15:13:23 +0200 Subject: [PATCH 1/3] Simplify update notification flow --- cmd/root.go | 7 +-- internal/config/config.go | 30 ++++++++++--- internal/config/default_config.toml | 4 ++ internal/ui/app.go | 37 ++++++++++++++++ internal/ui/app_test.go | 41 ++++++++++++++++++ internal/ui/components/input_prompt.go | 60 +++++++++++++++++++++++--- internal/ui/components/message.go | 1 + internal/update/github.go | 16 +++++-- internal/update/notify.go | 45 +++++++++++-------- internal/update/notify_test.go | 36 ++++++++++------ test/integration/update_test.go | 20 ++++----- 11 files changed, 239 insertions(+), 58 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 0f105051..4cefaf42 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -122,9 +122,10 @@ func startEmulator(ctx context.Context, rt runtime.Runtime, cfg *env.Env, tel *t } notifyOpts := update.NotifyOptions{ - GitHubToken: cfg.GitHubToken, - UpdatePrompt: appConfig.UpdatePrompt, - PersistDisable: config.DisableUpdatePrompt, + GitHubToken: cfg.GitHubToken, + UpdatePrompt: appConfig.CLI.UpdatePrompt, + SkippedVersion: appConfig.CLI.UpdateSkippedVersion, + PersistSkipVersion: config.SetUpdateSkippedVersion, } if isInteractiveMode(cfg) { diff --git a/internal/config/config.go b/internal/config/config.go index eea38fbf..6ed6a187 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -13,10 +13,15 @@ import ( //go:embed default_config.toml var defaultConfigTemplate string +type CLIConfig struct { + UpdatePrompt bool `mapstructure:"update_prompt"` + UpdateSkippedVersion string `mapstructure:"update_skipped_version"` +} + type Config struct { - Containers []ContainerConfig `mapstructure:"containers"` - Env map[string]map[string]string `mapstructure:"env"` - UpdatePrompt bool `mapstructure:"update_prompt"` + Containers []ContainerConfig `mapstructure:"containers"` + Env map[string]map[string]string `mapstructure:"env"` + CLI CLIConfig `mapstructure:"cli"` } func setDefaults() { @@ -27,7 +32,8 @@ func setDefaults() { "port": "4566", }, }) - viper.SetDefault("update_prompt", true) + viper.SetDefault("cli.update_prompt", true) + viper.SetDefault("cli.update_skipped_version", "") } func loadConfig(path string) error { @@ -110,7 +116,15 @@ func Set(key string, value any) error { } func DisableUpdatePrompt() error { - return Set("update_prompt", false) + return Set("cli.update_prompt", false) +} + +func SetUpdateSkippedVersion(version string) error { + return Set("cli.update_skipped_version", version) +} + +func GetUpdateSkippedVersion() string { + return viper.GetString("cli.update_skipped_version") } func Get() (*Config, error) { @@ -118,6 +132,12 @@ func Get() (*Config, error) { if err := viper.Unmarshal(&cfg); err != nil { return nil, fmt.Errorf("failed to unmarshal config: %w", err) } + if !viper.InConfig("cli.update_prompt") && viper.InConfig("update_prompt") { + cfg.CLI.UpdatePrompt = viper.GetBool("update_prompt") + } + if !viper.InConfig("cli.update_skipped_version") && viper.InConfig("update_skipped_version") { + cfg.CLI.UpdateSkippedVersion = viper.GetString("update_skipped_version") + } for i := range cfg.Containers { if err := cfg.Containers[i].Validate(); err != nil { return nil, fmt.Errorf("invalid container config: %w", err) diff --git a/internal/config/default_config.toml b/internal/config/default_config.toml index 45acb3bb..e6f97108 100644 --- a/internal/config/default_config.toml +++ b/internal/config/default_config.toml @@ -1,6 +1,10 @@ # lstk configuration file # Run 'lstk config path' to see where this file lives. +# CLI settings +[cli] +update_prompt = true + # Each [[containers]] block defines an emulator instance. # You can define multiple to run them side by side. [[containers]] diff --git a/internal/ui/app.go b/internal/ui/app.go index 4185bd3b..1b3a473d 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -95,6 +95,26 @@ func (a App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return a, tea.Quit } if a.pendingInput != nil { + if componentsUsesVerticalPrompt(a.inputPrompt, a.pendingInput.Options) { + switch msg.Type { + case tea.KeyUp: + a.inputPrompt = a.inputPrompt.SetSelectedIndex(a.inputPrompt.SelectedIndex() - 1) + return a, nil + case tea.KeyDown: + a.inputPrompt = a.inputPrompt.SetSelectedIndex(a.inputPrompt.SelectedIndex() + 1) + return a, nil + case tea.KeyEnter: + idx := a.inputPrompt.SelectedIndex() + if idx >= 0 && idx < len(a.pendingInput.Options) { + opt := a.pendingInput.Options[idx] + a.lines = appendLine(a.lines, styledLine{text: formatResolvedInput(*a.pendingInput, opt.Key)}) + responseCmd := sendInputResponseCmd(a.pendingInput.ResponseCh, output.InputResponse{SelectedKey: opt.Key}) + a.pendingInput = nil + a.inputPrompt = a.inputPrompt.Hide() + return a, responseCmd + } + } + } if opt := resolveOption(a.pendingInput.Options, msg); opt != nil { a.lines = appendLine(a.lines, styledLine{text: formatResolvedInput(*a.pendingInput, opt.Key)}) responseCmd := sendInputResponseCmd(a.pendingInput.ResponseCh, output.InputResponse{SelectedKey: opt.Key}) @@ -296,6 +316,19 @@ func (a *App) flushBufferedLines() { } func formatResolvedInput(req output.UserInputRequestEvent, selectedKey string) string { + // Special handling for lstk update prompt + if len(req.Options) > 0 && strings.Contains(req.Options[0].Label, "Update now") { + checkmark := styles.Success.Render(output.SuccessMarker()) + switch selectedKey { + case "u": + return checkmark + " Updating lstk..." + case "s": + return checkmark + " Skipped this version" + case "n": + return checkmark + " Won't ask again" + } + } + formatted := output.FormatPrompt(req.Prompt, req.Options) firstLine := strings.Split(formatted, "\n")[0] @@ -316,6 +349,10 @@ func formatResolvedInput(req output.UserInputRequestEvent, selectedKey string) s return fmt.Sprintf("%s %s", firstLine, selected) } +func componentsUsesVerticalPrompt(prompt components.InputPrompt, options []output.InputOption) bool { + return prompt.Visible() && len(options) > 1 && strings.Contains(options[0].Label, "[") +} + // resolveOption finds the best matching option for a key event, in priority order: // 1. "any" — matches any keypress // 2. "enter" — matches the Enter key explicitly diff --git a/internal/ui/app_test.go b/internal/ui/app_test.go index b2da93d8..da932b65 100644 --- a/internal/ui/app_test.go +++ b/internal/ui/app_test.go @@ -410,6 +410,47 @@ func TestAppEnterDoesNothingWithNonLetterLabel(t *testing.T) { } } +func TestAppEnterSelectsHighlightedVerticalOption(t *testing.T) { + t.Parallel() + + app := NewApp("dev", "", "", nil) + responseCh := make(chan output.InputResponse, 1) + + model, _ := app.Update(output.UserInputRequestEvent{ + Prompt: "Update lstk to latest version?", + Options: []output.InputOption{ + {Key: "u", Label: "Update now [U]"}, + {Key: "s", Label: "Skip this version [S]"}, + {Key: "n", Label: "Never ask again [N]"}, + }, + ResponseCh: responseCh, + }) + app = model.(App) + + model, _ = app.Update(tea.KeyMsg{Type: tea.KeyDown}) + app = model.(App) + + model, cmd := app.Update(tea.KeyMsg{Type: tea.KeyEnter}) + app = model.(App) + if cmd == nil { + t.Fatal("expected response command when enter is pressed on vertical prompt") + } + cmd() + + select { + case resp := <-responseCh: + if resp.SelectedKey != "s" { + t.Fatalf("expected s key, got %q", resp.SelectedKey) + } + case <-time.After(time.Second): + t.Fatal("timed out waiting for response on channel") + } + + if app.inputPrompt.Visible() { + t.Fatal("expected input prompt to be hidden after response") + } +} + func TestAppAnyKeyOptionResolvesOnAnyKeypress(t *testing.T) { t.Parallel() diff --git a/internal/ui/components/input_prompt.go b/internal/ui/components/input_prompt.go index 68d4bbb9..dfa40584 100644 --- a/internal/ui/components/input_prompt.go +++ b/internal/ui/components/input_prompt.go @@ -8,9 +8,10 @@ import ( ) type InputPrompt struct { - prompt string - options []output.InputOption - visible bool + prompt string + options []output.InputOption + visible bool + selectedIndex int } func NewInputPrompt() InputPrompt { @@ -21,6 +22,7 @@ func (p InputPrompt) Show(prompt string, options []output.InputOption) InputProm p.prompt = prompt p.options = options p.visible = true + p.selectedIndex = 0 return p } @@ -33,20 +35,31 @@ func (p InputPrompt) Visible() bool { return p.visible } +func (p InputPrompt) SelectedIndex() int { + return p.selectedIndex +} + +func (p InputPrompt) SetSelectedIndex(idx int) InputPrompt { + if idx >= 0 && idx < len(p.options) { + p.selectedIndex = idx + } + return p +} + func (p InputPrompt) View() string { if !p.visible { return "" } - lines := strings.Split(p.prompt, "\n") + if usesVerticalOptions(p.options) { + return p.viewVertical() + } + lines := strings.Split(p.prompt, "\n") firstLine := lines[0] var sb strings.Builder - - // "?" prefix in secondary color sb.WriteString(styles.Secondary.Render("? ")) - sb.WriteString(styles.Message.Render(firstLine)) if suffix := output.FormatPromptLabels(p.options); suffix != "" { @@ -60,3 +73,36 @@ func (p InputPrompt) View() string { return sb.String() } + +func (p InputPrompt) viewVertical() string { + var sb strings.Builder + + if p.prompt != "" { + sb.WriteString(styles.Secondary.Render("? ")) + sb.WriteString(styles.Message.Render(strings.TrimPrefix(p.prompt, "? "))) + sb.WriteString("\n") + } + + for i, opt := range p.options { + if i == p.selectedIndex { + sb.WriteString(styles.NimboMid.Render("● " + opt.Label)) + } else { + sb.WriteString(styles.Secondary.Render("○ " + opt.Label)) + } + sb.WriteString("\n") + } + + return sb.String() +} + +func usesVerticalOptions(options []output.InputOption) bool { + if len(options) < 2 { + return false + } + for _, opt := range options { + if strings.Contains(opt.Label, "[") && strings.Contains(opt.Label, "]") { + return true + } + } + return false +} diff --git a/internal/ui/components/message.go b/internal/ui/components/message.go index 4bf19e96..b28de421 100644 --- a/internal/ui/components/message.go +++ b/internal/ui/components/message.go @@ -14,6 +14,7 @@ func RenderMessage(e output.MessageEvent) string { func RenderWrappedMessage(e output.MessageEvent, width int) string { prefixText, prefix := messagePrefix(e) + if prefixText == "" { style := styles.Message if e.Severity == output.SeveritySecondary { diff --git a/internal/update/github.go b/internal/update/github.go index 472fbdac..992f19bf 100644 --- a/internal/update/github.go +++ b/internal/update/github.go @@ -37,22 +37,30 @@ func githubRequest(ctx context.Context, url, token string) (*http.Response, erro return http.DefaultClient.Do(req) } -func fetchLatestVersion(ctx context.Context, token string) (string, error) { +func fetchLatestRelease(ctx context.Context, token string) (*githubRelease, error) { resp, err := githubRequest(ctx, latestReleaseURL, token) if err != nil { - return "", err + return nil, err } defer func() { _ = resp.Body.Close() }() if resp.StatusCode != http.StatusOK { - return "", fmt.Errorf("GitHub API returned %s", resp.Status) + return nil, fmt.Errorf("GitHub API returned %s", resp.Status) } var release githubRelease if err := json.NewDecoder(resp.Body).Decode(&release); err != nil { - return "", err + return nil, err } + return &release, nil +} + +func fetchLatestVersion(ctx context.Context, token string) (string, error) { + release, err := fetchLatestRelease(ctx, token) + if err != nil { + return "", err + } return release.TagName, nil } diff --git a/internal/update/notify.go b/internal/update/notify.go index bc9ccb6e..12581b8d 100644 --- a/internal/update/notify.go +++ b/internal/update/notify.go @@ -12,12 +12,13 @@ import ( type versionFetcher func(ctx context.Context, token string) (string, error) type NotifyOptions struct { - GitHubToken string - UpdatePrompt bool - PersistDisable func() error + GitHubToken string + UpdatePrompt bool + SkippedVersion string + PersistSkipVersion func(version string) error } -const checkTimeout = 500 * time.Millisecond +const checkTimeout = 2 * time.Second func CheckQuietly(ctx context.Context, githubToken string) (current, latest string, available bool) { return checkQuietlyWithVersion(ctx, githubToken, version.Version(), fetchLatestVersion) @@ -25,6 +26,7 @@ func CheckQuietly(ctx context.Context, githubToken string) (current, latest stri func checkQuietlyWithVersion(ctx context.Context, githubToken string, currentVersion string, fetch versionFetcher) (current, latest string, available bool) { current = currentVersion + // Skip update check for dev builds if current == "dev" { return current, "", false } @@ -54,24 +56,31 @@ func notifyUpdateWithVersion(ctx context.Context, sink output.Sink, opts NotifyO return false } + if opts.SkippedVersion != "" && normalizeVersion(opts.SkippedVersion) == normalizeVersion(latest) { + return false + } + if !opts.UpdatePrompt { output.EmitNote(sink, fmt.Sprintf("Update available: %s → %s (run lstk update)", current, latest)) return false } - return promptAndUpdate(ctx, sink, opts.GitHubToken, current, latest, opts.PersistDisable) + return promptAndUpdate(ctx, sink, opts, current, latest) } -func promptAndUpdate(ctx context.Context, sink output.Sink, githubToken string, current, latest string, persistDisable func() error) (exitAfter bool) { - output.EmitWarning(sink, fmt.Sprintf("Update available: %s → %s", current, latest)) +func promptAndUpdate(ctx context.Context, sink output.Sink, opts NotifyOptions, current, latest string) (exitAfter bool) { + releaseNotesURL := fmt.Sprintf("https://github.com/%s/releases/latest", githubRepo) + + output.EmitNote(sink, fmt.Sprintf("New lstk version available! %s → %s", current, latest)) + output.EmitSecondary(sink, fmt.Sprintf("> Release notes: %s", releaseNotesURL)) responseCh := make(chan output.InputResponse, 1) output.EmitUserInputRequest(sink, output.UserInputRequestEvent{ - Prompt: "A new version is available", + Prompt: "Update lstk to latest version?", Options: []output.InputOption{ - {Key: "u", Label: "Update"}, - {Key: "s", Label: "SKIP"}, - {Key: "n", Label: "Never ask again"}, + {Key: "u", Label: "Update now [U]"}, + {Key: "r", Label: "Remind me next time [R]"}, + {Key: "s", Label: "Skip this version [S]"}, }, ResponseCh: responseCh, }) @@ -89,20 +98,22 @@ func promptAndUpdate(ctx context.Context, sink output.Sink, githubToken string, switch resp.SelectedKey { case "u": - if err := applyUpdate(ctx, sink, latest, githubToken); err != nil { + if err := applyUpdate(ctx, sink, latest, opts.GitHubToken); err != nil { output.EmitWarning(sink, fmt.Sprintf("Update failed: %v", err)) return false } output.EmitSuccess(sink, fmt.Sprintf("Updated to %s — please re-run your command.", latest)) return true - case "n": - if persistDisable != nil { - if err := persistDisable(); err != nil { + case "r": + return false + case "s": + if opts.PersistSkipVersion != nil { + if err := opts.PersistSkipVersion(latest); err != nil { output.EmitWarning(sink, fmt.Sprintf("Failed to save preference: %v", err)) } } return false - default: - return false } + + return false } diff --git a/internal/update/notify_test.go b/internal/update/notify_test.go index 62ba9e35..fd754ab1 100644 --- a/internal/update/notify_test.go +++ b/internal/update/notify_test.go @@ -124,29 +124,20 @@ func TestNotifyUpdatePromptSkip(t *testing.T) { assert.False(t, exit) } -func TestNotifyUpdatePromptNever(t *testing.T) { +func TestNotifyUpdatePromptRemind(t *testing.T) { server := newTestGitHubServer(t, "v2.0.0") defer server.Close() - persistCalled := false - var events []any sink := output.SinkFunc(func(event any) { events = append(events, event) if req, ok := event.(output.UserInputRequestEvent); ok { - req.ResponseCh <- output.InputResponse{SelectedKey: "n"} + req.ResponseCh <- output.InputResponse{SelectedKey: "r"} } }) - exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{ - UpdatePrompt: true, - PersistDisable: func() error { - persistCalled = true - return nil - }, - }, "1.0.0", testFetcher(server.URL)) + exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{UpdatePrompt: true}, "1.0.0", testFetcher(server.URL)) assert.False(t, exit) - assert.True(t, persistCalled) } func TestNotifyUpdatePromptCancelled(t *testing.T) { @@ -157,6 +148,11 @@ func TestNotifyUpdatePromptCancelled(t *testing.T) { sink := output.SinkFunc(func(event any) { events = append(events, event) if req, ok := event.(output.UserInputRequestEvent); ok { + assert.Equal(t, "Update lstk to latest version?", req.Prompt) + assert.Len(t, req.Options, 3) + assert.Equal(t, "u", req.Options[0].Key) + assert.Equal(t, "r", req.Options[1].Key) + assert.Equal(t, "s", req.Options[2].Key) req.ResponseCh <- output.InputResponse{Cancelled: true} } }) @@ -164,3 +160,19 @@ func TestNotifyUpdatePromptCancelled(t *testing.T) { exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{UpdatePrompt: true}, "1.0.0", testFetcher(server.URL)) assert.False(t, exit) } + +func TestNotifyUpdateSkippedVersion(t *testing.T) { + server := newTestGitHubServer(t, "v2.0.0") + defer server.Close() + + var events []any + sink := output.SinkFunc(func(event any) { events = append(events, event) }) + + // Should return early without prompting since v2.0.0 was skipped + exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{ + UpdatePrompt: true, + SkippedVersion: "v2.0.0", + }, "1.0.0", testFetcher(server.URL)) + assert.False(t, exit) + assert.Empty(t, events) // No events should be emitted +} diff --git a/test/integration/update_test.go b/test/integration/update_test.go index c5da4890..b649d2ff 100644 --- a/test/integration/update_test.go +++ b/test/integration/update_test.go @@ -247,7 +247,7 @@ func TestUpdateNotification(t *testing.T) { t.Run("prompt_disabled", func(t *testing.T) { configFile := filepath.Join(t.TempDir(), "config.toml") - require.NoError(t, os.WriteFile(configFile, []byte("update_prompt = false\n"), 0o644)) + require.NoError(t, os.WriteFile(configFile, []byte("[cli]\nupdate_prompt = false\n"), 0o644)) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() @@ -278,7 +278,7 @@ func TestUpdateNotification(t *testing.T) { t.Run("skip", func(t *testing.T) { configFile := filepath.Join(t.TempDir(), "config.toml") - require.NoError(t, os.WriteFile(configFile, []byte("update_prompt = true\n"), 0o644)) + require.NoError(t, os.WriteFile(configFile, []byte("[cli]\nupdate_prompt = true\n"), 0o644)) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() @@ -298,7 +298,7 @@ func TestUpdateNotification(t *testing.T) { }() require.Eventually(t, func() bool { - return bytes.Contains(output.Bytes(), []byte("new version is available")) + return bytes.Contains(output.Bytes(), []byte("New lstk version available")) }, 10*time.Second, 100*time.Millisecond, "update notification prompt should appear") _, err = ptmx.Write([]byte("s")) @@ -307,12 +307,12 @@ func TestUpdateNotification(t *testing.T) { _ = cmd.Wait() <-outputCh - assert.Contains(t, output.String(), "Update available: 0.0.1") + assert.Contains(t, output.String(), "New lstk version available") }) t.Run("never", func(t *testing.T) { configFile := filepath.Join(t.TempDir(), "config.toml") - require.NoError(t, os.WriteFile(configFile, []byte("update_prompt = true\n"), 0o644)) + require.NoError(t, os.WriteFile(configFile, []byte("[cli]\nupdate_prompt = true\n"), 0o644)) ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() @@ -332,7 +332,7 @@ func TestUpdateNotification(t *testing.T) { }() require.Eventually(t, func() bool { - return bytes.Contains(output.Bytes(), []byte("new version is available")) + return bytes.Contains(output.Bytes(), []byte("New lstk version available")) }, 10*time.Second, 100*time.Millisecond, "update notification prompt should appear") _, err = ptmx.Write([]byte("n")) @@ -341,7 +341,7 @@ func TestUpdateNotification(t *testing.T) { _ = cmd.Wait() <-outputCh - assert.Contains(t, output.String(), "Update available: 0.0.1") + assert.Contains(t, output.String(), "New lstk version available") // Verify config was updated to disable future prompts configData, err := os.ReadFile(configFile) @@ -357,7 +357,7 @@ func TestUpdateNotification(t *testing.T) { require.NoError(t, os.WriteFile(updateBinary, data, 0o755)) configFile := filepath.Join(t.TempDir(), "config.toml") - require.NoError(t, os.WriteFile(configFile, []byte("update_prompt = true\n"), 0o644)) + require.NoError(t, os.WriteFile(configFile, []byte("[cli]\nupdate_prompt = true\n"), 0o644)) ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() @@ -377,7 +377,7 @@ func TestUpdateNotification(t *testing.T) { }() require.Eventually(t, func() bool { - return bytes.Contains(output.Bytes(), []byte("new version is available")) + return bytes.Contains(output.Bytes(), []byte("New lstk version available")) }, 10*time.Second, 100*time.Millisecond, "update notification prompt should appear") _, err = ptmx.Write([]byte("u")) @@ -388,7 +388,7 @@ func TestUpdateNotification(t *testing.T) { out := output.String() require.NoError(t, err, "update should succeed: %s", out) - assert.Contains(t, out, "Update available: 0.0.1") + assert.Contains(t, out, "New lstk version available") assert.Contains(t, out, "Updated to") // Verify the binary was actually replaced From a6cfc17bc0daf965dcba481bba889a4115d0e85c Mon Sep 17 00:00:00 2001 From: George Tsiolis Date: Mon, 23 Mar 2026 17:29:17 +0200 Subject: [PATCH 2/3] Remove option from update prompt --- test/integration/start_test.go | 4 ++-- test/integration/update_test.go | 38 --------------------------------- 2 files changed, 2 insertions(+), 40 deletions(-) diff --git a/test/integration/start_test.go b/test/integration/start_test.go index 008f27d7..dd4aded6 100644 --- a/test/integration/start_test.go +++ b/test/integration/start_test.go @@ -207,7 +207,7 @@ func TestStartCommandSetsUpContainerCorrectly(t *testing.T) { t.Run("http health endpoint", func(t *testing.T) { resp, err := http.Get("http://localhost.localstack.cloud:4566/_localstack/health") require.NoError(t, err) - defer resp.Body.Close() + defer func() { require.NoError(t, resp.Body.Close()) }() assert.Equal(t, http.StatusOK, resp.StatusCode) }) @@ -221,7 +221,7 @@ func TestStartCommandSetsUpContainerCorrectly(t *testing.T) { } resp, err := client.Get("https://localhost.localstack.cloud/_localstack/health") require.NoError(t, err) - defer resp.Body.Close() + defer func() { require.NoError(t, resp.Body.Close()) }() assert.Equal(t, http.StatusOK, resp.StatusCode) }) } diff --git a/test/integration/update_test.go b/test/integration/update_test.go index b649d2ff..2221f870 100644 --- a/test/integration/update_test.go +++ b/test/integration/update_test.go @@ -310,44 +310,6 @@ func TestUpdateNotification(t *testing.T) { assert.Contains(t, output.String(), "New lstk version available") }) - t.Run("never", func(t *testing.T) { - configFile := filepath.Join(t.TempDir(), "config.toml") - require.NoError(t, os.WriteFile(configFile, []byte("[cli]\nupdate_prompt = true\n"), 0o644)) - - ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) - defer cancel() - - cmd := exec.CommandContext(ctx, tmpBinary, "--config", configFile) - cmd.Env = env.Without(env.AuthToken).With(env.AuthToken, "fake-token").With(env.APIEndpoint, mockServer.URL) - - ptmx, err := pty.Start(cmd) - require.NoError(t, err, "failed to start command in PTY") - defer func() { _ = ptmx.Close() }() - - output := &syncBuffer{} - outputCh := make(chan struct{}) - go func() { - _, _ = io.Copy(output, ptmx) - close(outputCh) - }() - - require.Eventually(t, func() bool { - return bytes.Contains(output.Bytes(), []byte("New lstk version available")) - }, 10*time.Second, 100*time.Millisecond, "update notification prompt should appear") - - _, err = ptmx.Write([]byte("n")) - require.NoError(t, err) - - _ = cmd.Wait() - <-outputCh - - assert.Contains(t, output.String(), "New lstk version available") - - // Verify config was updated to disable future prompts - configData, err := os.ReadFile(configFile) - require.NoError(t, err) - assert.Contains(t, string(configData), "update_prompt = false") - }) t.Run("update", func(t *testing.T) { // Copy binary since it will be replaced during the update From 0b42a8bf04fe27b0b3c2d8dced38736e7cf8e36b Mon Sep 17 00:00:00 2001 From: George Tsiolis Date: Thu, 26 Mar 2026 23:05:16 +0200 Subject: [PATCH 3/3] Address review comments --- cmd/root.go | 6 +- internal/config/config.go | 15 +---- internal/config/default_config.toml | 4 -- internal/output/events.go | 1 + internal/ui/app.go | 65 ++++++++++----------- internal/ui/app_test.go | 9 +-- internal/ui/components/input_prompt.go | 17 ++---- internal/ui/components/input_prompt_test.go | 8 ++- internal/update/notify.go | 25 ++------ internal/update/notify_test.go | 15 ----- 10 files changed, 54 insertions(+), 111 deletions(-) diff --git a/cmd/root.go b/cmd/root.go index 4cefaf42..1e50dc08 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -122,10 +122,8 @@ func startEmulator(ctx context.Context, rt runtime.Runtime, cfg *env.Env, tel *t } notifyOpts := update.NotifyOptions{ - GitHubToken: cfg.GitHubToken, - UpdatePrompt: appConfig.CLI.UpdatePrompt, - SkippedVersion: appConfig.CLI.UpdateSkippedVersion, - PersistSkipVersion: config.SetUpdateSkippedVersion, + GitHubToken: cfg.GitHubToken, + UpdatePrompt: appConfig.CLI.UpdatePrompt, } if isInteractiveMode(cfg) { diff --git a/internal/config/config.go b/internal/config/config.go index 6ed6a187..021128bd 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -14,8 +14,7 @@ import ( var defaultConfigTemplate string type CLIConfig struct { - UpdatePrompt bool `mapstructure:"update_prompt"` - UpdateSkippedVersion string `mapstructure:"update_skipped_version"` + UpdatePrompt bool `mapstructure:"update_prompt"` } type Config struct { @@ -33,7 +32,6 @@ func setDefaults() { }, }) viper.SetDefault("cli.update_prompt", true) - viper.SetDefault("cli.update_skipped_version", "") } func loadConfig(path string) error { @@ -119,14 +117,6 @@ func DisableUpdatePrompt() error { return Set("cli.update_prompt", false) } -func SetUpdateSkippedVersion(version string) error { - return Set("cli.update_skipped_version", version) -} - -func GetUpdateSkippedVersion() string { - return viper.GetString("cli.update_skipped_version") -} - func Get() (*Config, error) { var cfg Config if err := viper.Unmarshal(&cfg); err != nil { @@ -135,9 +125,6 @@ func Get() (*Config, error) { if !viper.InConfig("cli.update_prompt") && viper.InConfig("update_prompt") { cfg.CLI.UpdatePrompt = viper.GetBool("update_prompt") } - if !viper.InConfig("cli.update_skipped_version") && viper.InConfig("update_skipped_version") { - cfg.CLI.UpdateSkippedVersion = viper.GetString("update_skipped_version") - } for i := range cfg.Containers { if err := cfg.Containers[i].Validate(); err != nil { return nil, fmt.Errorf("invalid container config: %w", err) diff --git a/internal/config/default_config.toml b/internal/config/default_config.toml index e6f97108..45acb3bb 100644 --- a/internal/config/default_config.toml +++ b/internal/config/default_config.toml @@ -1,10 +1,6 @@ # lstk configuration file # Run 'lstk config path' to see where this file lives. -# CLI settings -[cli] -update_prompt = true - # Each [[containers]] block defines an emulator instance. # You can define multiple to run them side by side. [[containers]] diff --git a/internal/output/events.go b/internal/output/events.go index d9ea5116..ea8ddac9 100644 --- a/internal/output/events.go +++ b/internal/output/events.go @@ -122,6 +122,7 @@ type UserInputRequestEvent struct { Prompt string Options []InputOption ResponseCh chan<- InputResponse + Vertical bool } const ( diff --git a/internal/ui/app.go b/internal/ui/app.go index 1b3a473d..e8d14721 100644 --- a/internal/ui/app.go +++ b/internal/ui/app.go @@ -95,25 +95,8 @@ func (a App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return a, tea.Quit } if a.pendingInput != nil { - if componentsUsesVerticalPrompt(a.inputPrompt, a.pendingInput.Options) { - switch msg.Type { - case tea.KeyUp: - a.inputPrompt = a.inputPrompt.SetSelectedIndex(a.inputPrompt.SelectedIndex() - 1) - return a, nil - case tea.KeyDown: - a.inputPrompt = a.inputPrompt.SetSelectedIndex(a.inputPrompt.SelectedIndex() + 1) - return a, nil - case tea.KeyEnter: - idx := a.inputPrompt.SelectedIndex() - if idx >= 0 && idx < len(a.pendingInput.Options) { - opt := a.pendingInput.Options[idx] - a.lines = appendLine(a.lines, styledLine{text: formatResolvedInput(*a.pendingInput, opt.Key)}) - responseCmd := sendInputResponseCmd(a.pendingInput.ResponseCh, output.InputResponse{SelectedKey: opt.Key}) - a.pendingInput = nil - a.inputPrompt = a.inputPrompt.Hide() - return a, responseCmd - } - } + if a.pendingInput.Vertical { + return a.handleVerticalPromptKey(msg) } if opt := resolveOption(a.pendingInput.Options, msg); opt != nil { a.lines = appendLine(a.lines, styledLine{text: formatResolvedInput(*a.pendingInput, opt.Key)}) @@ -130,7 +113,7 @@ func (a App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if a.spinner.Visible() { a.spinner = a.spinner.SetText(output.FormatPrompt(msg.Prompt, msg.Options)) } else { - a.inputPrompt = a.inputPrompt.Show(msg.Prompt, msg.Options) + a.inputPrompt = a.inputPrompt.Show(msg.Prompt, msg.Options, msg.Vertical) } case spinner.TickMsg: var cmd tea.Cmd @@ -315,20 +298,36 @@ func (a *App) flushBufferedLines() { a.bufferedLines = nil } -func formatResolvedInput(req output.UserInputRequestEvent, selectedKey string) string { - // Special handling for lstk update prompt - if len(req.Options) > 0 && strings.Contains(req.Options[0].Label, "Update now") { - checkmark := styles.Success.Render(output.SuccessMarker()) - switch selectedKey { - case "u": - return checkmark + " Updating lstk..." - case "s": - return checkmark + " Skipped this version" - case "n": - return checkmark + " Won't ask again" +func (a App) handleVerticalPromptKey(msg tea.KeyMsg) (tea.Model, tea.Cmd) { + switch msg.Type { + case tea.KeyUp: + a.inputPrompt = a.inputPrompt.SetSelectedIndex(a.inputPrompt.SelectedIndex() - 1) + return a, nil + case tea.KeyDown: + a.inputPrompt = a.inputPrompt.SetSelectedIndex(a.inputPrompt.SelectedIndex() + 1) + return a, nil + case tea.KeyEnter: + idx := a.inputPrompt.SelectedIndex() + if idx >= 0 && idx < len(a.pendingInput.Options) { + opt := a.pendingInput.Options[idx] + a.lines = appendLine(a.lines, styledLine{text: formatResolvedInput(*a.pendingInput, opt.Key)}) + responseCmd := sendInputResponseCmd(a.pendingInput.ResponseCh, output.InputResponse{SelectedKey: opt.Key}) + a.pendingInput = nil + a.inputPrompt = a.inputPrompt.Hide() + return a, responseCmd } } + if opt := resolveOption(a.pendingInput.Options, msg); opt != nil { + a.lines = appendLine(a.lines, styledLine{text: formatResolvedInput(*a.pendingInput, opt.Key)}) + responseCmd := sendInputResponseCmd(a.pendingInput.ResponseCh, output.InputResponse{SelectedKey: opt.Key}) + a.pendingInput = nil + a.inputPrompt = a.inputPrompt.Hide() + return a, responseCmd + } + return a, nil +} +func formatResolvedInput(req output.UserInputRequestEvent, selectedKey string) string { formatted := output.FormatPrompt(req.Prompt, req.Options) firstLine := strings.Split(formatted, "\n")[0] @@ -349,10 +348,6 @@ func formatResolvedInput(req output.UserInputRequestEvent, selectedKey string) s return fmt.Sprintf("%s %s", firstLine, selected) } -func componentsUsesVerticalPrompt(prompt components.InputPrompt, options []output.InputOption) bool { - return prompt.Visible() && len(options) > 1 && strings.Contains(options[0].Label, "[") -} - // resolveOption finds the best matching option for a key event, in priority order: // 1. "any" — matches any keypress // 2. "enter" — matches the Enter key explicitly diff --git a/internal/ui/app_test.go b/internal/ui/app_test.go index da932b65..dbb26f98 100644 --- a/internal/ui/app_test.go +++ b/internal/ui/app_test.go @@ -417,13 +417,10 @@ func TestAppEnterSelectsHighlightedVerticalOption(t *testing.T) { responseCh := make(chan output.InputResponse, 1) model, _ := app.Update(output.UserInputRequestEvent{ - Prompt: "Update lstk to latest version?", - Options: []output.InputOption{ - {Key: "u", Label: "Update now [U]"}, - {Key: "s", Label: "Skip this version [S]"}, - {Key: "n", Label: "Never ask again [N]"}, - }, + Prompt: "Update lstk to latest version?", + Options: []output.InputOption{{Key: "u", Label: "Update now [U]"}, {Key: "s", Label: "Skip this version [S]"}, {Key: "n", Label: "Never ask again [N]"}}, ResponseCh: responseCh, + Vertical: true, }) app = model.(App) diff --git a/internal/ui/components/input_prompt.go b/internal/ui/components/input_prompt.go index dfa40584..fa1feec8 100644 --- a/internal/ui/components/input_prompt.go +++ b/internal/ui/components/input_prompt.go @@ -12,17 +12,19 @@ type InputPrompt struct { options []output.InputOption visible bool selectedIndex int + vertical bool } func NewInputPrompt() InputPrompt { return InputPrompt{} } -func (p InputPrompt) Show(prompt string, options []output.InputOption) InputPrompt { +func (p InputPrompt) Show(prompt string, options []output.InputOption, vertical bool) InputPrompt { p.prompt = prompt p.options = options p.visible = true p.selectedIndex = 0 + p.vertical = vertical return p } @@ -51,7 +53,7 @@ func (p InputPrompt) View() string { return "" } - if usesVerticalOptions(p.options) { + if p.vertical { return p.viewVertical() } @@ -95,14 +97,3 @@ func (p InputPrompt) viewVertical() string { return sb.String() } -func usesVerticalOptions(options []output.InputOption) bool { - if len(options) < 2 { - return false - } - for _, opt := range options { - if strings.Contains(opt.Label, "[") && strings.Contains(opt.Label, "]") { - return true - } - } - return false -} diff --git a/internal/ui/components/input_prompt_test.go b/internal/ui/components/input_prompt_test.go index 18a8576e..58f26989 100644 --- a/internal/ui/components/input_prompt_test.go +++ b/internal/ui/components/input_prompt_test.go @@ -14,6 +14,7 @@ func TestInputPromptView(t *testing.T) { name string prompt string options []output.InputOption + vertical bool contains []string excludes []string }{ @@ -21,12 +22,14 @@ func TestInputPromptView(t *testing.T) { name: "hidden returns empty", prompt: "", options: nil, + vertical: false, contains: nil, }, { name: "no options", prompt: "Continue?", options: nil, + vertical: false, contains: []string{"?", "Continue?"}, excludes: []string{"(", "["}, }, @@ -34,6 +37,7 @@ func TestInputPromptView(t *testing.T) { name: "single option shows parentheses", prompt: "Continue?", options: []output.InputOption{{Key: "enter", Label: "Press ENTER"}}, + vertical: false, contains: []string{"?", "Continue?", "(Press ENTER)"}, }, { @@ -43,12 +47,14 @@ func TestInputPromptView(t *testing.T) { {Key: "y", Label: "Y"}, {Key: "n", Label: "n"}, }, + vertical: false, contains: []string{"?", "Configure AWS profile?", "[Y/n]"}, }, { name: "multi-line prompt renders trailing lines", prompt: "First line\nSecond line\nThird line", options: []output.InputOption{{Key: "y", Label: "Y"}}, + vertical: false, contains: []string{"?", "First line", "Second line", "Third line", "(Y)"}, }, } @@ -67,7 +73,7 @@ func TestInputPromptView(t *testing.T) { return } - p = p.Show(tc.prompt, tc.options) + p = p.Show(tc.prompt, tc.options, tc.vertical) view := p.View() for _, s := range tc.contains { diff --git a/internal/update/notify.go b/internal/update/notify.go index 12581b8d..3db1c94e 100644 --- a/internal/update/notify.go +++ b/internal/update/notify.go @@ -12,10 +12,8 @@ import ( type versionFetcher func(ctx context.Context, token string) (string, error) type NotifyOptions struct { - GitHubToken string - UpdatePrompt bool - SkippedVersion string - PersistSkipVersion func(version string) error + GitHubToken string + UpdatePrompt bool } const checkTimeout = 2 * time.Second @@ -56,10 +54,6 @@ func notifyUpdateWithVersion(ctx context.Context, sink output.Sink, opts NotifyO return false } - if opts.SkippedVersion != "" && normalizeVersion(opts.SkippedVersion) == normalizeVersion(latest) { - return false - } - if !opts.UpdatePrompt { output.EmitNote(sink, fmt.Sprintf("Update available: %s → %s (run lstk update)", current, latest)) return false @@ -76,13 +70,10 @@ func promptAndUpdate(ctx context.Context, sink output.Sink, opts NotifyOptions, responseCh := make(chan output.InputResponse, 1) output.EmitUserInputRequest(sink, output.UserInputRequestEvent{ - Prompt: "Update lstk to latest version?", - Options: []output.InputOption{ - {Key: "u", Label: "Update now [U]"}, - {Key: "r", Label: "Remind me next time [R]"}, - {Key: "s", Label: "Skip this version [S]"}, - }, + Prompt: "Update lstk to latest version?", + Options: []output.InputOption{{Key: "u", Label: "Update now [U]"}, {Key: "r", Label: "Remind me next time [R]"}, {Key: "s", Label: "Skip this version [S]"}}, ResponseCh: responseCh, + Vertical: true, }) var resp output.InputResponse @@ -107,11 +98,7 @@ func promptAndUpdate(ctx context.Context, sink output.Sink, opts NotifyOptions, case "r": return false case "s": - if opts.PersistSkipVersion != nil { - if err := opts.PersistSkipVersion(latest); err != nil { - output.EmitWarning(sink, fmt.Sprintf("Failed to save preference: %v", err)) - } - } + output.EmitNote(sink, "Skipping version " + latest) return false } diff --git a/internal/update/notify_test.go b/internal/update/notify_test.go index fd754ab1..ae784256 100644 --- a/internal/update/notify_test.go +++ b/internal/update/notify_test.go @@ -161,18 +161,3 @@ func TestNotifyUpdatePromptCancelled(t *testing.T) { assert.False(t, exit) } -func TestNotifyUpdateSkippedVersion(t *testing.T) { - server := newTestGitHubServer(t, "v2.0.0") - defer server.Close() - - var events []any - sink := output.SinkFunc(func(event any) { events = append(events, event) }) - - // Should return early without prompting since v2.0.0 was skipped - exit := notifyUpdateWithVersion(context.Background(), sink, NotifyOptions{ - UpdatePrompt: true, - SkippedVersion: "v2.0.0", - }, "1.0.0", testFetcher(server.URL)) - assert.False(t, exit) - assert.Empty(t, events) // No events should be emitted -}