Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add setting for workspace root module configuration #423

Merged
merged 5 commits into from
Jul 14, 2020
Merged

Add setting for workspace root module configuration #423

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a781b00
Add setting for workspace root modules
aeschright Jun 26, 2020
fd77f37
Update package.json
aeschright Jun 29, 2020
dcceca6
Update package.json
aeschright Jul 2, 2020
d2f4438
Update namespace for language server config
aeschright Jul 8, 2020
a3e478a
Document root module setting
aeschright Jul 13, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 17 additions & 9 deletions out/extension.js
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion out/extension.js.map
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 9 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,15 @@
"maxNumberOfProblems": 100,
"trace.server": "off"
}
},
"terraform.rootModules": {
radeksimko marked this conversation as resolved.
Show resolved Hide resolved
"scope": "resource",
"type": "array",
aeschright marked this conversation as resolved.
Show resolved Hide resolved
"items": {
"type": "string"
},
"default": []
aeschright marked this conversation as resolved.
Show resolved Hide resolved
"description": "Per-workspace list of module directories for the language server to read"
paultyng marked this conversation as resolved.
Show resolved Hide resolved
}
}
},
Expand Down
26 changes: 17 additions & 9 deletions src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,16 @@ export function activate(context: vscode.ExtensionContext) {
context.subscriptions.push(
vscode.workspace.onDidChangeConfiguration(
(event: vscode.ConfigurationChangeEvent) => {
if (!event.affectsConfiguration('terraform.languageServer')) {
if (event.affectsConfiguration("terraform")) {
const reloadMsg = "Reload VSCode window to apply language server changes";
vscode.window.showInformationMessage(reloadMsg, "Reload").then((selected) => {
if (selected === "Reload") {
vscode.commands.executeCommand("workbench.action.reloadWindow");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this need to restart, why can't we just use the configuration RPC hook?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just read the LS issue, this is fine if its just the initial implementation and we can improve it over time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@radeksimko and I talked through it this morning, this is the simplest option to start with. We'll have to stagger the language server and extension releases to deploy this, so it makes the most sense to do something we know we can ship on the language server quickly.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reload will still be necessary with the latest proposed solution (which involves initializationOptions), there's some context here to explain why:
hashicorp/terraform-ls#189 (comment)

}
});
} else {
return;
}
const reloadMsg = 'Reload VSCode window to apply language server changes';
vscode.window.showInformationMessage(reloadMsg, 'Reload').then((selected) => {
if (selected === 'Reload') {
vscode.commands.executeCommand('workbench.action.reloadWindow');
}
});
}
)
);
Expand Down Expand Up @@ -104,13 +105,20 @@ async function startLsClient(cmd: string, config: vscode.WorkspaceConfiguration)
const binaryName = cmd.split("/").pop();
let serverOptions: ServerOptions;
let serverArgs: string[] = config.get("languageServer.args");
let additionalArgs: string[];

if (config.has("rootModules")) {
const rootModules: string[] = config.get("rootModules");
additionalArgs = rootModules.map(module => `-root-module=${module}`);
}
const args = serverArgs.concat(additionalArgs);

const setup = vscode.window.createOutputChannel(binaryName);
setup.appendLine(`Launching language server: ${cmd} ${serverArgs}`)
setup.appendLine(`Launching language server: ${cmd} ${args.join(" ")}`);

const executable: Executable = {
command: cmd,
args: serverArgs,
args: args,
options: {}
}
serverOptions = {
Expand Down