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 <[email protected]>
  • Loading branch information
vince-fugnitto committed Mar 29, 2019
1 parent 9b6234e commit d6dad75
Show file tree
Hide file tree
Showing 7 changed files with 219 additions and 125 deletions.
1 change: 1 addition & 0 deletions packages/cpp/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
"@theia/monaco": "^0.5.0",
"@theia/preferences": "^0.5.0",
"@theia/process": "^0.5.0",
"@theia/workspace": "^0.5.0",
"@theia/task": "^0.5.0",
"string-argv": "^0.1.1"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { injectable, inject } from 'inversify';
import { StatusBar, StatusBarAlignment } from '@theia/core/lib/browser';
import { CppBuildConfigurationManager, CppBuildConfiguration } from './cpp-build-configurations';
import { CPP_CHANGE_BUILD_CONFIGURATION } from './cpp-build-configurations-ui';
import { WorkspaceService } from '@theia/workspace/lib/browser';

@injectable()
export class CppBuildConfigurationsStatusBarElement {
Expand All @@ -28,15 +29,18 @@ export class CppBuildConfigurationsStatusBarElement {
@inject(StatusBar)
protected readonly statusBar: StatusBar;

@inject(WorkspaceService)
protected readonly workspaceService: WorkspaceService;

protected readonly cppIdentifier = 'cpp-configurator';

/**
* Display the `CppBuildConfiguration` status bar element,
* 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 +49,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 (${count} of ${this.workspaceService.tryGetRoots().length})`,
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;
}

}
147 changes: 79 additions & 68 deletions packages/cpp/src/browser/cpp-build-configurations-ui.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@
import { Command, CommandContribution, CommandRegistry, CommandService } from '@theia/core';
import { injectable, inject } from 'inversify';
import { QuickOpenService } from '@theia/core/lib/browser/quick-open/quick-open-service';
import { QuickOpenModel, QuickOpenItem, QuickOpenMode, } from '@theia/core/lib/browser/quick-open/quick-open-model';
import { FileSystem, FileSystemUtils } from '@theia/filesystem/lib/common';
import { FileSystem } from '@theia/filesystem/lib/common';
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 { CppBuildConfigurationManager, CppBuildConfiguration, CPP_BUILD_CONFIGURATIONS_PREFERENCE_KEY, isCppBuildConfiguration, equals } from './cpp-build-configurations';
import { EditorManager } from '@theia/editor/lib/browser';
import { CommonCommands } from '@theia/core/lib/browser';
import { CommonCommands, QuickPickService, LabelProvider, QuickPickItem } from '@theia/core/lib/browser';
import { WorkspaceService } from '@theia/workspace/lib/browser';

@injectable()
export class CppBuildConfigurationChanger implements QuickOpenModel {
export class CppBuildConfigurationChanger {

@inject(CommandService)
protected readonly commandService: CommandService;
Expand All @@ -40,88 +40,100 @@ 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;

readonly createItem: QuickOpenItem = new QuickOpenItem({
@inject(WorkspaceService)
protected readonly workspaceService: WorkspaceService;

/**
* Item used to trigger creation of a new build configuration.
*/
protected readonly createItem: QuickPickItem<'createNew'> = ({
label: 'Create New',
iconClass: 'fa fa-plus',
value: 'createNew',
description: 'Create a new build configuration',
run: (mode: QuickOpenMode): boolean => {
if (mode !== QuickOpenMode.OPEN) {
return false;
}
this.commandService.executeCommand(CPP_CREATE_NEW_BUILD_CONFIGURATION.id);
return true;
},
iconClass: 'fa fa-plus'
});

readonly resetItem: QuickOpenItem = new QuickOpenItem({
/**
* Item used to trigger reset of the active build configuration.
*/
protected readonly resetItem: QuickPickItem<'reset'> = ({
label: 'None',
iconClass: 'fa fa-times',
description: 'Reset active build configuration',
run: (mode: QuickOpenMode): boolean => {
if (mode !== QuickOpenMode.OPEN) {
return false;
}
this.commandService.executeCommand(CPP_RESET_BUILD_CONFIGURATION.id);
return true;
},
value: 'reset',
description: 'Reset the active build configuration',
iconClass: 'fa fa-times'
});

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();
/**
* Change the build configuration for a given root.
* If multiple roots are available, prompt users a first time to select their desired root.
* Once a root is determined, prompt users to select an active build configuration if applicable.
*/
async change(): Promise<void> {
// Prompt users to determine working root.
const root = await this.selectWorkspaceRoot();
// Prompt users to determine action (set active config, reset active config, create new config).
const action = await this.selectCppAction(root);
// Perform desired action.
if (action === 'createNew') {
this.commandService.executeCommand(CPP_CREATE_NEW_BUILD_CONFIGURATION.id);
}
if (action === 'reset') {
this.cppBuildConfigurations.setActiveConfig(undefined, root);
}
if (action && isCppBuildConfiguration(action)) {
this.cppBuildConfigurations.setActiveConfig(action, root);
}
}

const homeStat = await this.fileSystem.getCurrentUserHome();
const home = (homeStat) ? new URI(homeStat.uri).withoutScheme().toString() : undefined;
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' });
}

// Item to create a new build configuration
protected async selectCppAction(root: string | undefined): Promise<string | CppBuildConfiguration | undefined> {
const items: QuickPickItem<'createNew' | 'reset' | CppBuildConfiguration>[] = [];
// Add the 'Create New' item at all times.
items.push(this.createItem);

// Only return 'Create New' when no build configurations present
if (!configurations.length) {
return acceptor(items);
}

// Item to de-select any active build config
if (active) {
// Add the 'Reset' item if there currently is an active config.
if (this.cppBuildConfigurations.getActiveConfig(root)) {
items.push(this.resetItem);
}

// Add one item per build config
configurations.forEach(config => {
const uri = new URI(config.directory);
items.push(new QuickOpenItem({
// Display all valid configurations for a given root.
const configs = this.cppBuildConfigurations.getValidConfigs(root);
const active = this.cppBuildConfigurations.getActiveConfig(root);
configs.map(config => {
items.push({
label: config.name,
// add an icon for active build config, and an empty placeholder for all others
iconClass: (config === active) ? 'fa fa-check' : 'fa fa-empty-item',
description: (home) ? FileSystemUtils.tildifyPath(uri.path.toString(), home) : uri.path.toString(),
run: (mode: QuickOpenMode): boolean => {
if (mode !== QuickOpenMode.OPEN) {
return false;
}

this.cppBuildConfigurations.setActiveConfig(config);
return true;
description: config.directory,
iconClass: active && equals(config, active) ? 'fa fa-check' : 'fa fa-empty-item',
value: {
name: config.name,
directory: config.directory,
commands: config.commands
},
}));
});

acceptor(items);
}

open() {
const configs = this.cppBuildConfigurations.getValidConfigs();
this.quickOpenService.open(this, {
placeholder: (configs.length) ? 'Choose a build configuration...' : 'No build configurations present',
fuzzyMatchLabel: true,
fuzzyMatchDescription: true,
});
});
return this.quickPick.show(items, { placeholder: 'Select action' });
}

/** Create a new build configuration with placeholder values. */
Expand All @@ -131,7 +143,6 @@ export class CppBuildConfigurationChanger implements QuickOpenModel {
configs.push({ name: '', directory: '' });
await this.preferenceService.set(CPP_BUILD_CONFIGURATIONS_PREFERENCE_KEY, configs, PreferenceScope.Workspace);
}

}

export const CPP_CATEGORY = 'C/C++';
Expand Down Expand Up @@ -183,7 +194,7 @@ export class CppBuildConfigurationsContributions implements CommandContribution
execute: () => this.cppChangeBuildConfiguration.createConfig()
});
commands.registerCommand(CPP_CHANGE_BUILD_CONFIGURATION, {
execute: () => this.cppChangeBuildConfiguration.open()
execute: () => this.cppChangeBuildConfiguration.change()
});
}
}
51 changes: 42 additions & 9 deletions packages/cpp/src/browser/cpp-build-configurations.spec.ts
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 d6dad75

Please sign in to comment.