fb21333 · crush · git.iko.soy

git.iko.soy/crush

on main help sign in

refactor: remove weird context value usage (#153)

fb21333Carlos Alexandro Becker1 year ago

internal/app/lsp.go+1−4

compare with main

unifiedsplit

@@ -59,11 +59,8 @@ func (app *App) createAndStartLSPClient(ctx context.Context, name string, comman

59 // Create a child context that can be canceled when the app is shutting down

60 watchCtx, cancelFunc := context.WithCancel(ctx)

62− // Create a context with the server name for better identification

63− watchCtx = context.WithValue(watchCtx, "serverName", name)

64−

65 // Create the workspace watcher

62 // Create the workspace watcher

66− workspaceWatcher := watcher.NewWorkspaceWatcher(lspClient)

63+ workspaceWatcher := watcher.NewWorkspaceWatcher(name, lspClient)

68 // Store the cancel function to be called during cleanup

65 // Store the cancel function to be called during cleanup

69 app.cancelFuncsMutex.Lock()

66 app.cancelFuncsMutex.Lock()

internal/lsp/watcher/watcher.go+55−98

@@ -21,6 +21,7 @@ import (

21// WorkspaceWatcher manages LSP file watching

22type WorkspaceWatcher struct {

23 client *lsp.Client

24+ name string

24 workspacePath string

25 workspacePath string

26 debounceTime time.Duration

27 debounceTime time.Duration

@@ -33,8 +34,9 @@ type WorkspaceWatcher struct {

33}

34}

35// NewWorkspaceWatcher creates a new workspace watcher

36// NewWorkspaceWatcher creates a new workspace watcher

36−func NewWorkspaceWatcher(client *lsp.Client) *WorkspaceWatcher {

37+func NewWorkspaceWatcher(name string, client *lsp.Client) *WorkspaceWatcher {

37 return &WorkspaceWatcher{

38 return &WorkspaceWatcher{

39+ name: name,

38 client: client,

40 client: client,

39 debounceTime: 300 * time.Millisecond,

41 debounceTime: 300 * time.Millisecond,

40 debounceMap: make(map[string]*time.Timer),

42 debounceMap: make(map[string]*time.Timer),

@@ -95,7 +97,7 @@ func (w *WorkspaceWatcher) AddRegistrations(ctx context.Context, id string, watc

95 }

97 }

97 // Determine server type for specialized handling

99 // Determine server type for specialized handling

98− serverName := getServerNameFromContext(ctx)

100+ serverName := w.name

99 slog.Debug("Server type detected", "serverName", serverName)

101 slog.Debug("Server type detected", "serverName", serverName)

100

102

101 // Check if this server has sent file watchers

103 // Check if this server has sent file watchers

@@ -325,17 +327,7 @@ func (w *WorkspaceWatcher) WatchWorkspace(ctx context.Context, workspacePath str

325 cfg := config.Get()

327 cfg := config.Get()

326 w.workspacePath = workspacePath

328 w.workspacePath = workspacePath

327

329

328− // Store the watcher in the context for later use

329− ctx = context.WithValue(ctx, "workspaceWatcher", w)

330−

331− // If the server name isn't already in the context, try to detect it

332− if _, ok := ctx.Value("serverName").(string); !ok {

333− serverName := getServerNameFromContext(ctx)

334− ctx = context.WithValue(ctx, "serverName", serverName)

335− }

336−

337− serverName := getServerNameFromContext(ctx)

338− slog.Debug("Starting workspace watcher", "workspacePath", workspacePath, "serverName", serverName)

330+ slog.Debug("Starting workspace watcher", "workspacePath", workspacePath, "serverName", w.name)

339

331

340 // Register handler for file watcher registrations from the server

332 // Register handler for file watcher registrations from the server

341 lsp.RegisterFileWatchHandler(func(id string, watchers []protocol.FileSystemWatcher) {

333 lsp.RegisterFileWatchHandler(func(id string, watchers []protocol.FileSystemWatcher) {

@@ -697,40 +689,6 @@ func (w *WorkspaceWatcher) notifyFileEvent(ctx context.Context, uri string, chan

697 return w.client.DidChangeWatchedFiles(ctx, params)

689 return w.client.DidChangeWatchedFiles(ctx, params)

698}

690}

699

691

700−// getServerNameFromContext extracts the server name from the context

701−// This is a best-effort function that tries to identify which LSP server we're dealing with

702−func getServerNameFromContext(ctx context.Context) string {

703− // First check if the server name is directly stored in the context

704− if serverName, ok := ctx.Value("serverName").(string); ok && serverName != "" {

705− return strings.ToLower(serverName)

706− }

707−

708− // Otherwise, try to extract server name from the client command path

709− if w, ok := ctx.Value("workspaceWatcher").(*WorkspaceWatcher); ok && w != nil && w.client != nil && w.client.Cmd != nil {

710− path := strings.ToLower(w.client.Cmd.Path)

711−

712− // Extract server name from path

713− if strings.Contains(path, "typescript") || strings.Contains(path, "tsserver") || strings.Contains(path, "vtsls") {

714− return "typescript"

715− } else if strings.Contains(path, "gopls") {

716− return "gopls"

717− } else if strings.Contains(path, "rust-analyzer") {

718− return "rust-analyzer"

719− } else if strings.Contains(path, "pyright") || strings.Contains(path, "pylsp") || strings.Contains(path, "python") {

720− return "python"

721− } else if strings.Contains(path, "clangd") {

722− return "clangd"

723− } else if strings.Contains(path, "jdtls") || strings.Contains(path, "java") {

724− return "java"

725− }

726−

727− // Return the base name as fallback

728− return filepath.Base(path)

729− }

730−

731− return "unknown"

732−}

733−

734// shouldPreloadFiles determines if we should preload files for a specific language server

692// shouldPreloadFiles determines if we should preload files for a specific language server

735// Some servers work better with preloaded files, others don't need it

693// Some servers work better with preloaded files, others don't need it

736func shouldPreloadFiles(serverName string) bool {

694func shouldPreloadFiles(serverName string) bool {

@@ -884,64 +842,63 @@ func (w *WorkspaceWatcher) openMatchingFile(ctx context.Context, path string) {

884 }

842 }

885

843

886 // Check if this path should be watched according to server registrations

844 // Check if this path should be watched according to server registrations

887− if watched, _ := w.isPathWatched(path); watched {

845+ if watched, _ := w.isPathWatched(path); !watched {

888− // Get server name for specialized handling

846+ return

889− serverName := getServerNameFromContext(ctx)

847+ }

890

848

891− // Check if the file is a high-priority file that should be opened immediately

849+ serverName := w.name

892− // This helps with project initialization for certain language servers

893− if isHighPriorityFile(path, serverName) {

894− if cfg.Options.DebugLSP {

895− slog.Debug("Opening high-priority file", "path", path, "serverName", serverName)

896− }

897− if err := w.client.OpenFile(ctx, path); err != nil && cfg.Options.DebugLSP {

898− slog.Error("Error opening high-priority file", "path", path, "error", err)

899− }

900− return

901− }

902

850

903− // For non-high-priority files, we'll use different strategies based on server type

851+ // Get server name for specialized handling

904− if shouldPreloadFiles(serverName) {

852+ // Check if the file is a high-priority file that should be opened immediately

905− // For servers that benefit from preloading, open files but with limits

853+ // This helps with project initialization for certain language servers

854+ if isHighPriorityFile(path, serverName) {

855+ if cfg.Options.DebugLSP {

856+ slog.Debug("Opening high-priority file", "path", path, "serverName", serverName)

857+ }

858+ if err := w.client.OpenFile(ctx, path); err != nil && cfg.Options.DebugLSP {

859+ slog.Error("Error opening high-priority file", "path", path, "error", err)

860+ }

861+ return

862+ }

906

863

907− // Check file size - for preloading we're more conservative

864+ // For non-high-priority files, we'll use different strategies based on server type

908− if info.Size() > (1 * 1024 * 1024) { // 1MB limit for preloaded files

865+ if !shouldPreloadFiles(serverName) {

909− if cfg.Options.DebugLSP {

910− slog.Debug("Skipping large file for preloading", "path", path, "size", info.Size())

911− }

912− return

866+ return

913− }

867+ }

868+ // For servers that benefit from preloading, open files but with limits

914

869

915− // Check file extension for common source files

870+ // Check file size - for preloading we're more conservative

916− ext := strings.ToLower(filepath.Ext(path))

871+ if info.Size() > (1 * 1024 * 1024) { // 1MB limit for preloaded files

872+ if cfg.Options.DebugLSP {

873+ slog.Debug("Skipping large file for preloading", "path", path, "size", info.Size())

874+ }

875+ return

876+ }

917

877

918− // Only preload source files for the specific language

878+ // Check file extension for common source files

919− shouldOpen := false

879+ ext := strings.ToLower(filepath.Ext(path))

920

880

881+ // Only preload source files for the specific language

882+ var shouldOpen bool

921− switch serverName {

883+ switch serverName {

922− case "typescript", "typescript-language-server", "tsserver", "vtsls":

884+ case "typescript", "typescript-language-server", "tsserver", "vtsls":

923− shouldOpen = ext == ".ts" || ext == ".js" || ext == ".tsx" || ext == ".jsx"

885+ shouldOpen = ext == ".ts" || ext == ".js" || ext == ".tsx" || ext == ".jsx"

924− case "gopls":

886+ case "gopls":

925− shouldOpen = ext == ".go"

887+ shouldOpen = ext == ".go"

926− case "rust-analyzer":

888+ case "rust-analyzer":

927− shouldOpen = ext == ".rs"

889+ shouldOpen = ext == ".rs"

928− case "python", "pyright", "pylsp":

890+ case "python", "pyright", "pylsp":

929− shouldOpen = ext == ".py"

891+ shouldOpen = ext == ".py"

930− case "clangd":

892+ case "clangd":

931− shouldOpen = ext == ".c" || ext == ".cpp" || ext == ".h" || ext == ".hpp"

893+ shouldOpen = ext == ".c" || ext == ".cpp" || ext == ".h" || ext == ".hpp"

932− case "java", "jdtls":

894+ case "java", "jdtls":

933− shouldOpen = ext == ".java"

895+ shouldOpen = ext == ".java"

934− default:

935− // For unknown servers, be conservative

936− shouldOpen = false

937− }

896+ }

938

897

939− if shouldOpen {

898+ if shouldOpen {

940− // Don't need to check if it's already open - the client.OpenFile handles that

899+ // Don't need to check if it's already open - the client.OpenFile handles that

941− if err := w.client.OpenFile(ctx, path); err != nil && cfg.Options.DebugLSP {

900+ if err := w.client.OpenFile(ctx, path); err != nil && cfg.Options.DebugLSP {

942− slog.Error("Error opening file", "path", path, "error", err)

901+ slog.Error("Error opening file", "path", path, "error", err)

943− }

944− }

945 }

902 }

946 }

903 }

947}

904}