Carlos Alexandro Becker created
- used the synchronized client map
- lsps root markers don't need to be gitignore aware
- cache unexisting lsp binaries globally
closes #2223
Signed-off-by: Carlos Alexandro Becker <caarlos0@users.noreply.github.com>internal/agent/tools/glob.go | 2
internal/fsext/fileutil.go | 18 +++++-
internal/fsext/fileutil_test.go | 26 ++++----
internal/lsp/client.go | 2
internal/lsp/manager.go | 92 +++++++++++++++++++---------------
internal/ui/model/lsp.go | 2
6 files changed, 83 insertions(+), 59 deletions(-)
@@ -81,7 +81,7 @@ func globFiles(ctx context.Context, pattern, searchPath string, limit int) ([]st
slog.Warn("Ripgrep execution failed, falling back to doublestar", "error", err)
}
- return fsext.GlobWithDoubleStar(pattern, searchPath, limit)
+ return fsext.GlobGitignoreAware(pattern, searchPath, limit)
}
func runRipgrep(cmd *exec.Cmd, searchRoot string, limit int) ([]string, error) {
@@ -82,7 +82,19 @@ func (w *FastGlobWalker) ShouldSkipDir(path string) bool {
return w.directoryLister.shouldIgnore(path, nil, true)
}
-func GlobWithDoubleStar(pattern, searchPath string, limit int) ([]string, bool, error) {
+// Glob globs files.
+//
+// Does not respect gitignore.
+func Glob(pattern string, cwd string, limit int) ([]string, bool, error) {
+ return globWithDoubleStar(pattern, cwd, limit, false)
+}
+
+// GlobGitignoreAware globs files respecting gitignore.
+func GlobGitignoreAware(pattern string, cwd string, limit int) ([]string, bool, error) {
+ return globWithDoubleStar(pattern, cwd, limit, true)
+}
+
+func globWithDoubleStar(pattern, searchPath string, limit int, gitignore bool) ([]string, bool, error) {
// Normalize pattern to forward slashes on Windows so their config can use
// backslashes
pattern = filepath.ToSlash(pattern)
@@ -101,11 +113,11 @@ func GlobWithDoubleStar(pattern, searchPath string, limit int) ([]string, bool,
isDir := d.IsDir()
if isDir {
- if walker.ShouldSkipDir(path) {
+ if gitignore && walker.ShouldSkipDir(path) {
return filepath.SkipDir
}
} else {
- if walker.ShouldSkip(path) {
+ if gitignore && walker.ShouldSkip(path) {
return nil
}
}
@@ -24,7 +24,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
require.NoError(t, os.WriteFile(file, []byte("test content"), 0o644))
}
- matches, truncated, err := GlobWithDoubleStar("**/main.go", testDir, 0)
+ matches, truncated, err := GlobGitignoreAware("**/main.go", testDir, 0)
require.NoError(t, err)
require.False(t, truncated)
@@ -47,7 +47,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
require.NoError(t, os.WriteFile(filepath.Join(srcDir, "main.go"), []byte("package main"), 0o644))
require.NoError(t, os.WriteFile(pkgFile, []byte("test"), 0o644))
- matches, truncated, err := GlobWithDoubleStar("pkg", testDir, 0)
+ matches, truncated, err := GlobGitignoreAware("pkg", testDir, 0)
require.NoError(t, err)
require.False(t, truncated)
@@ -66,7 +66,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
require.NoError(t, os.MkdirAll(dir, 0o755))
}
- matches, truncated, err := GlobWithDoubleStar("**/pkg", testDir, 0)
+ matches, truncated, err := GlobGitignoreAware("**/pkg", testDir, 0)
require.NoError(t, err)
require.False(t, truncated)
@@ -95,7 +95,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
require.NoError(t, os.WriteFile(file, []byte("package main"), 0o644))
}
- matches, truncated, err := GlobWithDoubleStar("pkg/**", testDir, 0)
+ matches, truncated, err := GlobGitignoreAware("pkg/**", testDir, 0)
require.NoError(t, err)
require.False(t, truncated)
@@ -124,7 +124,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
require.NoError(t, os.WriteFile(file, []byte("test"), 0o644))
}
- matches, truncated, err := GlobWithDoubleStar("**/*.txt", testDir, 5)
+ matches, truncated, err := GlobGitignoreAware("**/*.txt", testDir, 5)
require.NoError(t, err)
require.True(t, truncated, "Expected truncation with limit")
require.Len(t, matches, 5, "Expected exactly 5 matches with limit")
@@ -143,7 +143,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
require.NoError(t, os.WriteFile(file, []byte("test"), 0o644))
}
- matches, truncated, err := GlobWithDoubleStar("a/b/c/file1.txt", testDir, 0)
+ matches, truncated, err := GlobGitignoreAware("a/b/c/file1.txt", testDir, 0)
require.NoError(t, err)
require.False(t, truncated)
@@ -171,7 +171,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
require.NoError(t, os.Chtimes(file2, m2, m2))
require.NoError(t, os.Chtimes(file3, m3, m3))
- matches, truncated, err := GlobWithDoubleStar("*.txt", testDir, 0)
+ matches, truncated, err := GlobGitignoreAware("*.txt", testDir, 0)
require.NoError(t, err)
require.False(t, truncated)
@@ -181,7 +181,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
t.Run("handles empty directory", func(t *testing.T) {
testDir := t.TempDir()
- matches, truncated, err := GlobWithDoubleStar("**", testDir, 0)
+ matches, truncated, err := GlobGitignoreAware("**", testDir, 0)
require.NoError(t, err)
require.False(t, truncated)
// Even empty directories should return the directory itself
@@ -191,7 +191,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
t.Run("handles non-existent search path", func(t *testing.T) {
nonExistentDir := filepath.Join(t.TempDir(), "does", "not", "exist")
- matches, truncated, err := GlobWithDoubleStar("**", nonExistentDir, 0)
+ matches, truncated, err := GlobGitignoreAware("**", nonExistentDir, 0)
require.Error(t, err, "Should return error for non-existent search path")
require.False(t, truncated)
require.Empty(t, matches)
@@ -219,17 +219,17 @@ func TestGlobWithDoubleStar(t *testing.T) {
ignoredFileInDir := filepath.Join(testDir, "backup", "old.txt")
require.NoError(t, os.WriteFile(ignoredFileInDir, []byte("old content"), 0o644))
- matches, truncated, err := GlobWithDoubleStar("*.tmp", testDir, 0)
+ matches, truncated, err := GlobGitignoreAware("*.tmp", testDir, 0)
require.NoError(t, err)
require.False(t, truncated)
require.Empty(t, matches, "Expected no matches for '*.tmp' pattern (should be ignored)")
- matches, truncated, err = GlobWithDoubleStar("backup", testDir, 0)
+ matches, truncated, err = GlobGitignoreAware("backup", testDir, 0)
require.NoError(t, err)
require.False(t, truncated)
require.Empty(t, matches, "Expected no matches for 'backup' pattern (should be ignored)")
- matches, truncated, err = GlobWithDoubleStar("*.txt", testDir, 0)
+ matches, truncated, err = GlobGitignoreAware("*.txt", testDir, 0)
require.NoError(t, err)
require.False(t, truncated)
require.Equal(t, []string{goodFile}, matches)
@@ -257,7 +257,7 @@ func TestGlobWithDoubleStar(t *testing.T) {
require.NoError(t, os.Chtimes(middleDir, tMiddle, tMiddle))
require.NoError(t, os.Chtimes(oldestFile, tNewest, tNewest))
- matches, truncated, err := GlobWithDoubleStar("*.rs", testDir, 0)
+ matches, truncated, err := GlobGitignoreAware("*.rs", testDir, 0)
require.NoError(t, err)
require.False(t, truncated)
require.Len(t, matches, 3)
@@ -85,7 +85,7 @@ func New(
resolver: resolver,
cwd: cwd,
}
- client.serverState.Store(StateStarting)
+ client.serverState.Store(StateStopped)
if err := client.createPowernapClient(); err != nil {
return nil, err
@@ -21,13 +21,14 @@ import (
"github.com/sourcegraph/jsonrpc2"
)
+var unavailable = csync.NewMap[string, struct{}]()
+
// Manager handles lazy initialization of LSP clients based on file types.
type Manager struct {
clients *csync.Map[string, *Client]
cfg *config.Config
manager *powernapconfig.Manager
callback func(name string, client *Client)
- mu sync.Mutex
}
// NewManager creates a new LSP manager service.
@@ -72,17 +73,12 @@ func (s *Manager) Clients() *csync.Map[string, *Client] {
// SetCallback sets a callback that is invoked when a new LSP
// client is successfully started. This allows the coordinator to add LSP tools.
func (s *Manager) SetCallback(cb func(name string, client *Client)) {
- s.mu.Lock()
- defer s.mu.Unlock()
s.callback = cb
}
// TrackConfigured will callback the user-configured LSPs, but will not create
// any clients.
func (s *Manager) TrackConfigured() {
- s.mu.Lock()
- defer s.mu.Unlock()
-
var wg sync.WaitGroup
for name := range s.manager.GetServers() {
if !s.isUserConfigured(name) {
@@ -102,16 +98,10 @@ func (s *Manager) Start(ctx context.Context, path string) {
return
}
- s.mu.Lock()
- defer s.mu.Unlock()
-
var wg sync.WaitGroup
for name, server := range s.manager.GetServers() {
- if !handles(server, path, s.cfg.WorkingDir()) {
- continue
- }
wg.Go(func() {
- s.startServer(ctx, name, server)
+ s.startServer(ctx, name, path, server)
})
}
wg.Wait()
@@ -149,12 +139,31 @@ var skipAutoStartCommands = map[string]bool{
"tflint": true,
}
-func (s *Manager) startServer(ctx context.Context, name string, server *powernapconfig.ServerConfig) {
+func (s *Manager) startServer(ctx context.Context, name, filepath string, server *powernapconfig.ServerConfig) {
+ cfg := s.buildConfig(name, server)
+ if cfg.Disabled {
+ return
+ }
+
+ if _, exists := unavailable.Get(name); exists {
+ return
+ }
+
+ if client, ok := s.clients.Get(name); ok {
+ switch client.GetServerState() {
+ case StateReady, StateStarting, StateDisabled:
+ s.callback(name, client)
+ // already done, return
+ return
+ }
+ }
+
userConfigured := s.isUserConfigured(name)
if !userConfigured {
if _, err := exec.LookPath(server.Command); err != nil {
slog.Debug("LSP server not installed, skipping", "name", name, "command", server.Command)
+ unavailable.Set(name, struct{}{})
return
}
if skipAutoStartCommands[server.Command] {
@@ -163,34 +172,43 @@ func (s *Manager) startServer(ctx context.Context, name string, server *powernap
}
}
- cfg := s.buildConfig(name, server)
- if client, ok := s.clients.Get(name); ok {
- switch client.GetServerState() {
- case StateReady, StateStarting:
- s.callback(name, client)
- // already done, return
- return
- }
+ // this is the slowest bit, so we do it last.
+ if !handles(server, filepath, s.cfg.WorkingDir()) {
+ // nothing to do
+ return
}
- client, err := New(
- ctx,
- name,
- cfg,
- s.cfg.Resolver(),
- s.cfg.WorkingDir(),
- s.cfg.Options.DebugLSP,
- )
+
+ // check again in case another goroutine started it in the meantime
+ var err error
+ client := s.clients.GetOrSet(name, func() *Client {
+ var cli *Client
+ cli, err = New(
+ ctx,
+ name,
+ cfg,
+ s.cfg.Resolver(),
+ s.cfg.WorkingDir(),
+ s.cfg.Options.DebugLSP,
+ )
+ return cli
+ })
if err != nil {
slog.Error("Failed to create LSP client", "name", name, "error", err)
return
}
- s.callback(name, client)
-
defer func() {
- s.clients.Set(name, client)
s.callback(name, client)
}()
+ switch client.GetServerState() {
+ case StateReady, StateStarting:
+ // already done, return
+ return
+ }
+
+ client.serverState.Store(StateStarting)
+ s.callback(name, client)
+
initCtx, cancel := context.WithTimeout(ctx, time.Duration(cmp.Or(cfg.Timeout, 30))*time.Second)
defer cancel()
@@ -271,7 +289,7 @@ func hasRootMarkers(dir string, markers []string) bool {
}
for _, pattern := range markers {
// Use fsext.GlobWithDoubleStar to find matches
- matches, _, err := fsext.GlobWithDoubleStar(pattern, dir, 1)
+ matches, _, err := fsext.Glob(pattern, dir, 1)
if err == nil && len(matches) > 0 {
return true
}
@@ -291,9 +309,6 @@ func handles(server *powernapconfig.ServerConfig, filePath, workDir string) bool
// in the middle of writing something.
// Generally it doesn't matter when shutting down Crush, though.
func (s *Manager) KillAll(context.Context) {
- s.mu.Lock()
- defer s.mu.Unlock()
-
var wg sync.WaitGroup
for name, client := range s.clients.Seq2() {
wg.Go(func() {
@@ -308,9 +323,6 @@ func (s *Manager) KillAll(context.Context) {
// StopAll stops all running LSP clients and clears the client map.
func (s *Manager) StopAll(ctx context.Context) {
- s.mu.Lock()
- defer s.mu.Unlock()
-
var wg sync.WaitGroup
for name, client := range s.clients.Seq2() {
wg.Go(func() {
@@ -108,7 +108,7 @@ func lspList(t *styles.Styles, lsps []LSPInfo, width, maxItems int) string {
icon = t.ResourceOfflineIcon.Foreground(t.Muted.GetBackground()).String()
description = t.ResourceStatus.Render("disabled")
default:
- icon = t.ResourceOfflineIcon.String()
+ continue
}
renderedLsps = append(renderedLsps, common.Status(t, common.StatusOpts{
Icon: icon,