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.

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

trigger workspace/didChangeConfiguration on compilation database modification #76

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

BenjaminNavarro
Copy link

I was annoyed to always have to call Restart language serveror Restart language server in lazy mode manually when my compilation database was modified so here is my attempt to fix this.

I simply use workspace.createFileSystemWatcher to monitor changes to the compilation_database.json file and when a change occur, I send the workspace/didChangeConfiguration notification to the server.

It seems to be working for me but since it's the first time I work on a VSCode extension I might have missed a couple things.

@BenjaminNavarro
Copy link
Author

Done. Could you please explain me the rationale between using single or double quotes? Both seem to be used in the code so I'd like to know when each one has to be used in the case I have other modification to make

@BenjaminNavarro
Copy link
Author

When can this be merged?

@MaskRay
Copy link
Owner

MaskRay commented Oct 28, 2019

If the initialization option compilationDatabaseDirectory is not empty, it specifies the directory that contains compile_commands.json. Can you update the patch to recognize compilationDatabaseDirectory?

// ccls/src/project.cc
    cdbDir = root;
    if (g_config->compilationDatabaseDirectory.size()) {
      if (sys::path::is_absolute(g_config->compilationDatabaseDirectory))
        cdbDir = g_config->compilationDatabaseDirectory;
      else
        sys::path::append(cdbDir, g_config->compilationDatabaseDirectory);
    }
    sys::path::append(path, cdbDir, "compile_commands.json");

@BenjaminNavarro
Copy link
Author

That should do it


if (config.get('index.reloadDatabaseOnChange', true)) {
let db_dir = config.get('misc.compilationDatabaseDirectory');
if (!db_dir || db_dir === '') {
Copy link
Owner

Choose a reason for hiding this comment

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

Delete the surrounding {}.

Copy link
Author

Choose a reason for hiding this comment

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

Ok of you prefer it that way

if (!db_dir || db_dir === '') {
db_dir = this.cwd;
}
const db_watcher = workspace.createFileSystemWatcher(db_dir + "/compile_commands.json", false, false, false);
Copy link
Owner

Choose a reason for hiding this comment

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

" -> '

I think single quotes are generally preferred, but for some historical reasons double quotes are still used in these files...

Copy link
Author

Choose a reason for hiding this comment

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

Wouldn't be better to change all quotes in the project once and for all and stick to that?

@BenjaminNavarro
Copy link
Author

Is it ok now?

MaskRay and others added 18 commits May 9, 2020 17:02
This allows to disable CodeLens extension-wise. Some users may want to see CodeLens provided by other extensions, thus they do not want to disable CodeLens globally.
Populate ClientConfig with all keys from VS Code WorkspaceConfiguration
(except those in blacklist) instead of a preset list.

With this change, it is possible to send initializationOptions to ccls
even if there is no corresponding configuration declaration in
package.json. So new initializationOption exposed in ccls no longer
necessarily reuqires an update of vscode-ccls.

Declarations in package.json are still useful as they still provide
documentations and default values.

Also no longer add workspaceSymbol.sort = false by default as this works
poorly when there are too many symbols.
  const defaults = {
    cwd: undefined,
    env: process.env
  };
now it shows names of fields as well as their types
+ byte offset of particular field in the tooltip
Adopt the emacs-ccls approach.

ccls.highlight.{function,type,type}.face define faces of basic symbol
kinds from which other symbol kinds
({member,staticMember}{Function,Variable}, etc) can inherit.

For example, if

    ccls.highlight.function.face: ["enabled"]

is specified, memberFunction will also be enabled because of the default
configurations:

    // Inherit from both ccls.highlight.{function,member}.face
    ccls.highlight.memberFunction.face: ["function", "member"]
    ccls.highlight.member.face: ["fontStyle: italic"]

For a symbol kind, its face property takes effect if "enabled" is
specified.
Watching a symlink instead of the target file won't actually trigger a
db reloading if the symlinked file changes
@Trass3r
Copy link

Trass3r commented Jun 26, 2020

Looks like this somehow picked up commits from master?

@GitMensch
Copy link

Should this be rebased to be merged?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants