Skip to content

Commit

Permalink
Restart clangd when changing build configuration
Browse files Browse the repository at this point in the history
Currently, when the user changes build configuration, we send a
DidChangeConfiguration notification to clangd.  This does not work in
the particular case where the user wants to go back to the default of
not using a specific build configuration.  There is no way (as of now)
to tell clangd that we don't want to use a specific
compile_commands.json anymore.

This patch works around it by restarting clangd when the user switches
config.  The new clangd will be initialized with the newly selected
build config.

The only way to test this end to end (with clangd involved) was to add a
UI test.  The test is skipped if clangd is not available, since we don't
expect everybody to have it installed.  If/when the cpp extension is
moved to its own repo, then we can make the test unconditional.

Change-Id: Id4e42ccd3b69a147a4c099e73440b8df00981e67
Signed-off-by: Simon Marchi <[email protected]>
  • Loading branch information
Simon Marchi authored and marcdumais-work committed Oct 31, 2018
1 parent 740927e commit 7d4fcad
Show file tree
Hide file tree
Showing 3 changed files with 200 additions and 16 deletions.
159 changes: 159 additions & 0 deletions examples/browser/test/cpp/cpp-change-build-config.ui-spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
/********************************************************************************
* Copyright (C) 2018 Ericsson and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import * as utils from '../utils';
import * as fs from 'fs';
import * as path from 'path';
import * as cp from 'child_process';
import { MainPage } from '../main-page/main-page';
import { TopPanel } from '../top-panel/top-panel';
import { LeftPanel } from '../left-panel/left-panel';
import { BottomPanel } from '../bottom-panel/bottom-panel';

let mainPage: MainPage;
let topPanel: TopPanel;
let leftPanel: LeftPanel;
let bottomPanel: BottomPanel;

before(() => {
const driver = browser;

driver.url('/');
driver.localStorage('DELETE');
driver.refresh();

mainPage = new MainPage(driver);
topPanel = new TopPanel(driver);
leftPanel = new LeftPanel(driver);
bottomPanel = new BottomPanel(driver);

mainPage.waitForStartup();
});

/**
* Prepare some test files in the workspace.
*/
function prepareWorkspace(): string {
// Put our stuff in a cpp/ subdirectory, because we share the workspace with other tests...
const rootDir = path.join(utils.getWorkspaceRoot());
const cppRootDir = path.join(rootDir, 'cpp');

fs.mkdirSync(cppRootDir);
fs.writeFileSync(path.join(cppRootDir, 'src.cpp'), `
#if MACRO == 0
#warning "MACRO IS ZERO"
#elif MACRO == 1
#warning "MACRO IS ONE"
#elif MACRO == 2
#warning "MACRO IS TWO"
#endif
int main() {}
`);

for (const buildnum of [1, 2]) {
const buildDir = path.join(cppRootDir, `build${buildnum}`);
fs.mkdirSync(buildDir);
fs.writeFileSync(path.join(buildDir, 'compile_commands.json'), `
[{
"file": "../src.cpp",
"directory": "${buildDir}",
"arguments": ["c++", "-c", "-DMACRO=${buildnum}", "../src.cpp"]
]]
`);
}

// Write the list of build configs in the workspace preferences. Hopefully,
// no other test needs to write preferences...
const dotTheiaDir = path.join(rootDir, '.theia');
fs.mkdirSync(dotTheiaDir);
fs.writeFileSync(path.join(dotTheiaDir, 'settings.json'), `
{
"cpp.buildConfigurations": [{
"name": "Build one",
"directory": "${path.join(cppRootDir, 'build1')}"
}, {
"name": "Build two",
"directory": "${path.join(cppRootDir, 'build2')}"
}]
}
`);

return rootDir;
}

/**
* Return whether clangd is available.
*/
function hasClangd() {
try {
const out = cp.execSync('clangd -version', {encoding: 'utf8'});
// Match 'clangd version' at the start of
// 'clangd version 8.0.0 (trunk 341484) (llvm/trunk 341481)'.
return out.indexOf('clangd version') === 0;
} catch (e) {
return false;
}
}

/**
* Open the build config quick open menu, click on the first config that
* matches `name`.
*/
function changeBuildConfig(name: string, driver: WebdriverIO.Client<void>) {
const statusBar = driver.element('#theia-statusBar');
const statusBarButton = statusBar.element('div.element*=Build Config');
statusBarButton.click();
driver.pause(300);

const entry = driver.element('div.monaco-icon-label*=' + name);
entry.click();
driver.pause(300);
}

describe('cpp extension', function() {
it('should be able to change build config', function() {
if (!hasClangd()) {
this.skip();
return;
}

prepareWorkspace();

// Open Files and Problems views
topPanel.toggleFilesView();
topPanel.openProblemsView();
bottomPanel.waitForProblemsView();

// Open our test source file
leftPanel.toggleDirectoryInFilesView('cpp');
leftPanel.openFileInFilesView('src.cpp');

// Confirm the expected diagnostic is there, change build
// configuration, check that the new diagnostic is there, and so on.
const problemsView = browser.element('#problems');
problemsView.waitForExist('div.message*=MACRO IS ZERO');

changeBuildConfig('Build one', browser);
problemsView.waitForExist('div.message*=MACRO IS ONE');

changeBuildConfig('Build two', browser);
problemsView.waitForExist('div.message*=MACRO IS TWO');

changeBuildConfig('None', browser);
problemsView.waitForExist('div.message*=MACRO IS ZERO');
});
});
26 changes: 26 additions & 0 deletions examples/browser/test/left-panel/left-panel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,32 @@ export class LeftPanel {
this.driver.pause(300);
}

/**
* Click on the first node named `name` in the files view to expand or
* collapse it. No check is done to make sure this node actually exists or
* represents a directory.
*/
toggleDirectoryInFilesView(name: string) {
this.waitForFilesViewVisible();
const files = this.driver.element('#files');
const element = files.element('div=' + name);
element.click();
this.driver.pause(300);
}

/**
* Double click on the first node named `name` in the files view to open
* it. Not check is done to make sure this node actually exists or
* represents a file.
*/
openFileInFilesView(name: string) {
this.waitForFilesViewVisible();
const files = this.driver.element('#files');
const element = files.element('div=' + name);
element.doubleClick();
this.driver.pause(300);
}

isGitContainerVisible(): boolean {
return (this.driver.isExisting('#theia-gitContainer') && this.driver.element('#theia-gitContainer').getAttribute('class').split(' ').indexOf('p-mod-hidden') === -1
&& this.isPanelVisible());
Expand Down
31 changes: 15 additions & 16 deletions packages/cpp/src/browser/cpp-language-client-contribution.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,13 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { inject, injectable } from 'inversify';
import { inject, injectable, postConstruct } from 'inversify';
import {
BaseLanguageClientContribution, LanguageClientFactory,
LanguageClientOptions,
ILanguageClient
} from '@theia/languages/lib/browser';
import { Languages, Workspace, DidChangeConfigurationParams, DidChangeConfigurationNotification } from '@theia/languages/lib/browser';
import { Languages, Workspace } from '@theia/languages/lib/browser';
import { ILogger } from '@theia/core/lib/common/logger';
import { MessageService } from '@theia/core/lib/common/message-service';
import { CPP_LANGUAGE_ID, CPP_LANGUAGE_NAME, HEADER_AND_SOURCE_FILE_EXTENSIONS } from '../common';
Expand Down Expand Up @@ -58,34 +58,33 @@ export class CppLanguageClientContribution extends BaseLanguageClientContributio
super(workspace, languages, languageClientFactory);
}

@postConstruct()
protected init() {
this.cppBuildConfigurations.onActiveConfigChange(config => this.onActiveBuildConfigChanged(config));
}

protected onReady(languageClient: ILanguageClient): void {
super.onReady(languageClient);

this.cppBuildConfigurations.onActiveConfigChange(config => this.onActiveBuildConfigChanged(config));

// Display the C/C++ build configurations status bar element to select active build config
this.cppBuildConfigurationsStatusBarElement.show();
}

private createClangdConfigurationParams(config: CppBuildConfiguration | undefined, isInitialize: boolean): ClangdConfigurationParamsChange {
private createClangdConfigurationParams(config: CppBuildConfiguration | undefined): ClangdConfigurationParamsChange {
const clangdParams: ClangdConfigurationParamsChange = {};

// During initialization, we don't need to send the compile commands
// path if there isn't one specified (it's clangd's default).
if (!isInitialize || config) {
clangdParams.compilationDatabasePath = config ? config.directory : '';
if (config) {
clangdParams.compilationDatabasePath = config.directory;
}

return clangdParams;
}

async onActiveBuildConfigChanged(config: CppBuildConfiguration | undefined) {
const interfaceParams: DidChangeConfigurationParams = {
settings: this.createClangdConfigurationParams(config, false)
};

const languageClient = await this.languageClient;
languageClient.sendNotification(DidChangeConfigurationNotification.type, interfaceParams);
// Restart clangd. The new config will be picked up when
// createOptions will be called to send the initialize request
// to the new instance of clangd.
this.restart();
}

protected get documentSelector() {
Expand All @@ -109,7 +108,7 @@ export class CppLanguageClientContribution extends BaseLanguageClientContributio

protected createOptions(): LanguageClientOptions {
const clientOptions = super.createOptions();
clientOptions.initializationOptions = this.createClangdConfigurationParams(this.cppBuildConfigurations.getActiveConfig(), true);
clientOptions.initializationOptions = this.createClangdConfigurationParams(this.cppBuildConfigurations.getActiveConfig());

clientOptions.initializationFailedHandler = () => {
const READ_INSTRUCTIONS_ACTION = 'Read Instructions';
Expand Down

0 comments on commit 7d4fcad

Please sign in to comment.