From 2fd65cdfe25d6701d9efe73fe25529c310e5bf76 Mon Sep 17 00:00:00 2001 From: Christian Rocha Date: Mon, 21 Jul 2025 18:47:53 -0400 Subject: [PATCH 1/2] fix(lint): variable shadowing --- internal/tui/tui.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 728437e6a9b46bf52c23b23a78f1cdeb4fa588c2..dda0ce2b9a5190953cf2bc288001a74c8c763b09 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -254,9 +254,9 @@ func (a *appModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return a, tea.Batch(cmds...) case splash.OnboardingCompleteMsg: a.isConfigured = config.HasInitialDataConfig() - updated, cmd := a.pages[a.currentPage].Update(msg) + updated, pageCmd := a.pages[a.currentPage].Update(msg) a.pages[a.currentPage] = updated.(util.Model) - cmds = append(cmds, cmd) + cmds = append(cmds, pageCmd) return a, tea.Batch(cmds...) // Key Press Messages case tea.KeyPressMsg: @@ -268,9 +268,9 @@ func (a *appModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { a.dialog = u.(dialogs.DialogCmp) cmds = append(cmds, dialogCmd) } else { - updated, cmd := a.pages[a.currentPage].Update(msg) + updated, pageCmd := a.pages[a.currentPage].Update(msg) a.pages[a.currentPage] = updated.(util.Model) - cmds = append(cmds, cmd) + cmds = append(cmds, pageCmd) } return a, tea.Batch(cmds...) } From fe9fdf2228f1af8a3fed99fa37036892e792e143 Mon Sep 17 00:00:00 2001 From: Christian Rocha Date: Mon, 21 Jul 2025 19:22:28 -0400 Subject: [PATCH 2/2] chore: tidy various comments --- internal/app/app.go | 25 +++++++++++++++++------- internal/app/lsp.go | 46 ++++++++++++++++++++++---------------------- internal/cmd/root.go | 7 ++++--- 3 files changed, 45 insertions(+), 33 deletions(-) diff --git a/internal/app/app.go b/internal/app/app.go index 654cb87ca643530eb030195f5b45679ee18af8e6..d63c90c6e2599f63e3a65cd8069b53638f45cc5f 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -53,6 +53,7 @@ type App struct { cleanupFuncs []func() } +// New initializes a new applcation instance. func New(ctx context.Context, conn *sql.DB, cfg *config.Config) (*App, error) { q := db.New(conn) sessions := session.NewService(q) @@ -78,10 +79,10 @@ func New(ctx context.Context, conn *sql.DB, cfg *config.Config) (*App, error) { app.setupEvents() - // Initialize LSP clients in the background + // Initialize LSP clients in the background. go app.initLSPClients(ctx) - // TODO: remove the concept of agent config most likely + // TODO: remove the concept of agent config, most likely. if cfg.IsConfigured() { if err := app.InitCoderAgent(); err != nil { return nil, fmt.Errorf("failed to initialize coder agent: %w", err) @@ -97,20 +98,22 @@ func (app *App) Config() *config.Config { return app.config } -// RunNonInteractive handles the execution flow when a prompt is provided via CLI flag. +// RunNonInteractive handles the execution flow when a prompt is provided via +// CLI flag. func (app *App) RunNonInteractive(ctx context.Context, prompt string, quiet bool) error { slog.Info("Running in non-interactive mode") ctx, cancel := context.WithCancel(ctx) defer cancel() - // Start spinner if not in quiet mode + // Start spinner if not in quiet mode. var spinner *format.Spinner if !quiet { spinner = format.NewSpinner(ctx, cancel, "Generating") spinner.Start() } - // Helper function to stop spinner once + + // Helper function to stop spinner once. stopSpinner := func() { if !quiet && spinner != nil { spinner.Stop() @@ -257,9 +260,10 @@ func (app *App) InitCoderAgent() error { return nil } +// Subscribe sends events to the TUI as tea.Msgs. func (app *App) Subscribe(program *tea.Program) { defer log.RecoverPanic("app.Subscribe", func() { - slog.Info("TUI subscription panic - attempting graceful shutdown") + slog.Info("TUI subscription panic: attempting graceful shutdown") program.Quit() }) @@ -271,6 +275,7 @@ func (app *App) Subscribe(program *tea.Program) { app.tuiWG.Wait() }) defer app.tuiWG.Done() + for { select { case <-tuiCtx.Done(): @@ -286,23 +291,28 @@ func (app *App) Subscribe(program *tea.Program) { } } -// Shutdown performs a clean shutdown of the application +// Shutdown performs a graceful shutdown of the application. func (app *App) Shutdown() { if app.CoderAgent != nil { app.CoderAgent.CancelAll() } + app.cancelFuncsMutex.Lock() for _, cancel := range app.watcherCancelFuncs { cancel() } app.cancelFuncsMutex.Unlock() + + // Wait for all LSP watchers to finish. app.lspWatcherWG.Wait() + // Get all LSP clients. app.clientsMutex.RLock() clients := make(map[string]*lsp.Client, len(app.LSPClients)) maps.Copy(clients, app.LSPClients) app.clientsMutex.RUnlock() + // Shutdown all LSP clients. for name, client := range clients { shutdownCtx, cancel := context.WithTimeout(app.globalCtx, 5*time.Second) if err := client.Shutdown(shutdownCtx); err != nil { @@ -311,6 +321,7 @@ func (app *App) Shutdown() { cancel() } + // Call call cleanup functions. for _, cleanup := range app.cleanupFuncs { if cleanup != nil { cleanup() diff --git a/internal/app/lsp.go b/internal/app/lsp.go index 33506016690645dd714c682ddd2e65e992d2d1f9..946a373e5a7a69dc78fcbcc894629ecf3e9485ac 100644 --- a/internal/app/lsp.go +++ b/internal/app/lsp.go @@ -10,10 +10,9 @@ import ( "github.com/charmbracelet/crush/internal/lsp/watcher" ) +// initLSPClients initializes LSP clients. func (app *App) initLSPClients(ctx context.Context) { - // Initialize LSP clients for name, clientConfig := range app.config.LSP { - // Start each client initialization in its own goroutine go app.createAndStartLSPClient(ctx, name, clientConfig.Command, clientConfig.Args...) } slog.Info("LSP clients initialization started in background") @@ -21,68 +20,68 @@ func (app *App) initLSPClients(ctx context.Context) { // createAndStartLSPClient creates a new LSP client, initializes it, and starts its workspace watcher func (app *App) createAndStartLSPClient(ctx context.Context, name string, command string, args ...string) { - // Create a specific context for initialization with a timeout slog.Info("Creating LSP client", "name", name, "command", command, "args", args) - // Create the LSP client + // Create LSP client. lspClient, err := lsp.NewClient(ctx, command, args...) if err != nil { slog.Error("Failed to create LSP client for", name, err) return } - // Create a longer timeout for initialization (some servers take time to start) + // Increase initialization timeout as some servers take more time to start. initCtx, cancel := context.WithTimeout(ctx, 30*time.Second) defer cancel() - // Initialize with the initialization context + // Initialize LSP client. _, err = lspClient.InitializeLSPClient(initCtx, app.config.WorkingDir()) if err != nil { slog.Error("Initialize failed", "name", name, "error", err) - // Clean up the client to prevent resource leaks lspClient.Close() return } - // Wait for the server to be ready + // Wait for the server to be ready. if err := lspClient.WaitForServerReady(initCtx); err != nil { slog.Error("Server failed to become ready", "name", name, "error", err) - // We'll continue anyway, as some functionality might still work + // Server never reached a ready state, but let's continue anyway, as + // some functionality might still work. lspClient.SetServerState(lsp.StateError) } else { + // Server reached a ready state scuccessfully. slog.Info("LSP server is ready", "name", name) lspClient.SetServerState(lsp.StateReady) } slog.Info("LSP client initialized", "name", name) - // Create a child context that can be canceled when the app is shutting down + // Create a child context that can be canceled when the app is shutting + // down. watchCtx, cancelFunc := context.WithCancel(ctx) - // Create the workspace watcher + // Create the workspace watcher. workspaceWatcher := watcher.NewWorkspaceWatcher(name, lspClient) - // Store the cancel function to be called during cleanup + // Store the cancel function to be called during cleanup. app.cancelFuncsMutex.Lock() app.watcherCancelFuncs = append(app.watcherCancelFuncs, cancelFunc) app.cancelFuncsMutex.Unlock() - // Add the watcher to a WaitGroup to track active goroutines - app.lspWatcherWG.Add(1) - // Add to map with mutex protection before starting goroutine app.clientsMutex.Lock() app.LSPClients[name] = lspClient app.clientsMutex.Unlock() + // Run workspace watcher. + app.lspWatcherWG.Add(1) go app.runWorkspaceWatcher(watchCtx, name, workspaceWatcher) } -// runWorkspaceWatcher executes the workspace watcher for an LSP client +// runWorkspaceWatcher executes the workspace watcher for an LSP client. func (app *App) runWorkspaceWatcher(ctx context.Context, name string, workspaceWatcher *watcher.WorkspaceWatcher) { defer app.lspWatcherWG.Done() defer log.RecoverPanic("LSP-"+name, func() { - // Try to restart the client + // Try to restart the client. app.restartLSPClient(ctx, name) }) @@ -90,31 +89,32 @@ func (app *App) runWorkspaceWatcher(ctx context.Context, name string, workspaceW slog.Info("Workspace watcher stopped", "client", name) } -// restartLSPClient attempts to restart a crashed or failed LSP client +// restartLSPClient attempts to restart a crashed or failed LSP client. func (app *App) restartLSPClient(ctx context.Context, name string) { - // Get the original configuration + // Get the original configuration. clientConfig, exists := app.config.LSP[name] if !exists { slog.Error("Cannot restart client, configuration not found", "client", name) return } - // Clean up the old client if it exists + // Clean up the old client if it exists. app.clientsMutex.Lock() oldClient, exists := app.LSPClients[name] if exists { - delete(app.LSPClients, name) // Remove from map before potentially slow shutdown + // Remove from map before potentially slow shutdown. + delete(app.LSPClients, name) } app.clientsMutex.Unlock() if exists && oldClient != nil { - // Try to shut it down gracefully, but don't block on errors + // Try to shut down client gracefully, but don't block on errors. shutdownCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second) _ = oldClient.Shutdown(shutdownCtx) cancel() } - // Create a new client using the shared function + // Create a new client using the shared function. app.createAndStartLSPClient(ctx, name, clientConfig.Command, clientConfig.Args...) slog.Info("Successfully restarted LSP client", "client", name) } diff --git a/internal/cmd/root.go b/internal/cmd/root.go index b3ea36c8a976face0bb29c3d77e0d2c82dfb1399..d63160992141da26b6a26610b06f1b601213e00d 100644 --- a/internal/cmd/root.go +++ b/internal/cmd/root.go @@ -48,6 +48,7 @@ to assist developers in writing, debugging, and understanding code directly from `, RunE: func(cmd *cobra.Command, args []string) error { // Load the config + // XXX: Handle errors. debug, _ := cmd.Flags().GetBool("debug") cwd, _ := cmd.Flags().GetString("cwd") prompt, _ := cmd.Flags().GetString("prompt") @@ -76,7 +77,7 @@ to assist developers in writing, debugging, and understanding code directly from ctx := cmd.Context() - // Connect DB, this will also run migrations + // Connect to DB; this will also run migrations. conn, err := db.Connect(ctx, cfg.Options.DataDirectory) if err != nil { return err @@ -95,13 +96,13 @@ to assist developers in writing, debugging, and understanding code directly from return err } - // Non-interactive mode + // Non-interactive mode. if prompt != "" { // Run non-interactive flow using the App method return app.RunNonInteractive(ctx, prompt, quiet) } - // Set up the TUI + // Set up the TUI. program := tea.NewProgram( tui.New(app), tea.WithAltScreen(),