From d7beb2dd4d1d1a1b2cc1aa42d218eda902892c48 Mon Sep 17 00:00:00 2001 From: Radek Simko Date: Thu, 21 Jul 2022 14:54:45 +0100 Subject: [PATCH] settings: break down indexing options --- docs/SETTINGS.md | 44 +++++++++++-------- docs/language-clients.md | 21 +++++++++ internal/langserver/handlers/initialize.go | 10 ++--- .../langserver/handlers/initialize_test.go | 4 +- internal/settings/settings.go | 14 +++--- internal/settings/settings_test.go | 22 +++++++--- 6 files changed, 79 insertions(+), 36 deletions(-) diff --git a/docs/SETTINGS.md b/docs/SETTINGS.md index 3b1e528d9..3595c8ba7 100644 --- a/docs/SETTINGS.md +++ b/docs/SETTINGS.md @@ -1,5 +1,11 @@ # Settings +## Data Structures + +We use (JSON) objects to group some config options. Where applicable and necessary, we refer to nested fields using the `.` as a separator. i.e. A hypothetical `bar` option under `{"foo": { "bar": "..." } }` would be referred to as `foo.bar`. + +Clients which expose these config options to the end-user are advised to match the option names and, if possible data structures. Some clients (VS Code extension) may however use flat structure, such as `{"foo.bar": "..."}` if using objects is not possible or practical. + ## Supported Options The language server supports the following configuration options: @@ -35,9 +41,11 @@ you can just add that folder to the workspace and it will be indexed as usual. ## **DEPRECATED**: `excludeModulePaths` (`[]string`) -Deprecated in favour of `ignorePaths` +Deprecated in favour of `indexing.ignorePaths` -## `indexing.ignorePaths` (`[]string`) +## `indexing` (object `{}`) + +### `ignorePaths` (`[]string`) Paths to ignore when indexing the workspace on initialization. This can serve as an escape hatch in large workspaces. Key side effect of ignoring a path @@ -52,6 +60,22 @@ of the target platform (e.g. `\` on Windows, or `/` on Unix), symlinks are followed, trailing slashes automatically removed, and `~` is replaced with your home directory. +## `ignoreDirectoryNames` (`[]string`) + +This allows excluding directories from being indexed upon initialization by passing a list of directory names. + +The following list of directories will always be ignored: + +- `.git` +- `.idea` +- `.vscode` +- `terraform.tfstate.d` +- `.terragrunt-cache` + +## **DEPRECATED**: `ignoreDirectoryNames` (`[]string`) + +Deprecated in favour of `indexing.ignoreDirectoryNames` + ## `commandPrefix` Some clients such as VS Code keep a global registry of commands published by language @@ -72,22 +96,6 @@ Or if left empty This setting should be deprecated once the language server supports multiple workspaces, as this arises in VS code because a server instance is started per VS Code workspace. -## **DEPRECATED**: `ignoreDirectoryNames` (`[]string`) - -Deprecated in favour of `indexing.ignoreDirectoryNames` - -## `indexing.ignoreDirectoryNames` (`[]string`) - -This allows excluding directories from being indexed upon initialization by passing a list of directory names. - -The following list of directories will always be ignored: - -- `.git` -- `.idea` -- `.vscode` -- `terraform.tfstate.d` -- `.terragrunt-cache` - ## `ignoreSingleFileWarning` (`bool`) This setting controls whether terraform-ls sends a warning about opening up a single Terraform file instead of a Terraform folder. Setting this to `true` will prevent the message being sent. The default value is `false`. diff --git a/docs/language-clients.md b/docs/language-clients.md index 56ce02b0f..3f605220c 100644 --- a/docs/language-clients.md +++ b/docs/language-clients.md @@ -16,6 +16,27 @@ Clients specifically should **not** send `*.tf.json`, `*.tfvars.json` nor Packer HCL config nor any other HCL config files as the server is not equipped to handle these file types. +## Configuration + +Unless the client allows the end-user to pass arbitrary config options (e.g. +generic Sublime Text LSP package without Terraform LSP package), the client +should expose configuration as per [SETTINGS.md](./SETTINGS.md). + +Client should match the option names exactly, and if possible match the +underlying data structures too. i.e. if a field is documented as `ignoreDirectoryNames`, +it should be exposed as `ignoreDirectoryNames`, **not** ~`IgnoreDirectoryNames`~, +or ~`ignore_directory_names`~. This is to avoid user confusion when the server +refers to any config option in informative, warning, or error messages. + +Client may use a flat structure using the `.` (single dot) as a separator between +the object name and option nested under it, such as `{ "foo.bar": "..." }` instead +of `{ "foo": { "bar": "..." } }`. This is acceptable in situations when using +objects is not possible or practical (e.g. VS Code wouldn't display objects +in the Settings UI). + +The server will generally refer to options using the `.` address, for simplicity +and avoidance of doubts. + ## Watched Files The server (`>= 0.28.0`) supports `workspace/didChangeWatchedFiles` notifications. diff --git a/internal/langserver/handlers/initialize.go b/internal/langserver/handlers/initialize.go index 3ae95a594..5baaad84e 100644 --- a/internal/langserver/handlers/initialize.go +++ b/internal/langserver/handlers/initialize.go @@ -186,8 +186,8 @@ func getTelemetryProperties(out *settings.DecodedOptions) map[string]interface{} properties["options.rootModulePaths"] = len(out.Options.XLegacyModulePaths) > 0 properties["options.excludeModulePaths"] = len(out.Options.XLegacyExcludeModulePaths) > 0 properties["options.commandPrefix"] = len(out.Options.CommandPrefix) > 0 - properties["options.indexing.ignoreDirectoryNames"] = len(out.Options.IgnoreDirectoryNames) > 0 - properties["options.indexing.ignorePaths"] = len(out.Options.IgnorePaths) > 0 + properties["options.indexing.ignoreDirectoryNames"] = len(out.Options.Indexing.IgnoreDirectoryNames) > 0 + properties["options.indexing.ignorePaths"] = len(out.Options.Indexing.IgnorePaths) > 0 properties["options.experimentalFeatures.prefillRequiredFields"] = out.Options.ExperimentalFeatures.PrefillRequiredFields properties["options.experimentalFeatures.validateOnSave"] = out.Options.ExperimentalFeatures.ValidateOnSave properties["options.ignoreSingleFileWarning"] = out.Options.IgnoreSingleFileWarning @@ -264,7 +264,7 @@ func (svc *service) setupWalker(ctx context.Context, params lsp.InitializeParams } var ignoredPaths []string - for _, rawPath := range options.IgnorePaths { + for _, rawPath := range options.Indexing.IgnorePaths { modPath, err := resolvePath(root.Path(), rawPath) if err != nil { jrpc2.ServerFromContext(ctx).Notify(ctx, "window/showMessage", &lsp.ShowMessageParams{ @@ -307,9 +307,9 @@ func (svc *service) setupWalker(ctx context.Context, params lsp.InitializeParams } } - svc.closedDirWalker.SetIgnoredDirectoryNames(options.IgnoreDirectoryNames) + svc.closedDirWalker.SetIgnoredDirectoryNames(options.Indexing.IgnoreDirectoryNames) svc.closedDirWalker.SetIgnoredPaths(ignoredPaths) - svc.openDirWalker.SetIgnoredDirectoryNames(options.IgnoreDirectoryNames) + svc.openDirWalker.SetIgnoredDirectoryNames(options.Indexing.IgnoreDirectoryNames) svc.openDirWalker.SetIgnoredPaths(ignoredPaths) return nil diff --git a/internal/langserver/handlers/initialize_test.go b/internal/langserver/handlers/initialize_test.go index de48418ff..d69a12064 100644 --- a/internal/langserver/handlers/initialize_test.go +++ b/internal/langserver/handlers/initialize_test.go @@ -196,7 +196,9 @@ func TestInitialize_ignoreDirectoryNames(t *testing.T) { "rootUri": %q, "processId": 12345, "initializationOptions": { - "indexing.ignoreDirectoryNames": [%q] + "indexing": { + "ignoreDirectoryNames": [%q] + } } }`, tmpDir.URI, "ignore")}) waitForWalkerPath(t, ss, wc, tmpDir) diff --git a/internal/settings/settings.go b/internal/settings/settings.go index dec70088d..f3bdfb9ce 100644 --- a/internal/settings/settings.go +++ b/internal/settings/settings.go @@ -15,10 +15,14 @@ type ExperimentalFeatures struct { PrefillRequiredFields bool `mapstructure:"prefillRequiredFields"` } +type Indexing struct { + IgnoreDirectoryNames []string `mapstructure:"ignoreDirectoryNames"` + IgnorePaths []string `mapstructure:"ignorePaths"` +} + type Options struct { - CommandPrefix string `mapstructure:"commandPrefix"` - IgnoreDirectoryNames []string `mapstructure:"indexing.ignoreDirectoryNames"` - IgnorePaths []string `mapstructure:"indexing.ignorePaths"` + CommandPrefix string `mapstructure:"commandPrefix"` + Indexing Indexing `mapstructure:"indexing"` // ExperimentalFeatures encapsulates experimental features users can opt into. ExperimentalFeatures ExperimentalFeatures `mapstructure:"experimentalFeatures"` @@ -48,8 +52,8 @@ func (o *Options) Validate() error { } } - if len(o.IgnoreDirectoryNames) > 0 { - for _, directory := range o.IgnoreDirectoryNames { + if len(o.Indexing.IgnoreDirectoryNames) > 0 { + for _, directory := range o.Indexing.IgnoreDirectoryNames { if directory == datadir.DataDirName { return fmt.Errorf("cannot ignore directory %q", datadir.DataDirName) } diff --git a/internal/settings/settings_test.go b/internal/settings/settings_test.go index cbca1730c..dd727b055 100644 --- a/internal/settings/settings_test.go +++ b/internal/settings/settings_test.go @@ -16,14 +16,16 @@ func TestDecodeOptions_nil(t *testing.T) { } opts := out.Options - if opts.IgnoreDirectoryNames != nil { - t.Fatalf("expected no options for nil, %#v given", opts.IgnoreDirectoryNames) + if opts.Indexing.IgnoreDirectoryNames != nil { + t.Fatalf("expected no options for nil, %#v given", opts.Indexing.IgnoreDirectoryNames) } } func TestDecodeOptions_wrongType(t *testing.T) { _, err := DecodeOptions(map[string]interface{}{ - "indexing.ignorePaths": "/random/path", + "indexing": map[string]interface{}{ + "ignorePaths": "/random/path", + }, }) if err == nil { t.Fatal("expected decoding of wrong type to result in error") @@ -32,14 +34,16 @@ func TestDecodeOptions_wrongType(t *testing.T) { func TestDecodeOptions_success(t *testing.T) { out, err := DecodeOptions(map[string]interface{}{ - "indexing.ignorePaths": []string{"/random/path"}, + "indexing": map[string]interface{}{ + "ignorePaths": []string{"/random/path"}, + }, }) if err != nil { t.Fatal(err) } opts := out.Options expectedPaths := []string{"/random/path"} - if diff := cmp.Diff(expectedPaths, opts.IgnorePaths); diff != "" { + if diff := cmp.Diff(expectedPaths, opts.Indexing.IgnorePaths); diff != "" { t.Fatalf("options mismatch: %s", diff) } } @@ -55,7 +59,9 @@ func TestValidate_IgnoreDirectoryNames_error(t *testing.T) { for _, table := range tables { out, err := DecodeOptions(map[string]interface{}{ - "indexing.ignoreDirectoryNames": []string{table.input}, + "indexing": map[string]interface{}{ + "ignoreDirectoryNames": []string{table.input}, + }, }) if err != nil { t.Fatal(err) @@ -69,7 +75,9 @@ func TestValidate_IgnoreDirectoryNames_error(t *testing.T) { } func TestValidate_IgnoreDirectoryNames_success(t *testing.T) { out, err := DecodeOptions(map[string]interface{}{ - "indexing.ignoreDirectoryNames": []string{"directory"}, + "indexing": map[string]interface{}{ + "ignoreDirectoryNames": []string{"directory"}, + }, }) if err != nil { t.Fatal(err)