From 0b6a6b27d576a25a87ddde1a7f4dd085f2a25406 Mon Sep 17 00:00:00 2001 From: Carlos Alexandro Becker Date: Fri, 25 Jul 2025 11:46:02 -0300 Subject: [PATCH 1/7] ci: publish to winget, nur; fixes brew token (#297) --- .goreleaser.yml | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/.goreleaser.yml b/.goreleaser.yml index 827574aab39b51ff3bb32a9faf98da7773bad605..17dab8f9cb7b88781d54746e6372c990127eaf62 100644 --- a/.goreleaser.yml +++ b/.goreleaser.yml @@ -110,6 +110,7 @@ homebrew_casks: - repository: owner: charmbracelet name: homebrew-tap + token: "{{ .Env.HOMEBREW_TAP_GITHUB_TOKEN }}" npms: - name: "@charmland/crush" @@ -134,6 +135,32 @@ nfpms: - src: ./manpages/crush.1.gz dst: /usr/share/man/man1/crush.1.gz +nix: + - repository: + owner: "charmbracelet" + name: nur + token: "{{ .Env.HOMEBREW_TAP_GITHUB_TOKEN }}" + extra_install: |- + installManPage ./manpages/crush.1.gz. + installShellCompletion ./completions/* + +winget: + - publisher: charmbracelet + copyright: Charmbracelet, Inc + repository: + owner: "charmbracelet" + name: winget-pkgs + token: "{{ .Env.HOMEBREW_TAP_GITHUB_TOKEN }}" + branch: "crush-{{.Version}}" + pull_request: + enabled: true + draft: false + check_boxes: true + base: + owner: microsoft + name: winget-pkgs + branch: master + changelog: sort: asc disable: "{{ .IsNightly }}" From 76c95de579f1412bfdf74261f95524e14179b350 Mon Sep 17 00:00:00 2001 From: Christian Rocha Date: Fri, 25 Jul 2025 13:40:50 -0400 Subject: [PATCH 2/7] =?UTF-8?q?fix:=20don=E2=80=99t=20panic=20when=20fetch?= =?UTF-8?q?ing=20document=20URI=20path=20fails?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: remove hardcoded panic when fetching document URI path * chore: add structured logging to LSP URI conversion errors --- internal/llm/tools/diagnostics.go | 18 ++++++++++-- internal/lsp/client.go | 13 +++++++-- internal/lsp/protocol/pattern_interfaces.go | 20 +++++++++++-- internal/lsp/protocol/uri.go | 25 +++++++++++----- internal/lsp/util/edit.go | 32 +++++++++++++++++---- internal/lsp/watcher/watcher.go | 14 +++++++-- 6 files changed, 101 insertions(+), 21 deletions(-) diff --git a/internal/llm/tools/diagnostics.go b/internal/llm/tools/diagnostics.go index 6d2da798a6547b11f861e8e5051d297809cbb9af..fc9bd211735afd4d1f8a536a90d5705d88bd9790 100644 --- a/internal/llm/tools/diagnostics.go +++ b/internal/llm/tools/diagnostics.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "log/slog" "maps" "sort" "strings" @@ -118,7 +119,13 @@ func waitForLspDiagnostics(ctx context.Context, filePath string, lsps map[string return } - if diagParams.URI.Path() == filePath || hasDiagnosticsChanged(client.GetDiagnostics(), originalDiags) { + path, err := diagParams.URI.Path() + if err != nil { + slog.Error("Failed to convert diagnostic URI to path", "uri", diagParams.URI, "error", err) + return + } + + if path == filePath || hasDiagnosticsChanged(client.GetDiagnostics(), originalDiags) { select { case diagChan <- struct{}{}: default: @@ -216,10 +223,15 @@ func getDiagnostics(filePath string, lsps map[string]*lsp.Client) string { diagnostics := client.GetDiagnostics() if len(diagnostics) > 0 { for location, diags := range diagnostics { - isCurrentFile := location.Path() == filePath + path, err := location.Path() + if err != nil { + slog.Error("Failed to convert diagnostic location URI to path", "uri", location, "error", err) + continue + } + isCurrentFile := path == filePath for _, diag := range diags { - formattedDiag := formatDiagnostic(location.Path(), diag, lspName) + formattedDiag := formatDiagnostic(path, diag, lspName) if isCurrentFile { fileDiagnostics = append(fileDiagnostics, formattedDiag) diff --git a/internal/lsp/client.go b/internal/lsp/client.go index f7298a0d90240c020d451ace1548bef699d1b36d..219a5df5fb87197f0490f218cddc24ab3b138371 100644 --- a/internal/lsp/client.go +++ b/internal/lsp/client.go @@ -449,7 +449,12 @@ func (c *Client) pingTypeScriptServer(ctx context.Context) error { // If we have any open files, try to get document symbols for one for uri := range c.openFiles { - filePath := protocol.DocumentURI(uri).Path() + filePath, err := protocol.DocumentURI(uri).Path() + if err != nil { + slog.Error("Failed to convert URI to path for TypeScript symbol collection", "uri", uri, "error", err) + continue + } + if strings.HasSuffix(filePath, ".ts") || strings.HasSuffix(filePath, ".js") || strings.HasSuffix(filePath, ".tsx") || strings.HasSuffix(filePath, ".jsx") { var symbols []protocol.DocumentSymbol @@ -712,7 +717,11 @@ func (c *Client) CloseAllFiles(ctx context.Context) { // First collect all URIs that need to be closed for uri := range c.openFiles { // Convert URI back to file path using proper URI handling - filePath := protocol.DocumentURI(uri).Path() + filePath, err := protocol.DocumentURI(uri).Path() + if err != nil { + slog.Error("Failed to convert URI to path for file closing", "uri", uri, "error", err) + continue + } filesToClose = append(filesToClose, filePath) } c.openFilesMu.Unlock() diff --git a/internal/lsp/protocol/pattern_interfaces.go b/internal/lsp/protocol/pattern_interfaces.go index 9ee48cb4565cef6bc9168467aaa7e4fdc21a36a8..5cb5dbb84ea385d96ac33fa2075d6590872da3cd 100644 --- a/internal/lsp/protocol/pattern_interfaces.go +++ b/internal/lsp/protocol/pattern_interfaces.go @@ -2,6 +2,7 @@ package protocol import ( "fmt" + "log/slog" ) // PatternInfo is an interface for types that represent glob patterns @@ -36,21 +37,36 @@ func (g *GlobPattern) AsPattern() (PatternInfo, error) { return nil, fmt.Errorf("nil pattern") } + var err error + switch v := g.Value.(type) { case string: return StringPattern{Pattern: v}, nil + case RelativePattern: // Handle BaseURI which could be string or DocumentUri basePath := "" switch baseURI := v.BaseURI.Value.(type) { case string: - basePath = DocumentURI(baseURI).Path() + basePath, err = DocumentURI(baseURI).Path() + if err != nil { + slog.Error("Failed to convert URI to path", "uri", baseURI, "error", err) + return nil, fmt.Errorf("invalid URI: %s", baseURI) + } + case DocumentURI: - basePath = baseURI.Path() + basePath, err = baseURI.Path() + if err != nil { + slog.Error("Failed to convert DocumentURI to path", "uri", baseURI, "error", err) + return nil, fmt.Errorf("invalid DocumentURI: %s", baseURI) + } + default: return nil, fmt.Errorf("unknown BaseURI type: %T", v.BaseURI.Value) } + return RelativePatternInfo{RP: v, BasePath: basePath}, nil + default: return nil, fmt.Errorf("unknown pattern type: %T", g.Value) } diff --git a/internal/lsp/protocol/uri.go b/internal/lsp/protocol/uri.go index a63ed406cc319c2f293406fe4cdd83c237c0c74a..ccc45f23e46b3ea41ac28c525eca6c39c201695e 100644 --- a/internal/lsp/protocol/uri.go +++ b/internal/lsp/protocol/uri.go @@ -70,7 +70,7 @@ func (uri *DocumentURI) UnmarshalText(data []byte) (err error) { // DocumentUri("").Path() returns the empty string. // // Path panics if called on a URI that is not a valid filename. -func (uri DocumentURI) Path() string { +func (uri DocumentURI) Path() (string, error) { filename, err := filename(uri) if err != nil { // e.g. ParseRequestURI failed. @@ -79,22 +79,33 @@ func (uri DocumentURI) Path() string { // direct string manipulation; all DocumentUris // received from the client pass through // ParseRequestURI, which ensures validity. - panic(err) + return "", fmt.Errorf("invalid URI %q: %w", uri, err) } - return filepath.FromSlash(filename) + return filepath.FromSlash(filename), nil } // Dir returns the URI for the directory containing the receiver. -func (uri DocumentURI) Dir() DocumentURI { +func (uri DocumentURI) Dir() (DocumentURI, error) { + // XXX: Legacy comment: // This function could be more efficiently implemented by avoiding any call // to Path(), but at least consolidates URI manipulation. - return URIFromPath(uri.DirPath()) + + path, err := uri.DirPath() + if err != nil { + return "", fmt.Errorf("invalid URI %q: %w", uri, err) + } + + return URIFromPath(path), nil } // DirPath returns the file path to the directory containing this URI, which // must be a file URI. -func (uri DocumentURI) DirPath() string { - return filepath.Dir(uri.Path()) +func (uri DocumentURI) DirPath() (string, error) { + path, err := uri.Path() + if err != nil { + return "", err + } + return filepath.Dir(path), nil } func filename(uri DocumentURI) (string, error) { diff --git a/internal/lsp/util/edit.go b/internal/lsp/util/edit.go index b32eac0c66cffe7d50066a3e5db3e81332e4fa83..12d8e428a7214338bd7ef66c6d71dd512484b243 100644 --- a/internal/lsp/util/edit.go +++ b/internal/lsp/util/edit.go @@ -11,7 +11,10 @@ import ( ) func applyTextEdits(uri protocol.DocumentURI, edits []protocol.TextEdit) error { - path := uri.Path() + path, err := uri.Path() + if err != nil { + return fmt.Errorf("invalid URI: %w", err) + } // Read the file content content, err := os.ReadFile(path) @@ -148,7 +151,11 @@ func applyTextEdit(lines []string, edit protocol.TextEdit) ([]string, error) { // applyDocumentChange applies a DocumentChange (create/rename/delete operations) func applyDocumentChange(change protocol.DocumentChange) error { if change.CreateFile != nil { - path := change.CreateFile.URI.Path() + path, err := change.CreateFile.URI.Path() + if err != nil { + return fmt.Errorf("invalid URI: %w", err) + } + if change.CreateFile.Options != nil { if change.CreateFile.Options.Overwrite { // Proceed with overwrite @@ -164,7 +171,11 @@ func applyDocumentChange(change protocol.DocumentChange) error { } if change.DeleteFile != nil { - path := change.DeleteFile.URI.Path() + path, err := change.DeleteFile.URI.Path() + if err != nil { + return fmt.Errorf("invalid URI: %w", err) + } + if change.DeleteFile.Options != nil && change.DeleteFile.Options.Recursive { if err := os.RemoveAll(path); err != nil { return fmt.Errorf("failed to delete directory recursively: %w", err) @@ -177,8 +188,19 @@ func applyDocumentChange(change protocol.DocumentChange) error { } if change.RenameFile != nil { - oldPath := change.RenameFile.OldURI.Path() - newPath := change.RenameFile.NewURI.Path() + var newPath, oldPath string + var err error + + oldPath, err = change.RenameFile.OldURI.Path() + if err != nil { + return err + } + + newPath, err = change.RenameFile.NewURI.Path() + if err != nil { + return err + } + if change.RenameFile.Options != nil { if !change.RenameFile.Options.Overwrite { if _, err := os.Stat(newPath); err == nil { diff --git a/internal/lsp/watcher/watcher.go b/internal/lsp/watcher/watcher.go index 080870937bee98be852979748dab456fa6a53b66..58b6d41decd1f551c4a474a6921e075b54e75a6e 100644 --- a/internal/lsp/watcher/watcher.go +++ b/internal/lsp/watcher/watcher.go @@ -617,7 +617,11 @@ func (w *WorkspaceWatcher) matchesPattern(path string, pattern protocol.GlobPatt return false } // For relative patterns - basePath = protocol.DocumentURI(basePath).Path() + if basePath, err = protocol.DocumentURI(basePath).Path(); err != nil { + // XXX: Do we want to return here, or send the error up the stack? + slog.Error("Error converting base path to URI", "basePath", basePath, "error", err) + } + basePath = filepath.ToSlash(basePath) // Make path relative to basePath for matching @@ -660,7 +664,13 @@ func (w *WorkspaceWatcher) debounceHandleFileEvent(ctx context.Context, uri stri // handleFileEvent sends file change notifications func (w *WorkspaceWatcher) handleFileEvent(ctx context.Context, uri string, changeType protocol.FileChangeType) { // If the file is open and it's a change event, use didChange notification - filePath := protocol.DocumentURI(uri).Path() + filePath, err := protocol.DocumentURI(uri).Path() + if err != nil { + // XXX: Do we want to return here, or send the error up the stack? + slog.Error("Error converting URI to path", "uri", uri, "error", err) + return + } + if changeType == protocol.FileChangeType(protocol.Deleted) { w.client.ClearDiagnosticsForURI(protocol.DocumentURI(uri)) } else if changeType == protocol.FileChangeType(protocol.Changed) && w.client.IsFileOpen(filePath) { From 98f51b09368485cd3e28ab3b96b5638438fb3889 Mon Sep 17 00:00:00 2001 From: Andrey Nering Date: Fri, 25 Jul 2025 15:57:50 -0300 Subject: [PATCH 3/7] fix: allow to override catwalk url --- internal/config/load.go | 2 +- internal/config/provider.go | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/config/load.go b/internal/config/load.go index 44bcf8e3ce87953b9c3589cacaf2fe8a248e97aa..77f53356b1e529cb5592366e1f2f3a8d757a315f 100644 --- a/internal/config/load.go +++ b/internal/config/load.go @@ -17,7 +17,7 @@ import ( "github.com/charmbracelet/crush/internal/log" ) -const catwalkURL = "https://catwalk.charm.sh" +const defaultCatwalkURL = "https://catwalk.charm.sh" // LoadReader config via io.Reader. func LoadReader(fd io.Reader) (*Config, error) { diff --git a/internal/config/provider.go b/internal/config/provider.go index ba02f9d8e1bc0f2ec58c2ed3e736a87e1d7a614b..e1efc503cac556f529f390c63d7dde51870c729b 100644 --- a/internal/config/provider.go +++ b/internal/config/provider.go @@ -1,6 +1,7 @@ package config import ( + "cmp" "encoding/json" "fmt" "log/slog" @@ -74,6 +75,7 @@ func loadProvidersFromCache(path string) ([]catwalk.Provider, error) { } func Providers() ([]catwalk.Provider, error) { + catwalkURL := cmp.Or(os.Getenv("CATWALK_URL"), defaultCatwalkURL) client := catwalk.NewWithURL(catwalkURL) path := providerCacheFileData() return loadProvidersOnce(client, path) From 2d809a724ca4fe2417d1c57017c51b2a8130b817 Mon Sep 17 00:00:00 2001 From: ras0q Date: Sat, 26 Jul 2025 06:37:17 +0900 Subject: [PATCH 4/7] fix(tui): typo --- internal/tui/page/chat/chat.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/tui/page/chat/chat.go b/internal/tui/page/chat/chat.go index a17cad404c6a22cd71ceba27a11c1ac8ee4c6564..c03a369ae371c0b9fadb9cd170e35618aa5e5b40 100644 --- a/internal/tui/page/chat/chat.go +++ b/internal/tui/page/chat/chat.go @@ -835,7 +835,7 @@ func (p *chatPage) Help() help.KeyMap { ), key.NewBinding( key.WithKeys("g", "home"), - key.WithHelp("g", "hone"), + key.WithHelp("g", "home"), ), key.NewBinding( key.WithKeys("G", "end"), From 4b0f84f8c6a9027bd536628f68ac1cb123cff91e Mon Sep 17 00:00:00 2001 From: Kujtim Hoxha Date: Sat, 26 Jul 2025 12:17:36 +0200 Subject: [PATCH 5/7] chore: address review requests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove Slice() method from csync.Slice and use slices.Collect(s.Seq()) instead - Optimize Seq2() method to avoid calling removed Slice() method - Update Seq() to use Seq2() for better performance - Update all references throughout the codebase to use the new pattern 💖 Generated with Crush Co-Authored-By: Crush --- internal/csync/slices.go | 19 +++++-------------- internal/csync/slices_test.go | 7 ++++--- internal/tui/exp/list/filterable.go | 2 +- internal/tui/exp/list/filterable_group.go | 3 ++- internal/tui/exp/list/grouped.go | 4 +++- internal/tui/exp/list/list.go | 21 +++++++++++---------- 6 files changed, 26 insertions(+), 30 deletions(-) diff --git a/internal/csync/slices.go b/internal/csync/slices.go index ba47062b51de1a038836467f15bd4e9672ab3303..176e53d13585517df72b28969a9ced54383706f9 100644 --- a/internal/csync/slices.go +++ b/internal/csync/slices.go @@ -112,15 +112,6 @@ func (s *Slice[T]) Len() int { return len(s.inner) } -// Slice returns a copy of the underlying slice. -func (s *Slice[T]) Slice() []T { - s.mu.RLock() - defer s.mu.RUnlock() - result := make([]T, len(s.inner)) - copy(result, s.inner) - return result -} - // SetSlice replaces the entire slice with a new one. func (s *Slice[T]) SetSlice(items []T) { s.mu.Lock() @@ -138,10 +129,8 @@ func (s *Slice[T]) Clear() { // Seq returns an iterator that yields elements from the slice. func (s *Slice[T]) Seq() iter.Seq[T] { - // Take a snapshot to avoid holding the lock during iteration - items := s.Slice() return func(yield func(T) bool) { - for _, v := range items { + for _, v := range s.Seq2() { if !yield(v) { return } @@ -151,8 +140,10 @@ func (s *Slice[T]) Seq() iter.Seq[T] { // Seq2 returns an iterator that yields index-value pairs from the slice. func (s *Slice[T]) Seq2() iter.Seq2[int, T] { - // Take a snapshot to avoid holding the lock during iteration - items := s.Slice() + s.mu.RLock() + items := make([]T, len(s.inner)) + copy(items, s.inner) + s.mu.RUnlock() return func(yield func(int, T) bool) { for i, v := range items { if !yield(i, v) { diff --git a/internal/csync/slices_test.go b/internal/csync/slices_test.go index d86b02537fe0534ed60de0e314e1e1d9b7b866b6..ad4d7b408deb55ad68bc62d5327096573df8f8b6 100644 --- a/internal/csync/slices_test.go +++ b/internal/csync/slices_test.go @@ -1,6 +1,7 @@ package csync import ( + "slices" "sync" "sync/atomic" "testing" @@ -145,7 +146,7 @@ func TestSlice(t *testing.T) { assert.Equal(t, 4, s.Len()) expected := []int{1, 2, 4, 5} - actual := s.Slice() + actual := slices.Collect(s.Seq()) assert.Equal(t, expected, actual) // Delete out of bounds @@ -203,7 +204,7 @@ func TestSlice(t *testing.T) { s.SetSlice(newItems) assert.Equal(t, 3, s.Len()) - assert.Equal(t, newItems, s.Slice()) + assert.Equal(t, newItems, slices.Collect(s.Seq())) // Verify it's a copy newItems[0] = 999 @@ -224,7 +225,7 @@ func TestSlice(t *testing.T) { original := []int{1, 2, 3} s := NewSliceFrom(original) - copy := s.Slice() + copy := slices.Collect(s.Seq()) assert.Equal(t, original, copy) // Verify it's a copy diff --git a/internal/tui/exp/list/filterable.go b/internal/tui/exp/list/filterable.go index 909bcf8a42c728aea398bdc16794b63d7d6e725d..b2b7412488d7233236f6d09ac2b7c374a34ef959 100644 --- a/internal/tui/exp/list/filterable.go +++ b/internal/tui/exp/list/filterable.go @@ -97,7 +97,7 @@ func NewFilterableList[T FilterableItem](items []T, opts ...filterableListOption f.list = New[T](items, f.listOptions...).(*list[T]) f.updateKeyMaps() - f.items = f.list.items.Slice() + f.items = slices.Collect(f.list.items.Seq()) if f.inputHidden { return f diff --git a/internal/tui/exp/list/filterable_group.go b/internal/tui/exp/list/filterable_group.go index 2543723b4b74072510c31453441495d27ec05c9d..88b790c678aede588956bce4df002b766646feff 100644 --- a/internal/tui/exp/list/filterable_group.go +++ b/internal/tui/exp/list/filterable_group.go @@ -2,6 +2,7 @@ package list import ( "regexp" + "slices" "sort" "strings" @@ -179,7 +180,7 @@ func (f *filterableGroupList[T]) inputHeight() int { func (f *filterableGroupList[T]) Filter(query string) tea.Cmd { var cmds []tea.Cmd - for _, item := range f.items.Slice() { + for _, item := range slices.Collect(f.items.Seq()) { if i, ok := any(item).(layout.Focusable); ok { cmds = append(cmds, i.Blur()) } diff --git a/internal/tui/exp/list/grouped.go b/internal/tui/exp/list/grouped.go index 4b9124fdd0a0e3335d3b22ef122dc5cd86c5785b..cb54628a70e84cb80eeb162a0d9f836f14271641 100644 --- a/internal/tui/exp/list/grouped.go +++ b/internal/tui/exp/list/grouped.go @@ -1,6 +1,8 @@ package list import ( + "slices" + tea "github.com/charmbracelet/bubbletea/v2" "github.com/charmbracelet/crush/internal/csync" "github.com/charmbracelet/crush/internal/tui/components/core/layout" @@ -89,7 +91,7 @@ func (g *groupedList[T]) convertItems() { func (g *groupedList[T]) SetGroups(groups []Group[T]) tea.Cmd { g.groups = groups g.convertItems() - return g.SetItems(g.items.Slice()) + return g.SetItems(slices.Collect(g.items.Seq())) } func (g *groupedList[T]) Groups() []Group[T] { diff --git a/internal/tui/exp/list/list.go b/internal/tui/exp/list/list.go index 3af90d405382ba5df207774c4f2fba109717034a..8deae123c0df651b21c9f256b2e9ace72a5f7b00 100644 --- a/internal/tui/exp/list/list.go +++ b/internal/tui/exp/list/list.go @@ -1,6 +1,7 @@ package list import ( + "slices" "strings" "github.com/charmbracelet/bubbles/v2/key" @@ -201,7 +202,7 @@ func (l *list[T]) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return l, nil case anim.StepMsg: var cmds []tea.Cmd - for _, item := range l.items.Slice() { + for _, item := range slices.Collect(l.items.Seq()) { if i, ok := any(item).(HasAnim); ok && i.Spinning() { updated, cmd := i.Update(msg) cmds = append(cmds, cmd) @@ -288,7 +289,7 @@ func (l *list[T]) viewPosition() (int, int) { func (l *list[T]) recalculateItemPositions() { currentContentHeight := 0 - for _, item := range l.items.Slice() { + for _, item := range slices.Collect(l.items.Seq()) { rItem, ok := l.renderedItems.Get(item.ID()) if !ok { continue @@ -561,7 +562,7 @@ func (l *list[T]) focusSelectedItem() tea.Cmd { return nil } var cmds []tea.Cmd - for _, item := range l.items.Slice() { + for _, item := range slices.Collect(l.items.Seq()) { if f, ok := any(item).(layout.Focusable); ok { if item.ID() == l.selectedItem && !f.IsFocused() { cmds = append(cmds, f.Focus()) @@ -580,7 +581,7 @@ func (l *list[T]) blurSelectedItem() tea.Cmd { return nil } var cmds []tea.Cmd - for _, item := range l.items.Slice() { + for _, item := range slices.Collect(l.items.Seq()) { if f, ok := any(item).(layout.Focusable); ok { if item.ID() == l.selectedItem && f.IsFocused() { cmds = append(cmds, f.Blur()) @@ -655,7 +656,7 @@ func (l *list[T]) AppendItem(item T) tea.Cmd { l.items.Append(item) l.indexMap = csync.NewMap[string, int]() - for inx, item := range l.items.Slice() { + for inx, item := range slices.Collect(l.items.Seq()) { l.indexMap.Set(item.ID(), inx) } if l.width > 0 && l.height > 0 { @@ -702,7 +703,7 @@ func (l *list[T]) DeleteItem(id string) tea.Cmd { } l.items.Delete(inx) l.renderedItems.Del(id) - for inx, item := range l.items.Slice() { + for inx, item := range slices.Collect(l.items.Seq()) { l.indexMap.Set(item.ID(), inx) } @@ -767,7 +768,7 @@ func (l *list[T]) IsFocused() bool { // Items implements List. func (l *list[T]) Items() []T { - return l.items.Slice() + return slices.Collect(l.items.Seq()) } func (l *list[T]) incrementOffset(n int) { @@ -822,7 +823,7 @@ func (l *list[T]) PrependItem(item T) tea.Cmd { } l.items.Prepend(item) l.indexMap = csync.NewMap[string, int]() - for inx, item := range l.items.Slice() { + for inx, item := range slices.Collect(l.items.Seq()) { l.indexMap.Set(item.ID(), inx) } if l.width > 0 && l.height > 0 { @@ -926,7 +927,7 @@ func (l *list[T]) SelectedItem() *T { func (l *list[T]) SetItems(items []T) tea.Cmd { l.items.SetSlice(items) var cmds []tea.Cmd - for inx, item := range l.items.Slice() { + for inx, item := range slices.Collect(l.items.Seq()) { if i, ok := any(item).(Indexable); ok { i.SetIndex(inx) } @@ -949,7 +950,7 @@ func (l *list[T]) reset(selectedItem string) tea.Cmd { l.selectedItem = selectedItem l.indexMap = csync.NewMap[string, int]() l.renderedItems = csync.NewMap[string, renderedItem]() - for inx, item := range l.items.Slice() { + for inx, item := range slices.Collect(l.items.Seq()) { l.indexMap.Set(item.ID(), inx) if l.width > 0 && l.height > 0 { cmds = append(cmds, item.SetSize(l.width, l.height)) From 42b659d1a26e2e628914c834f54777477cbede81 Mon Sep 17 00:00:00 2001 From: Kujtim Hoxha Date: Sat, 26 Jul 2025 12:38:00 +0200 Subject: [PATCH 6/7] chore: make anim concurrent safe --- internal/format/spinner.go | 4 +- internal/tui/components/anim/anim.go | 111 ++++++++++-------- internal/tui/components/anim/example/main.go | 2 +- .../tui/components/chat/messages/messages.go | 4 +- .../tui/components/chat/sidebar/sidebar.go | 1 + 5 files changed, 67 insertions(+), 55 deletions(-) diff --git a/internal/format/spinner.go b/internal/format/spinner.go index da64fb93ce262e04a0b5fb9da8c4aea8403d10d8..69e443d0f67adadd1e3f9b9a13129850324b6184 100644 --- a/internal/format/spinner.go +++ b/internal/format/spinner.go @@ -20,7 +20,7 @@ type Spinner struct { type model struct { cancel context.CancelFunc - anim anim.Anim + anim *anim.Anim } func (m model) Init() tea.Cmd { return m.anim.Init() } @@ -37,7 +37,7 @@ func (m model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } } mm, cmd := m.anim.Update(msg) - m.anim = mm.(anim.Anim) + m.anim = mm.(*anim.Anim) return m, cmd } diff --git a/internal/tui/components/anim/anim.go b/internal/tui/components/anim/anim.go index 6f69e7b5332c7bbbb1aee6f1191acda5c7ddcfd8..05ac4da98281248d1774a10e95f4d8e2f177e048 100644 --- a/internal/tui/components/anim/anim.go +++ b/internal/tui/components/anim/anim.go @@ -6,7 +6,6 @@ import ( "image/color" "math/rand/v2" "strings" - "sync" "sync/atomic" "time" @@ -15,6 +14,8 @@ import ( tea "github.com/charmbracelet/bubbletea/v2" "github.com/charmbracelet/lipgloss/v2" "github.com/lucasb-eyer/go-colorful" + + "github.com/charmbracelet/crush/internal/csync" ) const ( @@ -72,10 +73,7 @@ type animCache struct { ellipsisFrames []string } -var ( - animCacheMutex sync.RWMutex - animCacheMap = make(map[string]*animCache) -) +var animCacheMap = csync.NewMap[string, *animCache]() // settingsHash creates a hash key for the settings to use for caching func settingsHash(opts Settings) string { @@ -105,22 +103,23 @@ const () type Anim struct { width int cyclingCharWidth int - label []string + label *csync.Slice[string] labelWidth int labelColor color.Color startTime time.Time birthOffsets []time.Duration initialFrames [][]string // frames for the initial characters - initialized bool - cyclingFrames [][]string // frames for the cycling characters - step int // current main frame step - ellipsisStep int // current ellipsis frame step - ellipsisFrames []string // ellipsis animation frames + initialized atomic.Bool + cyclingFrames [][]string // frames for the cycling characters + step atomic.Int64 // current main frame step + ellipsisStep atomic.Int64 // current ellipsis frame step + ellipsisFrames *csync.Slice[string] // ellipsis animation frames id int } // New creates a new Anim instance with the specified width and label. -func New(opts Settings) (a Anim) { +func New(opts Settings) *Anim { + a := &Anim{} // Validate settings. if opts.Size < 1 { opts.Size = defaultNumCyclingChars @@ -142,16 +141,14 @@ func New(opts Settings) (a Anim) { // Check cache first cacheKey := settingsHash(opts) - animCacheMutex.RLock() - cached, exists := animCacheMap[cacheKey] - animCacheMutex.RUnlock() + cached, exists := animCacheMap.Get(cacheKey) if exists { // Use cached values a.width = cached.width a.labelWidth = cached.labelWidth - a.label = cached.label - a.ellipsisFrames = cached.ellipsisFrames + a.label = csync.NewSliceFrom(cached.label) + a.ellipsisFrames = csync.NewSliceFrom(cached.ellipsisFrames) a.initialFrames = cached.initialFrames a.cyclingFrames = cached.cyclingFrames } else { @@ -228,17 +225,23 @@ func New(opts Settings) (a Anim) { } // Cache the results + labelSlice := make([]string, a.label.Len()) + for i, v := range a.label.Seq2() { + labelSlice[i] = v + } + ellipsisSlice := make([]string, a.ellipsisFrames.Len()) + for i, v := range a.ellipsisFrames.Seq2() { + ellipsisSlice[i] = v + } cached = &animCache{ initialFrames: a.initialFrames, cyclingFrames: a.cyclingFrames, width: a.width, labelWidth: a.labelWidth, - label: a.label, - ellipsisFrames: a.ellipsisFrames, + label: labelSlice, + ellipsisFrames: ellipsisSlice, } - animCacheMutex.Lock() - animCacheMap[cacheKey] = cached - animCacheMutex.Unlock() + animCacheMap.Set(cacheKey, cached) } // Random assign a birth to each character for a stagged entrance effect. @@ -269,28 +272,30 @@ func (a *Anim) renderLabel(label string) { if a.labelWidth > 0 { // Pre-render the label. labelRunes := []rune(label) - a.label = make([]string, len(labelRunes)) - for i := range a.label { - a.label[i] = lipgloss.NewStyle(). + a.label = csync.NewSlice[string]() + for i := range labelRunes { + rendered := lipgloss.NewStyle(). Foreground(a.labelColor). Render(string(labelRunes[i])) + a.label.Append(rendered) } // Pre-render the ellipsis frames which come after the label. - a.ellipsisFrames = make([]string, len(ellipsisFrames)) - for i, frame := range ellipsisFrames { - a.ellipsisFrames[i] = lipgloss.NewStyle(). + a.ellipsisFrames = csync.NewSlice[string]() + for _, frame := range ellipsisFrames { + rendered := lipgloss.NewStyle(). Foreground(a.labelColor). Render(frame) + a.ellipsisFrames.Append(rendered) } } else { - a.label = nil - a.ellipsisFrames = nil + a.label = csync.NewSlice[string]() + a.ellipsisFrames = csync.NewSlice[string]() } } // Width returns the total width of the animation. -func (a Anim) Width() (w int) { +func (a *Anim) Width() (w int) { w = a.width if a.labelWidth > 0 { w += labelGapWidth + a.labelWidth @@ -308,12 +313,12 @@ func (a Anim) Width() (w int) { } // Init starts the animation. -func (a Anim) Init() tea.Cmd { +func (a *Anim) Init() tea.Cmd { return a.Step() } // Update processes animation steps (or not). -func (a Anim) Update(msg tea.Msg) (tea.Model, tea.Cmd) { +func (a *Anim) Update(msg tea.Msg) (tea.Model, tea.Cmd) { switch msg := msg.(type) { case StepMsg: if msg.id != a.id { @@ -321,19 +326,19 @@ func (a Anim) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return a, nil } - a.step++ - if a.step >= len(a.cyclingFrames) { - a.step = 0 + step := a.step.Add(1) + if int(step) >= len(a.cyclingFrames) { + a.step.Store(0) } - if a.initialized && a.labelWidth > 0 { + if a.initialized.Load() && a.labelWidth > 0 { // Manage the ellipsis animation. - a.ellipsisStep++ - if a.ellipsisStep >= ellipsisAnimSpeed*len(ellipsisFrames) { - a.ellipsisStep = 0 + ellipsisStep := a.ellipsisStep.Add(1) + if int(ellipsisStep) >= ellipsisAnimSpeed*len(ellipsisFrames) { + a.ellipsisStep.Store(0) } - } else if !a.initialized && time.Since(a.startTime) >= maxBirthOffset { - a.initialized = true + } else if !a.initialized.Load() && time.Since(a.startTime) >= maxBirthOffset { + a.initialized.Store(true) } return a, a.Step() default: @@ -342,35 +347,41 @@ func (a Anim) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } // View renders the current state of the animation. -func (a Anim) View() string { +func (a *Anim) View() string { var b strings.Builder + step := int(a.step.Load()) for i := range a.width { switch { - case !a.initialized && i < len(a.birthOffsets) && time.Since(a.startTime) < a.birthOffsets[i]: + case !a.initialized.Load() && i < len(a.birthOffsets) && time.Since(a.startTime) < a.birthOffsets[i]: // Birth offset not reached: render initial character. - b.WriteString(a.initialFrames[a.step][i]) + b.WriteString(a.initialFrames[step][i]) case i < a.cyclingCharWidth: // Render a cycling character. - b.WriteString(a.cyclingFrames[a.step][i]) + b.WriteString(a.cyclingFrames[step][i]) case i == a.cyclingCharWidth: // Render label gap. b.WriteString(labelGap) case i > a.cyclingCharWidth: // Label. - b.WriteString(a.label[i-a.cyclingCharWidth-labelGapWidth]) + if labelChar, ok := a.label.Get(i - a.cyclingCharWidth - labelGapWidth); ok { + b.WriteString(labelChar) + } } } // Render animated ellipsis at the end of the label if all characters // have been initialized. - if a.initialized && a.labelWidth > 0 { - b.WriteString(a.ellipsisFrames[a.ellipsisStep/ellipsisAnimSpeed]) + if a.initialized.Load() && a.labelWidth > 0 { + ellipsisStep := int(a.ellipsisStep.Load()) + if ellipsisFrame, ok := a.ellipsisFrames.Get(ellipsisStep / ellipsisAnimSpeed); ok { + b.WriteString(ellipsisFrame) + } } return b.String() } // Step is a command that triggers the next step in the animation. -func (a Anim) Step() tea.Cmd { +func (a *Anim) Step() tea.Cmd { return tea.Tick(time.Second/time.Duration(fps), func(t time.Time) tea.Msg { return StepMsg{id: a.id} }) diff --git a/internal/tui/components/anim/example/main.go b/internal/tui/components/anim/example/main.go index 23e2eef1f354ec473e52f54f817de4a2480d82ed..0bf47654ecbeeb3293c8ad59b40ec35016607b1c 100644 --- a/internal/tui/components/anim/example/main.go +++ b/internal/tui/components/anim/example/main.go @@ -56,7 +56,7 @@ func (m model) View() tea.View { return v } - if a, ok := m.anim.(anim.Anim); ok { + if a, ok := m.anim.(*anim.Anim); ok { l := lipgloss.NewLayer(a.View()). Width(a.Width()). X(m.w/2 - a.Width()/2). diff --git a/internal/tui/components/chat/messages/messages.go b/internal/tui/components/chat/messages/messages.go index 9f70691aa9843b8d823b26be247636b31212d2eb..ae8d8fdc4f7b0a8eaca777ffca406d976356e22c 100644 --- a/internal/tui/components/chat/messages/messages.go +++ b/internal/tui/components/chat/messages/messages.go @@ -45,7 +45,7 @@ type messageCmp struct { // Core message data and state message message.Message // The underlying message content spinning bool // Whether to show loading animation - anim anim.Anim // Animation component for loading states + anim *anim.Anim // Animation component for loading states // Thinking viewport for displaying reasoning content thinkingViewport viewport.Model @@ -91,7 +91,7 @@ func (m *messageCmp) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.spinning = m.shouldSpin() if m.spinning { u, cmd := m.anim.Update(msg) - m.anim = u.(anim.Anim) + m.anim = u.(*anim.Anim) return m, cmd } } diff --git a/internal/tui/components/chat/sidebar/sidebar.go b/internal/tui/components/chat/sidebar/sidebar.go index 1aa239bdc15cec6898a4cba1e4dc7a867b5e4ce0..ee6985b6803ad21b600e17c3d2c90cd59f54a827 100644 --- a/internal/tui/components/chat/sidebar/sidebar.go +++ b/internal/tui/components/chat/sidebar/sidebar.go @@ -614,6 +614,7 @@ func (m *sidebarCmp) mcpBlockCompact(maxWidth int) string { } func (m *sidebarCmp) filesBlock() string { + return "" t := styles.CurrentTheme() section := t.S().Subtle.Render( From ff3c926b634d0db1498bb22bd2cb1146af9d7bcd Mon Sep 17 00:00:00 2001 From: Kujtim Hoxha Date: Sat, 26 Jul 2025 12:51:09 +0200 Subject: [PATCH 7/7] chore: fix tool spinning --- internal/tui/components/chat/messages/tool.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/tui/components/chat/messages/tool.go b/internal/tui/components/chat/messages/tool.go index 2f639c5c5d192ba9c59402976e552462d8ebcd0b..c3a075bea088f913d3b0677d5a5a4031bd885a49 100644 --- a/internal/tui/components/chat/messages/tool.go +++ b/internal/tui/components/chat/messages/tool.go @@ -297,7 +297,7 @@ func (m *toolCallCmp) SetSize(width int, height int) tea.Cmd { // shouldSpin determines whether the tool call should show a loading animation. // Returns true if the tool call is not finished or if the result doesn't match the call ID. func (m *toolCallCmp) shouldSpin() bool { - return !m.call.Finished + return !m.call.Finished && !m.cancelled } // Spinning returns whether the tool call is currently showing a loading animation