From 99d11d0e63d54e8461728c8b5bdcdc01a7438f22 Mon Sep 17 00:00:00 2001 From: Rebecca Stambler Date: Tue, 24 Dec 2019 16:16:06 -0500 Subject: [PATCH] internal/lsp: refactor and document options This change cleans up the structure of the Options struct in order to clearly delineate which options should be configurable for the user. Follow-up work is needed to refactor the completion options to fit into this structure, as well as to make sure that the name of the field is the name of the setting. This will make it easier to generate markdown documentation from the code. Also, remove options that are no longer in-use and mark them as deprecated. Change-Id: Ib34ae25789e21b76077a564601e487fbebfc5f48 Reviewed-on: https://go-review.googlesource.com/c/tools/+/212519 Run-TryBot: Rebecca Stambler TryBot-Result: Gobot Gobot Reviewed-by: Heschi Kreinick --- internal/lsp/general.go | 4 +- internal/lsp/source/options.go | 112 ++++++++++++++++++--------- internal/lsp/text_synchronization.go | 5 -- internal/lsp/watched_files.go | 3 - 4 files changed, 77 insertions(+), 47 deletions(-) diff --git a/internal/lsp/general.go b/internal/lsp/general.go index 3da24d1c4b..1b98b58eeb 100644 --- a/internal/lsp/general.go +++ b/internal/lsp/general.go @@ -92,7 +92,7 @@ func (s *Server) initialize(ctx context.Context, params *protocol.ParamInitializ TriggerCharacters: []string{"(", ","}, }, TextDocumentSync: &protocol.TextDocumentSyncOptions{ - Change: options.TextDocumentSyncKind, + Change: protocol.Incremental, OpenClose: true, Save: protocol.SaveOptions{ IncludeText: false, @@ -130,7 +130,7 @@ func (s *Server) initialized(ctx context.Context, params *protocol.InitializedPa ) } - if options.WatchFileChanges && options.DynamicWatchedFilesSupported { + if options.DynamicWatchedFilesSupported { registrations = append(registrations, protocol.Registration{ ID: "workspace/didChangeWatchedFiles", Method: "workspace/didChangeWatchedFiles", diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 2d0aa7f49a..6df8186334 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -43,11 +43,22 @@ import ( var ( DefaultOptions = Options{ - Env: os.Environ(), - TextDocumentSyncKind: protocol.Incremental, - HoverKind: SynopsisDocumentation, - InsertTextFormat: protocol.PlainTextTextFormat, - PreferredContentFormat: protocol.Markdown, + ClientOptions: DefaultClientOptions, + ServerOptions: DefaultServerOptions, + UserOptions: DefaultUserOptions, + DebuggingOptions: DefaultDebuggingOptions, + ExperimentalOptions: DefaultExperimentalOptions, + Hooks: DefaultHooks, + } + DefaultClientOptions = ClientOptions{ + InsertTextFormat: protocol.SnippetTextFormat, + PreferredContentFormat: protocol.Markdown, + ConfigurationSupported: true, + DynamicConfigurationSupported: true, + DynamicWatchedFilesSupported: true, + LineFoldingOnly: false, + } + DefaultServerOptions = ServerOptions{ SupportedCodeActions: map[FileKind]map[protocol.CodeActionKind]bool{ Go: { protocol.SourceOrganizeImports: true, @@ -61,6 +72,10 @@ var ( SupportedCommands: []string{ "tidy", // for go.mod files }, + } + DefaultUserOptions = UserOptions{ + Env: os.Environ(), + HoverKind: SynopsisDocumentation, Completion: CompletionOptions{ Documentation: true, Deep: true, @@ -68,61 +83,84 @@ var ( Literal: true, Budget: 100 * time.Millisecond, }, + LinkTarget: "pkg.go.dev", + } + DefaultHooks = Hooks{ ComputeEdits: myers.ComputeEdits, URLRegexp: regexp.MustCompile(`(http|ftp|https)://([\w_-]+(?:(?:\.[\w_-]+)+))([\w.,@?^=%&:/~+#-]*[\w@?^=%&/~+#-])?`), Analyzers: defaultAnalyzers, GoDiff: true, - LinkTarget: "pkg.go.dev", - TempModfile: false, } + DefaultExperimentalOptions = ExperimentalOptions{ + TempModfile: false, + } + DefaultDebuggingOptions = DebuggingOptions{} ) type Options struct { - // Env is the current set of environment overrides on this view. - Env []string + ClientOptions + ServerOptions + UserOptions + DebuggingOptions + ExperimentalOptions + Hooks +} - // BuildFlags is used to adjust the build flags applied to the view. - BuildFlags []string - - HoverKind HoverKind - DisabledAnalyses map[string]struct{} - - StaticCheck bool - GoDiff bool - - WatchFileChanges bool +type ClientOptions struct { InsertTextFormat protocol.InsertTextFormat ConfigurationSupported bool DynamicConfigurationSupported bool DynamicWatchedFilesSupported bool PreferredContentFormat protocol.MarkupKind LineFoldingOnly bool +} +type ServerOptions struct { SupportedCodeActions map[FileKind]map[protocol.CodeActionKind]bool + SupportedCommands []string +} - SupportedCommands []string +type UserOptions struct { + // Env is the current set of environment overrides on this view. + Env []string - // TODO: Remove the option once we are certain there are no issues here. - TextDocumentSyncKind protocol.TextDocumentSyncKind + // BuildFlags is used to adjust the build flags applied to the view. + BuildFlags []string - Completion CompletionOptions + // HoverKind specifies the format of the content for hover requests. + HoverKind HoverKind - ComputeEdits diff.ComputeEdits - URLRegexp *regexp.Regexp + // DisabledAnalyses specify analyses that the user would like to disable. + DisabledAnalyses map[string]struct{} - Analyzers map[string]*analysis.Analyzer + // StaticCheck enables additional analyses from staticcheck.io. + StaticCheck bool + + // LinkTarget is the website used for documentation. + LinkTarget string // LocalPrefix is used to specify goimports's -local behavior. LocalPrefix string - VerboseOutput bool + Completion CompletionOptions +} +type Hooks struct { + GoDiff bool + ComputeEdits diff.ComputeEdits + URLRegexp *regexp.Regexp + Analyzers map[string]*analysis.Analyzer +} + +type ExperimentalOptions struct { // WARNING: This configuration will be changed in the future. // It only exists while this feature is under development. // Disable use of the -modfile flag in Go 1.14. TempModfile bool +} - LinkTarget string +type DebuggingOptions struct { + VerboseOutput bool } type CompletionOptions struct { @@ -241,12 +279,6 @@ func (o *Options) set(name string, value interface{}) OptionResult { } o.BuildFlags = flags - case "noIncrementalSync": - if v, ok := result.asBool(); ok && v { - o.TextDocumentSyncKind = protocol.Full - } - case "watchFileChanges": - result.setBool(&o.WatchFileChanges) case "completionDocumentation": result.setBool(&o.Completion.Documentation) case "usePlaceholders": @@ -312,9 +344,6 @@ func (o *Options) set(name string, value interface{}) OptionResult { case "staticcheck": result.setBool(&o.StaticCheck) - case "go-diff": - result.setBool(&o.GoDiff) - case "local": localPrefix, ok := value.(string) if !ok { @@ -349,6 +378,15 @@ func (o *Options) set(name string, value interface{}) OptionResult { result.State = OptionDeprecated result.Replacement = "completeUnimported" + case "noIncrementalSync": + result.State = OptionDeprecated + + case "watchFileChanges": + result.State = OptionDeprecated + + case "go-diff": + result.State = OptionDeprecated + default: result.State = OptionUnexpected } diff --git a/internal/lsp/text_synchronization.go b/internal/lsp/text_synchronization.go index 490b0e22e2..17fa2a3740 100644 --- a/internal/lsp/text_synchronization.go +++ b/internal/lsp/text_synchronization.go @@ -132,11 +132,6 @@ func (s *Server) changedText(ctx context.Context, uri span.URI, changes []protoc return []byte(changes[0].Text), nil } - // We only accept an incremental change if that's what the server expects. - if s.session.Options().TextDocumentSyncKind == protocol.Full { - return nil, errors.Errorf("expected a full content change, received incremental changes for %s", uri) - } - return s.applyIncrementalChanges(ctx, uri, changes) } diff --git a/internal/lsp/watched_files.go b/internal/lsp/watched_files.go index fb97d878db..f32074c3f3 100644 --- a/internal/lsp/watched_files.go +++ b/internal/lsp/watched_files.go @@ -20,9 +20,6 @@ func (s *Server) didChangeWatchedFiles(ctx context.Context, params *protocol.Did ctx := telemetry.File.With(ctx, uri) for _, view := range s.session.Views() { - if !view.Options().WatchFileChanges { - continue - } action := toFileAction(change.Type) switch action { case source.Change, source.Create: