d6643a6 · crush · git.iko.soy

git.iko.soy/crush

on main help sign in

fix(lsp): files outside cwd (#2180)

d6643a6Carlos Alexandro Becker8 months ago

internal/lsp/client.go+15−50

compare with main

unifiedsplit

@@ -8,13 +8,13 @@ import (

8 "maps"

9 "os"

10 "path/filepath"

11− "strings"

12 "sync"

11 "sync"

13 "sync/atomic"

12 "sync/atomic"

14 "time"

13 "time"

16 "github.com/charmbracelet/crush/internal/config"

15 "github.com/charmbracelet/crush/internal/config"

17 "github.com/charmbracelet/crush/internal/csync"

16 "github.com/charmbracelet/crush/internal/csync"

17+ "github.com/charmbracelet/crush/internal/fsext"

18 "github.com/charmbracelet/crush/internal/home"

19 powernap "github.com/charmbracelet/x/powernap/pkg/lsp"

20 "github.com/charmbracelet/x/powernap/pkg/lsp/protocol"

@@ -35,7 +35,7 @@ type Client struct {

35 debug bool

37 // Working directory this LSP is scoped to.

38− workDir string

38+ cwd string

40 // File types this LSP server handles (e.g., .go, .rs, .py)

41 fileTypes []string

@@ -66,7 +66,14 @@ type Client struct {

66}

68// New creates a new LSP client using the powernap implementation.

69+func New(

70+ ctx context.Context,

71+ name string,

72+ cfg config.LSPConfig,

69−func New(ctx context.Context, name string, cfg config.LSPConfig, resolver config.VariableResolver, debug bool) (*Client, error) {

73+ resolver config.VariableResolver,

74+ cwd string,

75+ debug bool,

76+) (*Client, error) {

70 client := &Client{

77 client := &Client{

71 name: name,

78 name: name,

72 fileTypes: cfg.FileTypes,

79 fileTypes: cfg.FileTypes,

@@ -76,6 +83,7 @@ func New(ctx context.Context, name string, cfg config.LSPConfig, resolver config

76 ctx: ctx,

83 ctx: ctx,

77 debug: debug,

84 debug: debug,

78 resolver: resolver,

85 resolver: resolver,

86+ cwd: cwd,

79 }

87 }

80 client.serverState.Store(StateStarting)

88 client.serverState.Store(StateStarting)

@@ -134,13 +142,7 @@ func (c *Client) Close(ctx context.Context) error {

134

142

135// createPowernapClient creates a new powernap client with the current configuration.

143// createPowernapClient creates a new powernap client with the current configuration.

136func (c *Client) createPowernapClient() error {

144func (c *Client) createPowernapClient() error {

137− workDir, err := os.Getwd()

138− if err != nil {

139− return fmt.Errorf("failed to get working directory: %w", err)

140− }

141−

142− rootURI := string(protocol.URIFromPath(workDir))

145+ rootURI := string(protocol.URIFromPath(c.cwd))

143− c.workDir = workDir

144

146

145 command, err := c.resolver.ResolveValue(c.config.Command)

147 command, err := c.resolver.ResolveValue(c.config.Command)

146 if err != nil {

148 if err != nil {

@@ -157,7 +159,7 @@ func (c *Client) createPowernapClient() error {

157 WorkspaceFolders: []protocol.WorkspaceFolder{

159 WorkspaceFolders: []protocol.WorkspaceFolder{

158 {

160 {

159 URI: rootURI,

161 URI: rootURI,

160− Name: filepath.Base(workDir),

162+ Name: filepath.Base(c.cwd),

161 },

163 },

162 },

164 },

163 }

165 }

@@ -321,15 +323,8 @@ type OpenFileInfo struct {

321// HandlesFile checks if this LSP client handles the given file based on its

323// HandlesFile checks if this LSP client handles the given file based on its

322// extension and whether it's within the working directory.

324// extension and whether it's within the working directory.

323func (c *Client) HandlesFile(path string) bool {

325func (c *Client) HandlesFile(path string) bool {

324− // Check if file is within working directory.

326+ if !fsext.HasPrefix(path, c.cwd) {

325− absPath, err := filepath.Abs(path)

326− if err != nil {

327− slog.Debug("Cannot resolve path", "name", c.name, "file", path, "error", err)

328− return false

329− }

330− relPath, err := filepath.Rel(c.workDir, absPath)

331− if err != nil || strings.HasPrefix(relPath, "..") {

332− slog.Debug("File outside workspace", "name", c.name, "file", path, "workDir", c.workDir)

327+ slog.Debug("File outside workspace", "name", c.name, "file", path, "workDir", c.cwd)

333 return false

328 return false

334 }

329 }

335 return handlesFiletype(c.name, c.fileTypes, path)

330 return handlesFiletype(c.name, c.fileTypes, path)

@@ -472,31 +467,6 @@ func (c *Client) OpenFileOnDemand(ctx context.Context, filepath string) error {

472 return c.OpenFile(ctx, filepath)

467 return c.OpenFile(ctx, filepath)

473}

468}

474

469

475−// GetDiagnosticsForFile ensures a file is open and returns its diagnostics.

476−func (c *Client) GetDiagnosticsForFile(ctx context.Context, filepath string) ([]protocol.Diagnostic, error) {

477− documentURI := protocol.URIFromPath(filepath)

478−

479− // Make sure the file is open

480− if !c.IsFileOpen(filepath) {

481− if err := c.OpenFile(ctx, filepath); err != nil {

482− return nil, fmt.Errorf("failed to open file for diagnostics: %w", err)

483− }

484−

485− // Give the LSP server a moment to process the file

486− time.Sleep(100 * time.Millisecond)

487− }

488−

489− // Get diagnostics

490− diagnostics, _ := c.diagnostics.Get(documentURI)

491−

492− return diagnostics, nil

493−}

494−

495−// ClearDiagnosticsForURI removes diagnostics for a specific URI from the cache.

496−func (c *Client) ClearDiagnosticsForURI(uri protocol.DocumentURI) {

497− c.diagnostics.Del(uri)

498−}

499−

500// RegisterNotificationHandler registers a notification handler.

470// RegisterNotificationHandler registers a notification handler.

501func (c *Client) RegisterNotificationHandler(method string, handler transport.NotificationHandler) {

471func (c *Client) RegisterNotificationHandler(method string, handler transport.NotificationHandler) {

502 c.client.RegisterNotificationHandler(method, handler)

472 c.client.RegisterNotificationHandler(method, handler)

@@ -507,11 +477,6 @@ func (c *Client) RegisterServerRequestHandler(method string, handler transport.H

507 c.client.RegisterHandler(method, handler)

477 c.client.RegisterHandler(method, handler)

508}

478}

509

479

510−// DidChangeWatchedFiles sends a workspace/didChangeWatchedFiles notification to the server.

511−func (c *Client) DidChangeWatchedFiles(ctx context.Context, params protocol.DidChangeWatchedFilesParams) error {

512− return c.client.NotifyDidChangeWatchedFiles(ctx, params.Changes)

513−}

514−

515// openKeyConfigFiles opens important configuration files that help initialize the server.

480// openKeyConfigFiles opens important configuration files that help initialize the server.

516func (c *Client) openKeyConfigFiles(ctx context.Context) {

481func (c *Client) openKeyConfigFiles(ctx context.Context) {

517 wd, err := os.Getwd()

482 wd, err := os.Getwd()

internal/lsp/client_test.go+1−1

@@ -23,7 +23,7 @@ func TestClient(t *testing.T) {

23 // but we can still test the basic structure

24 client, err := New(ctx, "test", cfg, config.NewEnvironmentVariableResolver(env.NewFromMap(map[string]string{

25 "THE_CMD": "echo",

26− })), false)

26+ })), ".", false)

27 if err != nil {

28 // Expected to fail with echo command, skip the rest

29 t.Skipf("Powernap client creation failed as expected with dummy command: %v", err)

internal/lsp/manager.go+16−5

@@ -65,8 +65,8 @@ func NewManager(cfg *config.Config) *Manager {

65}

67// Clients returns the map of LSP clients.

68−func (m *Manager) Clients() *csync.Map[string, *Client] {

68+func (s *Manager) Clients() *csync.Map[string, *Client] {

69− return m.clients

69+ return s.clients

70}

72// SetCallback sets a callback that is invoked when a new LSP

@@ -79,13 +79,17 @@ func (s *Manager) SetCallback(cb func(name string, client *Client)) {

80// Start starts an LSP server that can handle the given file path.

81// If an appropriate LSP is already running, this is a no-op.

82−func (s *Manager) Start(ctx context.Context, filePath string) {

82+func (s *Manager) Start(ctx context.Context, path string) {

83+ if !fsext.HasPrefix(path, s.cfg.WorkingDir()) {

84+ return

85+ }

86+

83 s.mu.Lock()

87 s.mu.Lock()

84 defer s.mu.Unlock()

88 defer s.mu.Unlock()

86 var wg sync.WaitGroup

90 var wg sync.WaitGroup

87 for name, server := range s.manager.GetServers() {

91 for name, server := range s.manager.GetServers() {

88− if !handles(server, filePath, s.cfg.WorkingDir()) {

92+ if !handles(server, path, s.cfg.WorkingDir()) {

89 continue

93 continue

90 }

94 }

91 wg.Go(func() {

95 wg.Go(func() {

@@ -150,7 +154,14 @@ func (s *Manager) startServer(ctx context.Context, name string, server *powernap

150 return

154 return

151 }

155 }

152 }

156 }

157+ client, err := New(

158+ ctx,

159+ name,

160+ cfg,

161+ s.cfg.Resolver(),

162+ s.cfg.WorkingDir(),

153− client, err := New(ctx, name, cfg, s.cfg.Resolver(), s.cfg.Options.DebugLSP)

163+ s.cfg.Options.DebugLSP,

164+ )

154 if err != nil {

165 if err != nil {

155 slog.Error("Failed to create LSP client", "name", name, "error", err)

166 slog.Error("Failed to create LSP client", "name", name, "error", err)

156 return

167 return