Skip to content

Commit

Permalink
Support multiple root cpp build configurations
Browse files Browse the repository at this point in the history 
Added support for multiple root cpp build configurations.
Instead of maintaining a single active configuration, a map is
used to maintain the relationship between the workspace root and its
given `CppBuildConfiguration`. This feature is an experimental
PR based on new developments in clangd to support multiple
compilation databases.

- Update the `cpp` manager to handle the new data structure.
- Update the test cases (fix the task test case with incorrect imports and false positive results).

Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
  • Loading branch information
@vince-fugnitto
vince-fugnitto committed Mar 26, 2019
1 parent 6d605f8 commit e779e8c
Show file tree
Hide file tree
Showing 7 changed files with 178 additions and 63 deletions.
1 change: 1 addition & 0 deletions packages/cpp/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
"@theia/preferences": "^0.4.0",
"@theia/process": "^0.4.0",
"@theia/task": "^0.4.0",
"@theia/workspace": "^0.4.0",
"string-argv": "^0.1.1"
},
"publishConfig": {
Expand Down
19 changes: 15 additions & 4 deletions packages/cpp/src/browser/cpp-build-configurations-statusbar-element.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ export class CppBuildConfigurationsStatusBarElement {
* and listen to changes to the active build configuration.
*/
show(): void {
this.setCppBuildConfigElement(this.cppManager.getActiveConfig());
this.cppManager.onActiveConfigChange(config => this.setCppBuildConfigElement(config));
this.setCppBuildConfigElement(this.getValidActiveCount());
this.cppManager.onActiveConfigChange(() => this.setCppBuildConfigElement(this.getValidActiveCount()));
}

/**
Expand All @@ -45,14 +45,25 @@ export class CppBuildConfigurationsStatusBarElement {
*
* @param config the active `CppBuildConfiguration`.
*/
protected setCppBuildConfigElement(config: CppBuildConfiguration | undefined): void {
protected setCppBuildConfigElement(count: number): void {
this.statusBar.setElement(this.cppIdentifier, {
text: `$(wrench) C/C++ ${config ? '(' + config.name + ')' : 'Build Config'}`,
text: `$(wrench) C/C++ Build Config (Active ${count})`,
tooltip: 'C/C++ Build Config',
alignment: StatusBarAlignment.RIGHT,
command: CPP_CHANGE_BUILD_CONFIGURATION.id,
priority: 0.5,
});
}

/**
* Get the valid active configuration count.
*/
protected getValidActiveCount(): number {
let items: (CppBuildConfiguration | undefined)[] = [];
if (this.cppManager.getAllActiveConfigs) {
items = [...this.cppManager.getAllActiveConfigs().values()].filter(config => !!config);
}
return items.length;
}

}
61 changes: 55 additions & 6 deletions packages/cpp/src/browser/cpp-build-configurations-ui.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ import URI from '@theia/core/lib/common/uri';
import { PreferenceScope, PreferenceService } from '@theia/preferences/lib/browser';
import { CppBuildConfigurationManager, CppBuildConfiguration, CPP_BUILD_CONFIGURATIONS_PREFERENCE_KEY } from './cpp-build-configurations';
import { EditorManager } from '@theia/editor/lib/browser';
import { CommonCommands } from '@theia/core/lib/browser';
import { CommonCommands, QuickPickService, LabelProvider } from '@theia/core/lib/browser';
import { WorkspaceService } from '@theia/workspace/lib/browser';

@injectable()
export class CppBuildConfigurationChanger implements QuickOpenModel {
Expand All @@ -40,12 +41,26 @@ export class CppBuildConfigurationChanger implements QuickOpenModel {
@inject(FileSystem)
protected readonly fileSystem: FileSystem;

@inject(LabelProvider)
protected readonly labelProvider: LabelProvider;

@inject(QuickPickService)
protected readonly quickPick: QuickPickService;

@inject(QuickOpenService)
protected readonly quickOpenService: QuickOpenService;

@inject(PreferenceService)
protected readonly preferenceService: PreferenceService;

@inject(WorkspaceService)
protected readonly workspaceService: WorkspaceService;

/**
* The selected root to update the config.
*/
protected selectedRoot: string | undefined;

readonly createItem: QuickOpenItem = new QuickOpenItem({
label: 'Create New',
iconClass: 'fa fa-plus',
Expand All @@ -67,15 +82,20 @@ export class CppBuildConfigurationChanger implements QuickOpenModel {
if (mode !== QuickOpenMode.OPEN) {
return false;
}
this.commandService.executeCommand(CPP_RESET_BUILD_CONFIGURATION.id);
this.cppBuildConfigurations.setActiveConfig(undefined, this.selectedRoot);
return true;
},
});

async onType(lookFor: string, acceptor: (items: QuickOpenItem[]) => void): Promise<void> {
const items: QuickOpenItem[] = [];
const active: CppBuildConfiguration | undefined = this.cppBuildConfigurations.getActiveConfig();
const configurations = this.cppBuildConfigurations.getValidConfigs();
let active: CppBuildConfiguration | undefined;
if (this.selectedRoot) {
active = this.cppBuildConfigurations.getActiveConfig();
} else {
active = this.cppBuildConfigurations.getActiveConfig(this.selectedRoot);
}
const configurations = this.cppBuildConfigurations.getValidConfigs(this.selectedRoot);

const homeStat = await this.fileSystem.getCurrentUserHome();
const home = (homeStat) ? new URI(homeStat.uri).withoutScheme().toString() : undefined;
Expand All @@ -88,13 +108,27 @@ export class CppBuildConfigurationChanger implements QuickOpenModel {
return acceptor(items);
}

console.log(
`
active: ${active ? active.name : active}
root: ${this.selectedRoot}
`
);

// Item to de-select any active build config
if (active) {
items.push(this.resetItem);
}

// Add one item per build config
configurations.forEach(config => {
console.log(
`
for=config: ${config.name}
for=active: ${active ? active.name : active}
equal?: ${config === active}
`
);
const uri = new URI(config.directory);
items.push(new QuickOpenItem({
label: config.name,
Expand All @@ -106,7 +140,7 @@ export class CppBuildConfigurationChanger implements QuickOpenModel {
return false;
}

this.cppBuildConfigurations.setActiveConfig(config);
this.cppBuildConfigurations.setActiveConfig(config, this.selectedRoot);
return true;
},
}));
Expand All @@ -115,7 +149,9 @@ export class CppBuildConfigurationChanger implements QuickOpenModel {
acceptor(items);
}

open() {
async open() {
const root = await this.selectWorkspaceRoot();
this.selectedRoot = root;
const configs = this.cppBuildConfigurations.getValidConfigs();
this.quickOpenService.open(this, {
placeholder: (configs.length) ? 'Choose a build configuration...' : 'No build configurations present',
Expand All @@ -132,6 +168,19 @@ export class CppBuildConfigurationChanger implements QuickOpenModel {
await this.preferenceService.set(CPP_BUILD_CONFIGURATIONS_PREFERENCE_KEY, configs, PreferenceScope.Workspace);
}

protected async selectWorkspaceRoot(): Promise<string | undefined> {
const roots = this.workspaceService.tryGetRoots();
return this.quickPick.show(roots.map(
({ uri }) => ({
label: this.labelProvider.getName(new URI(uri).withoutScheme()),
value: uri,
description: this.cppBuildConfigurations.getActiveConfig(uri)
? this.cppBuildConfigurations.getActiveConfig(uri)!.name
: 'undefined'
})
), { placeholder: 'Select workspace root' });
}

}

export const CPP_CATEGORY = 'C/C++';
Expand Down
51 changes: 42 additions & 9 deletions packages/cpp/src/browser/cpp-build-configurations.spec.ts
View file
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/********************************************************************************
* Copyright (C) 2018 Ericsson and others.
* Copyright (C) 2018-2019 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
Expand All @@ -14,6 +14,9 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { enableJSDOM } from '@theia/core/lib/browser/test/jsdom';
let disableJSDOM = enableJSDOM();

import { ContainerModule, Container } from 'inversify';
import { expect } from 'chai';
import { FileSystem } from '@theia/filesystem/lib/common';
Expand All @@ -25,9 +28,19 @@ import { FileSystemNode } from '@theia/filesystem/lib/node/node-filesystem';
import { bindCppPreferences } from './cpp-preferences';
import { PreferenceService } from '@theia/core/lib/browser/preferences/preference-service';
import { MockPreferenceService } from '@theia/core/lib/browser/preferences/test/mock-preference-service';
import { WorkspaceService } from '@theia/workspace/lib/browser/workspace-service';

let container: Container;

disableJSDOM();

before(() => {
disableJSDOM = enableJSDOM();
});
after(() => {
disableJSDOM();
});

beforeEach(function () {
const m = new ContainerModule(bind => {
bind(CppBuildConfigurationManager).to(CppBuildConfigurationManagerImpl).inSingletonScope();
Expand All @@ -38,14 +51,22 @@ beforeEach(function () {
});

container = new Container();
container.bind(WorkspaceService).toConstantValue(sinon.createStubInstance(WorkspaceService));
container.load(m);
});

/**
* Get an instance of the `CppBuildConfigurationManager`.
*/
function getManager(): CppBuildConfigurationManager {
return container.get<CppBuildConfigurationManager>(CppBuildConfigurationManager);
}

/**
* Create the .theia/builds.json file with `buildsJsonContent` as its content
* and create/return an instance of the build configuration service. If
* `buildsJsonContent` is undefined, don't create .theia/builds.json.
* If `activeBuildConfigName` is not undefined, also create an entrty in the
* If `activeBuildConfigName` is not undefined, also create an entry in the
* storage service representing the saved active build config.
*/
async function initializeTest(buildConfigurations: CppBuildConfiguration[] | undefined,
Expand All @@ -66,8 +87,14 @@ async function initializeTest(buildConfigurations: CppBuildConfiguration[] | und
// Save active build config
if (activeBuildConfigName !== undefined) {
const storage = container.get<StorageService>(StorageService);
storage.setData('cpp.active-build-configuration', {
configName: activeBuildConfigName,
storage.setData('cpp.active-build-configurations-map', {
configs: [[
'/tmp',
{
name: 'Release',
directory: '/tmp/builds/release',
}
]],
});
}

Expand All @@ -81,7 +108,7 @@ describe('build-configurations', function () {
const cppBuildConfigurations = await initializeTest(undefined, undefined);

const configs = cppBuildConfigurations.getConfigs();
const active = cppBuildConfigurations.getActiveConfig();
const active = cppBuildConfigurations.getActiveConfig('/tmp');

expect(active).eq(undefined);
expect(configs).lengthOf(0);
Expand All @@ -91,7 +118,7 @@ describe('build-configurations', function () {
const cppBuildConfigurations = await initializeTest([], undefined);

const configs = cppBuildConfigurations.getConfigs();
const active = cppBuildConfigurations.getActiveConfig();
const active = cppBuildConfigurations.getActiveConfig('/tmp');

expect(active).eq(undefined);
expect(configs).lengthOf(0);
Expand All @@ -108,7 +135,7 @@ describe('build-configurations', function () {
const cppBuildConfigurations = await initializeTest(builds, undefined);

const configs = cppBuildConfigurations.getConfigs();
const active = cppBuildConfigurations.getActiveConfig();
const active = cppBuildConfigurations.getActiveConfig('/tmp');

expect(active).eq(undefined);
expect(configs).to.be.an('array').of.lengthOf(2);
Expand All @@ -125,8 +152,11 @@ describe('build-configurations', function () {
}];
const cppBuildConfigurations = await initializeTest(builds, 'Debug');

const manager = getManager();
manager.setActiveConfig(builds[1], '/tmp');

const configs = cppBuildConfigurations.getConfigs();
const active = cppBuildConfigurations.getActiveConfig();
const active = cppBuildConfigurations.getActiveConfig('/tmp');

expect(active).to.be.deep.eq(builds[1]);
expect(configs).to.be.an('array').of.lengthOf(2);
Expand All @@ -143,8 +173,11 @@ describe('build-configurations', function () {
}];
const cppBuildConfigurations = await initializeTest(builds, 'foobar');

const manager = getManager();
manager.setActiveConfig(undefined, '/tmp');

const configs = cppBuildConfigurations.getConfigs();
const active = cppBuildConfigurations.getActiveConfig();
const active = cppBuildConfigurations.getActiveConfig('/tmp');

expect(active).to.be.eq(undefined);
expect(configs).to.be.an('array').of.lengthOf(2);
Expand Down
Loading

0 comments on commit e779e8c

Please sign in to comment.