settings: break down indexing options (#1010)

Author: radeksimko

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`.

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.

internal/langserver/handlers/initialize.go

@@ -174,7 +174,8 @@ func getTelemetryProperties(out *settings.DecodedOptions) map[string]interface{}
 		"options.rootModulePaths":                         false,
 		"options.excludeModulePaths":                      false,
 		"options.commandPrefix":                           false,
-		"options.ignoreDirectoryNames":                    false,
+		"options.indexing.ignoreDirectoryNames":           false,
+		"options.indexing.ignorePaths":                    false,
 		"options.experimentalFeatures.validateOnSave":     false,
 		"options.terraformExecPath":                       false,
 		"options.terraformExecTimeout":                    "",
@@ -186,8 +187,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
@@ -271,7 +272,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{
@@ -314,9 +315,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

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)

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"`
@@ -49,8 +53,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)
 			}

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)