From 37e78550a80577f8f8de396bc440a1a0a5bf0fdf Mon Sep 17 00:00:00 2001 From: Giulio Girardi Date: Wed, 16 Dec 2020 14:04:54 +0100 Subject: [PATCH 1/4] Add more specific configurations --- package.json | 52 ++++++++++++++++++++++++++++++-- src/arguments.ts | 69 +++++++++++++++++++++++++++++++++++++++++++ src/clangd-context.ts | 3 +- 3 files changed, 120 insertions(+), 4 deletions(-) create mode 100644 src/arguments.ts diff --git a/package.json b/package.json index fab06ed3..4d2da6b7 100644 --- a/package.json +++ b/package.json @@ -86,13 +86,13 @@ ], "configuration": { "type": "object", - "title": "clangd", + "title": "Clangd", "properties": { "clangd.path": { "type": "string", "default": "clangd", "scope": "machine-overridable", - "description": "The path to clangd executable, e.g.: /usr/bin/clangd." + "markdownDescription": "The path to clangd executable, e.g.: `/usr/bin/clangd`." }, "clangd.arguments": { "type": "array", @@ -100,7 +100,7 @@ "items": { "type": "string" }, - "description": "Arguments for clangd server." + "description": "Arguments for clangd server. For backwards compatibility it has precedence over other UI specified options." }, "clangd.trace": { "type": "string", @@ -129,6 +129,52 @@ "default": false, "description": "Check for language server updates on startup." }, + "clangd.compileCommandsDir": { + "type": "string", + "default": "", + "markdownDescription": "Specify a path to look for `compile_commands.json`. If path is invalid, clangd will look in the current directory and parent paths of each source file. If not specified clangd will look in the parent folders of each opened file." + }, + "clangd.queryDrivers": { + "type": "array", + "default": [], + "items": { + "type": "string" + }, + "markdownDescription": "Globs for white-listing gcc-compatible drivers that are safe to execute. Drivers matching any of these globs will be used to extract system includes. For example `/usr/bin/**/clang-*` or `/path/to/repo/**/g++-*`" + }, + "clangd.backgroundIndex": { + "type": "boolean", + "default": true, + "description": "Index project code in the background and persist index on disk." + }, + "clangd.clangTidy": { + "type": "boolean", + "default": false, + "description": "Enable clang-tidy diagnostics." + }, + "clangd.crossFileRename": { + "type": "boolean", + "default": false, + "description": "Enable cross-file rename feature." + }, + "clangd.headerInsertion": { + "type":"string", + "enum": [ + "iwyu", + "never" + ], + "default": "iwyu", + "markdownEnumDescriptions": [ + "Include what you use. Insert the owning header for top-level symbols, unless the header is already directly included or the symbol is forward-declared", + "Never insert `#include` directives as part of code completion" + ], + "markdownDescription": "Add `#include` directives when accepting code completions." + }, + "clangd.limitResults": { + "type": "integer", + "default": 100, + "description": "Limit the number of results returned by clangd. 0 means no limit." + }, "clangd.onConfigChanged": { "type": "string", "default": "prompt", diff --git a/src/arguments.ts b/src/arguments.ts new file mode 100644 index 00000000..193b7e43 --- /dev/null +++ b/src/arguments.ts @@ -0,0 +1,69 @@ +import * as config from './config'; + +function argsContainFlag(args: string[], flag: string) { + for (let arg of args) { + if (arg.startsWith(flag)) + return true; + } + + return false; +} + +export function getClangdArguments() { + let args = config.get('arguments'); + + let compileCommandsDirFlag = '--compile-commands-dir'; + + if (!argsContainFlag(args, compileCommandsDirFlag)) { + let compileCommandsDir = config.get('compileCommandsDir'); + if (compileCommandsDir.length > 0) + args.push(`${compileCommandsDirFlag}=${compileCommandsDir}`); + } + + let queryDriversFlag = '--query-driver'; + + if (!argsContainFlag(args, queryDriversFlag)) { + let queryDrivers = config.get('queryDrivers'); + if (queryDrivers.length > 0) { + let drivers = queryDrivers.join(','); + args.push(`${queryDriversFlag}=${drivers}`); + } + } + + let backgroundIndexFlag = '--background-index'; + + if (!argsContainFlag(args, backgroundIndexFlag)) { + let backgroundIndex = config.get('backgroundIndex'); + args.push(`${backgroundIndexFlag}=${backgroundIndex}`); + } + + let clangTidyFlag = '--clang-tidy'; + + if (!argsContainFlag(args, clangTidyFlag)) { + let clangTidy = config.get('clangTidy'); + args.push(`${clangTidyFlag}=${clangTidy}`) + } + + let crossFileRenameFlag = '--cross-file-rename'; + + if (!argsContainFlag(args, crossFileRenameFlag)) { + let crossFileRename = config.get('crossFileRename'); + args.push(`${crossFileRenameFlag}=${crossFileRename}`); + } + + let headerInsertionFlag = '--header-insertion'; + + if (!argsContainFlag(args, headerInsertionFlag)) { + let headerInsertion = config.get('headerInsertion'); + args.push(`${headerInsertionFlag}=${headerInsertion}`); + } + + let limitResultsFlag = '--limit-results'; + + if (!argsContainFlag(args, limitResultsFlag)) { + let limitResults = config.get('limitResults'); + args.push(`${limitResultsFlag}=${limitResults}`); + } + + return args; +} \ No newline at end of file diff --git a/src/clangd-context.ts b/src/clangd-context.ts index dc8a304a..b6b35386 100644 --- a/src/clangd-context.ts +++ b/src/clangd-context.ts @@ -1,6 +1,7 @@ import * as vscode from 'vscode'; import * as vscodelc from 'vscode-languageclient/node'; +import * as args from './arguments'; import * as config from './config'; import * as configFileWatcher from './config-file-watcher'; import * as fileStatus from './file-status'; @@ -50,7 +51,7 @@ export class ClangdContext implements vscode.Disposable { const clangd: vscodelc.Executable = { command: clangdPath, - args: config.get('arguments') + args: args.getClangdArguments() }; const traceFile = config.get('trace'); if (!!traceFile) { From 4f901805d46d78600784ae2607e9c60200491e9a Mon Sep 17 00:00:00 2001 From: Giulio Girardi Date: Fri, 18 Dec 2020 11:26:09 +0100 Subject: [PATCH 2/4] Add version check --- package-lock.json | 6 ++-- package.json | 18 +++++------ src/arguments.ts | 75 ++++++++++++++++++++++++++++++------------- src/clangd-context.ts | 2 +- 4 files changed, 65 insertions(+), 36 deletions(-) diff --git a/package-lock.json b/package-lock.json index da6ca427..adcc113c 100644 --- a/package-lock.json +++ b/package-lock.json @@ -79,9 +79,9 @@ "dev": true }, "@types/node": { - "version": "6.14.9", - "resolved": "https://registry.npmjs.org/@types/node/-/node-6.14.9.tgz", - "integrity": "sha512-leP/gxHunuazPdZaCvsCefPQxinqUDsCxCR5xaDUrY2MkYxQRFZZwU5e7GojyYsGB7QVtCi7iVEl/hoFXQYc+w==", + "version": "8.10.66", + "resolved": "https://registry.npmjs.org/@types/node/-/node-8.10.66.tgz", + "integrity": "sha512-tktOkFUA4kXx2hhhrB8bIFb5TbwzS4uOhKEmwiD+NoiL0qtP2OQ9mFldbgD4dV1djrlBYP6eBuQZiWjuHUpqFw==", "dev": true }, "@types/vscode": { diff --git a/package.json b/package.json index 4d2da6b7..2929261b 100644 --- a/package.json +++ b/package.json @@ -52,7 +52,7 @@ "devDependencies": { "@types/glob": "^7.1.1", "@types/mocha": "^7.0.2", - "@types/node": "^6.0.40", + "@types/node": "^8.10.66", "@types/vscode": "1.46.*", "clang-format": "1.4.0", "glob": "^7.1.4", @@ -132,7 +132,7 @@ "clangd.compileCommandsDir": { "type": "string", "default": "", - "markdownDescription": "Specify a path to look for `compile_commands.json`. If path is invalid, clangd will look in the current directory and parent paths of each source file. If not specified clangd will look in the parent folders of each opened file." + "markdownDescription": "Specify a path to look for `compile_commands.json`. If path is invalid, clangd will look in the current directory and parent paths of each source file. If not specified clangd will look in the parent folders of each opened file. Corresponds to `--compile-commands-dir` flag." }, "clangd.queryDrivers": { "type": "array", @@ -140,25 +140,25 @@ "items": { "type": "string" }, - "markdownDescription": "Globs for white-listing gcc-compatible drivers that are safe to execute. Drivers matching any of these globs will be used to extract system includes. For example `/usr/bin/**/clang-*` or `/path/to/repo/**/g++-*`" + "markdownDescription": "Globs for white-listing gcc-compatible drivers that are safe to execute. Drivers matching any of these globs will be used to extract system includes. For example `/usr/bin/**/clang-*` or `/path/to/repo/**/g++-*`. Corresponds to `--query-driver` flag. Supported since clangd version 9." }, "clangd.backgroundIndex": { "type": "boolean", "default": true, - "description": "Index project code in the background and persist index on disk." + "markdownDescription": "Index project code in the background and persist index on disk. Corresponds to `--background-index` flag. Supported since clangd version 8." }, "clangd.clangTidy": { "type": "boolean", "default": false, - "description": "Enable clang-tidy diagnostics." + "markdownDescription": "Enable clang-tidy diagnostics. Corresponds to `--clang-tidy` flag. Supported since clangd version 9." }, "clangd.crossFileRename": { "type": "boolean", "default": false, - "description": "Enable cross-file rename feature." + "markdownDescription": "Enable cross-file rename feature. Corresponds to `--cross-file-rename` flag. Supported since clangd version 10." }, "clangd.headerInsertion": { - "type":"string", + "type": "string", "enum": [ "iwyu", "never" @@ -168,12 +168,12 @@ "Include what you use. Insert the owning header for top-level symbols, unless the header is already directly included or the symbol is forward-declared", "Never insert `#include` directives as part of code completion" ], - "markdownDescription": "Add `#include` directives when accepting code completions." + "markdownDescription": "Add `#include` directives when accepting code completions. Corresponds to `--header-insertion` flag. Supported since clangd version 9." }, "clangd.limitResults": { "type": "integer", "default": 100, - "description": "Limit the number of results returned by clangd. 0 means no limit." + "markdownDescription": "Limit the number of results returned by clangd. 0 means no limit. Corresponds to `--limit-results` flag." }, "clangd.onConfigChanged": { "type": "string", diff --git a/src/arguments.ts b/src/arguments.ts index 193b7e43..4fc5b60f 100644 --- a/src/arguments.ts +++ b/src/arguments.ts @@ -1,5 +1,10 @@ +import * as cp from 'child_process'; +import * as util from 'util'; + import * as config from './config'; +const exec = util.promisify(cp.exec); + function argsContainFlag(args: string[], flag: string) { for (let arg of args) { if (arg.startsWith(flag)) @@ -9,8 +14,22 @@ function argsContainFlag(args: string[], flag: string) { return false; } -export function getClangdArguments() { +async function getClangdMajorVersion(clangdPath: string) { + const output = await exec(`${clangdPath} --version`); + const regexv = /clangd version ([0-9]+)\..*/; + const match = output.stdout.match(regexv); + + if (output.stderr || !match) { + throw new Error('Could not determine clangd version') + } + + return Number.parseInt(match[1]); +} + +export async function getClangdArguments(clangdPath: string) { let args = config.get('arguments'); + // Versions before clangd 7 are not checked + let version = await getClangdMajorVersion(clangdPath); let compileCommandsDirFlag = '--compile-commands-dir'; @@ -20,42 +39,52 @@ export function getClangdArguments() { args.push(`${compileCommandsDirFlag}=${compileCommandsDir}`); } - let queryDriversFlag = '--query-driver'; + if (version >= 9) { + let queryDriversFlag = '--query-driver'; - if (!argsContainFlag(args, queryDriversFlag)) { - let queryDrivers = config.get('queryDrivers'); - if (queryDrivers.length > 0) { - let drivers = queryDrivers.join(','); - args.push(`${queryDriversFlag}=${drivers}`); + if (!argsContainFlag(args, queryDriversFlag)) { + let queryDrivers = config.get('queryDrivers'); + if (queryDrivers.length > 0) { + let drivers = queryDrivers.join(','); + args.push(`${queryDriversFlag}=${drivers}`); + } } } - let backgroundIndexFlag = '--background-index'; + if (version >= 8) { + let backgroundIndexFlag = '--background-index'; - if (!argsContainFlag(args, backgroundIndexFlag)) { - let backgroundIndex = config.get('backgroundIndex'); - args.push(`${backgroundIndexFlag}=${backgroundIndex}`); + if (!argsContainFlag(args, backgroundIndexFlag)) { + let backgroundIndex = config.get('backgroundIndex'); + args.push(`${backgroundIndexFlag}=${backgroundIndex}`); + } } - let clangTidyFlag = '--clang-tidy'; + if (version >= 9) { + let clangTidyFlag = '--clang-tidy'; - if (!argsContainFlag(args, clangTidyFlag)) { - let clangTidy = config.get('clangTidy'); - args.push(`${clangTidyFlag}=${clangTidy}`) + if (!argsContainFlag(args, clangTidyFlag)) { + let clangTidy = config.get('clangTidy'); + args.push(`${clangTidyFlag}=${clangTidy}`) + } } - let crossFileRenameFlag = '--cross-file-rename'; + if (version >= 10) { + let crossFileRenameFlag = '--cross-file-rename'; - if (!argsContainFlag(args, crossFileRenameFlag)) { - let crossFileRename = config.get('crossFileRename'); - args.push(`${crossFileRenameFlag}=${crossFileRename}`); + if (!argsContainFlag(args, crossFileRenameFlag)) { + let crossFileRename = config.get('crossFileRename'); + args.push(`${crossFileRenameFlag}=${crossFileRename}`); + } } - let headerInsertionFlag = '--header-insertion'; + if (version >= 9) { + let headerInsertionFlag = '--header-insertion'; - if (!argsContainFlag(args, headerInsertionFlag)) { - let headerInsertion = config.get('headerInsertion'); - args.push(`${headerInsertionFlag}=${headerInsertion}`); + if (!argsContainFlag(args, headerInsertionFlag)) { + let headerInsertion = config.get('headerInsertion'); + args.push(`${headerInsertionFlag}=${headerInsertion}`); + } } let limitResultsFlag = '--limit-results'; diff --git a/src/clangd-context.ts b/src/clangd-context.ts index b6b35386..0f068d61 100644 --- a/src/clangd-context.ts +++ b/src/clangd-context.ts @@ -51,7 +51,7 @@ export class ClangdContext implements vscode.Disposable { const clangd: vscodelc.Executable = { command: clangdPath, - args: args.getClangdArguments() + args: await args.getClangdArguments(clangdPath) }; const traceFile = config.get('trace'); if (!!traceFile) { From e775de961307f7af4f17882de2c11fc4e5642759 Mon Sep 17 00:00:00 2001 From: Giulio Girardi Date: Fri, 18 Dec 2020 12:20:42 +0100 Subject: [PATCH 3/4] Add prompt to update settings in case of conflict --- src/arguments.ts | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/src/arguments.ts b/src/arguments.ts index 4fc5b60f..8be95f87 100644 --- a/src/arguments.ts +++ b/src/arguments.ts @@ -1,5 +1,6 @@ import * as cp from 'child_process'; import * as util from 'util'; +import * as vscode from 'vscode'; import * as config from './config'; @@ -30,6 +31,7 @@ export async function getClangdArguments(clangdPath: string) { let args = config.get('arguments'); // Versions before clangd 7 are not checked let version = await getClangdMajorVersion(clangdPath); + let overridenOptionWarning = false; let compileCommandsDirFlag = '--compile-commands-dir'; @@ -37,6 +39,8 @@ export async function getClangdArguments(clangdPath: string) { let compileCommandsDir = config.get('compileCommandsDir'); if (compileCommandsDir.length > 0) args.push(`${compileCommandsDirFlag}=${compileCommandsDir}`); + } else { + overridenOptionWarning = true; } if (version >= 9) { @@ -48,6 +52,8 @@ export async function getClangdArguments(clangdPath: string) { let drivers = queryDrivers.join(','); args.push(`${queryDriversFlag}=${drivers}`); } + } else { + overridenOptionWarning = true; } } @@ -57,6 +63,8 @@ export async function getClangdArguments(clangdPath: string) { if (!argsContainFlag(args, backgroundIndexFlag)) { let backgroundIndex = config.get('backgroundIndex'); args.push(`${backgroundIndexFlag}=${backgroundIndex}`); + } else { + overridenOptionWarning = true; } } @@ -66,6 +74,8 @@ export async function getClangdArguments(clangdPath: string) { if (!argsContainFlag(args, clangTidyFlag)) { let clangTidy = config.get('clangTidy'); args.push(`${clangTidyFlag}=${clangTidy}`) + } else { + overridenOptionWarning = true; } } @@ -75,6 +85,8 @@ export async function getClangdArguments(clangdPath: string) { if (!argsContainFlag(args, crossFileRenameFlag)) { let crossFileRename = config.get('crossFileRename'); args.push(`${crossFileRenameFlag}=${crossFileRename}`); + } else { + overridenOptionWarning = true; } } @@ -84,6 +96,8 @@ export async function getClangdArguments(clangdPath: string) { if (!argsContainFlag(args, headerInsertionFlag)) { let headerInsertion = config.get('headerInsertion'); args.push(`${headerInsertionFlag}=${headerInsertion}`); + } else { + overridenOptionWarning = true; } } @@ -92,6 +106,20 @@ export async function getClangdArguments(clangdPath: string) { if (!argsContainFlag(args, limitResultsFlag)) { let limitResults = config.get('limitResults'); args.push(`${limitResultsFlag}=${limitResults}`); + } else { + overridenOptionWarning = true; + } + + if (overridenOptionWarning) { + let action = await vscode.window.showWarningMessage( + 'Setting "clangd.arguments" overrides one or more options that can now be set with more specific settings. This does not cause any error, but updating your configuration is advised.', + 'Open settings'); + + if (action == 'Open settings') { + vscode.commands.executeCommand( + 'workbench.action.openSettings', + '@ext:llvm-vs-code-extensions.vscode-clangd') + } } return args; From 0b12ef0f15d335f4a4fa220d7acc322a52320a34 Mon Sep 17 00:00:00 2001 From: Giulio Girardi Date: Fri, 18 Dec 2020 22:08:23 +0100 Subject: [PATCH 4/4] Fix default values for the new settings --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 2929261b..f6dcfa91 100644 --- a/package.json +++ b/package.json @@ -149,12 +149,12 @@ }, "clangd.clangTidy": { "type": "boolean", - "default": false, + "default": true, "markdownDescription": "Enable clang-tidy diagnostics. Corresponds to `--clang-tidy` flag. Supported since clangd version 9." }, "clangd.crossFileRename": { "type": "boolean", - "default": false, + "default": true, "markdownDescription": "Enable cross-file rename feature. Corresponds to `--cross-file-rename` flag. Supported since clangd version 10." }, "clangd.headerInsertion": {