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

fix the workspace pref not being applied on change #5498

Merged
merged 2 commits into from
Jun 26, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ Breaking changes:
- `debug.restart` renamed to `workbench.action.debug.restart`
- [preferences] renamed overridenPreferenceName to overriddenPreferenceName
- [task] `cwd`, which used to be defined directly under `Task`, is moved into `Task.options` object
- [preferences] removed constructor from the `FolderPreferenceProvider` class
kittaakos marked this conversation as resolved.
Show resolved Hide resolved
- [workspace] `isMultiRootWorkspaceOpened()` is renamed into `isMultiRootWorkspaceEnabled()`

## v0.7.0

Expand Down
11 changes: 10 additions & 1 deletion packages/core/src/browser/preferences/preference-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,16 @@ export class PreferenceServiceImpl implements PreferenceService, FrontendApplica
if (scope > change.scope && value !== undefined) {
// preference defined in a more specific scope
break;
} else if (scope === change.scope) {
} else if (scope === change.scope && change.newValue !== undefined) {
// preference is changed into something other than `undefined`
acceptChange(change);
} else if (scope < change.scope && change.newValue === undefined && value !== undefined) {
// preference is changed to `undefined`, use the value from a more general scope
change = {
...change,
newValue: value,
scope
};
acceptChange(change);
}
}
Expand Down
3 changes: 1 addition & 2 deletions packages/debug/src/browser/debug-prefix-configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,12 @@ export class DebugPrefixConfiguration implements CommandContribution, CommandHan
}

async onType(_lookFor: string, acceptor: (items: QuickOpenItem[]) => void): Promise<void> {
const isMulti: boolean = !!this.workspaceService.workspace && !this.workspaceService.workspace.isDirectory;
const items: QuickOpenItem[] = [];
const configurations = this.debugConfigurationManager.all;
Array.from(configurations).forEach(config => {
items.push(new QuickOpenItem({
label: config.configuration.name,
description: isMulti
description: this.workspaceService.isMultiRootWorkspaceOpened
? this.labelProvider.getName(new URI(config.workspaceFolderUri))
: '',
run: (mode: QuickOpenMode) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export class DebugConfigurationWidget extends ReactWidget {
return configuration.name + '__CONF__' + workspaceFolderUri;
}
protected toName({ configuration, workspaceFolderUri }: DebugSessionOptions): string {
if (!workspaceFolderUri || !this.workspaceService.isMultiRootWorkspaceOpened) {
if (!workspaceFolderUri || !this.workspaceService.isMultiRootWorkspaceEnabled) {
return configuration.name;
}
return configuration.name + ' (' + new URI(workspaceFolderUri).path.base + ')';
Expand Down
2 changes: 1 addition & 1 deletion packages/file-search/src/browser/quick-file-open.ts
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ export class QuickFileOpenService implements QuickOpenModel, QuickOpenHandler {
private async toItem(uriOrString: URI | string, group?: QuickOpenGroupItemOptions) {
const uri = uriOrString instanceof URI ? uriOrString : new URI(uriOrString);
let description = this.labelProvider.getLongName(uri.parent);
if (this.workspaceService.workspace && !this.workspaceService.workspace.isDirectory) {
if (this.workspaceService.isMultiRootWorkspaceOpened) {
const rootUri = this.workspaceService.getWorkspaceRootUri(uri);
if (rootUri) {
description = `${rootUri.displayName} • ${description}`;
Expand Down
2 changes: 1 addition & 1 deletion packages/navigator/src/browser/navigator-widget.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export class FileNavigatorWidget extends FileTreeWidget {

protected getContainerTreeNode(): TreeNode | undefined {
const root = this.model.root;
if (this.workspaceService.isMultiRootWorkspaceOpened) {
if (this.workspaceService.isMultiRootWorkspaceEnabled) {
return root;
}
if (WorkspaceNode.is(root)) {
Expand Down
18 changes: 10 additions & 8 deletions packages/preferences/src/browser/folder-preference-provider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ import { inject, injectable } from 'inversify';
import URI from '@theia/core/lib/common/uri';
import { PreferenceScope } from '@theia/core/lib/browser';
import { AbstractResourcePreferenceProvider } from './abstract-resource-preference-provider';
import { FileSystem, FileStat } from '@theia/filesystem/lib/common';
import { FileStat } from '@theia/filesystem/lib/common';
import { WorkspaceService } from '@theia/workspace/lib/browser/workspace-service';

export const FolderPreferenceProviderFactory = Symbol('FolderPreferenceProviderFactory');
export interface FolderPreferenceProviderFactory {
Expand All @@ -34,21 +35,22 @@ export interface FolderPreferenceProviderOptions {
@injectable()
export class FolderPreferenceProvider extends AbstractResourcePreferenceProvider {

readonly folderUri: URI;
@inject(WorkspaceService) protected readonly workspaceService: WorkspaceService;
@inject(FolderPreferenceProviderOptions) protected readonly options: FolderPreferenceProviderOptions;

constructor(
@inject(FolderPreferenceProviderOptions) protected readonly options: FolderPreferenceProviderOptions,
@inject(FileSystem) protected readonly fileSystem: FileSystem
) {
super();
this.folderUri = new URI(this.options.folder.uri);
get folderUri(): URI {
return new URI(this.options.folder.uri);
}

protected getUri(): URI {
return this.options.configUri;
}

protected getScope(): PreferenceScope {
if (!this.workspaceService.isMultiRootWorkspaceOpened) {
// when FolderPreferenceProvider is used as a delegate of WorkspacePreferenceProvider in a one-folder workspace
return PreferenceScope.Workspace;
}
return PreferenceScope.Folder;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export class WorkspacePreferenceProvider extends PreferenceProvider {
if (!workspace) {
return undefined;
}
if (workspace.isDirectory) {
if (!this.workspaceService.isMultiRootWorkspaceOpened) {
return this.folderPreferenceProvider;
}
return this.workspaceFileProviderFactory({
Expand Down Expand Up @@ -109,9 +109,8 @@ export class WorkspacePreferenceProvider extends PreferenceProvider {
}

protected ensureResourceUri(): string | undefined {
const workspace = this.workspaceService.workspace;
if (workspace && workspace.isDirectory) {
return workspace.uri;
if (this.workspaceService.workspace && !this.workspaceService.isMultiRootWorkspaceOpened) {
return this.workspaceService.workspace.uri;
}
return undefined;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,7 @@ export class SearchInWorkspaceResultTreeWidget extends TreeWidget {
id: rootUri,
parent: this.model.root as SearchInWorkspaceRoot,
icon: FOLDER_ICON,
visible: this.workspaceService.workspace && !this.workspaceService.workspace.isDirectory
visible: this.workspaceService.isMultiRootWorkspaceOpened
};
}

Expand Down
6 changes: 3 additions & 3 deletions packages/workspace/src/browser/workspace-commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ export class WorkspaceCommandContribution implements CommandContribution {
registry.registerCommand(WorkspaceCommands.FILE_COMPARE, this.newMultiUriAwareCommandHandler(this.compareHandler));
this.preferences.ready.then(() => {
registry.registerCommand(WorkspaceCommands.ADD_FOLDER, this.newMultiUriAwareCommandHandler({
isEnabled: () => this.workspaceService.isMultiRootWorkspaceOpened,
isEnabled: () => this.workspaceService.isMultiRootWorkspaceEnabled,
isVisible: uris => !uris.length || this.areWorkspaceRoots(uris),
execute: async uris => {
const uri = await this.fileDialogService.showOpenDialog({
Expand Down Expand Up @@ -293,7 +293,7 @@ export class WorkspaceCommandContribution implements CommandContribution {
}));
registry.registerCommand(WorkspaceCommands.REMOVE_FOLDER, this.newMultiUriAwareCommandHandler({
execute: uris => this.removeFolderFromWorkspace(uris),
isEnabled: () => this.workspaceService.isMultiRootWorkspaceOpened,
isEnabled: () => this.workspaceService.isMultiRootWorkspaceEnabled,
isVisible: uris => this.areWorkspaceRoots(uris) && this.workspaceService.saved
}));
});
Expand Down Expand Up @@ -441,7 +441,7 @@ export class WorkspaceRootUriAwareCommandHandler extends UriAwareCommandHandler<

protected getUri(): URI | undefined {
const uri = super.getUri();
if (this.workspaceService.isMultiRootWorkspaceOpened) {
if (this.workspaceService.isMultiRootWorkspaceEnabled) {
return uri;
}
if (uri) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ export class WorkspaceFrontendContribution implements CommandContribution, Keybi
execute: () => this.quickOpenWorkspace.select()
});
commands.registerCommand(WorkspaceCommands.SAVE_WORKSPACE_AS, {
isEnabled: () => this.workspaceService.isMultiRootWorkspaceOpened,
isEnabled: () => this.workspaceService.isMultiRootWorkspaceEnabled,
execute: () => this.saveWorkspaceAs()
});
commands.registerCommand(WorkspaceCommands.SAVE_AS,
Expand Down
28 changes: 25 additions & 3 deletions packages/workspace/src/browser/workspace-service.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -600,10 +600,10 @@ describe('WorkspaceService', () => {
});
});

describe('isMultiRootWorkspaceOpened status', () => {
describe('isMultiRootWorkspaceEnabled status', () => {
it('should be true if there is an opened workspace and preference["workspace.supportMultiRootWorkspace"] = true, otherwise false', () => {
mockPreferenceValues['workspace.supportMultiRootWorkspace'] = true;
expect(wsService.isMultiRootWorkspaceOpened).to.be.false;
expect(wsService.isMultiRootWorkspaceEnabled).to.be.false;

const file = <FileStat>{
uri: 'file:///home/file',
Expand All @@ -612,9 +612,31 @@ describe('WorkspaceService', () => {
};
wsService['_workspace'] = file;
mockPreferenceValues['workspace.supportMultiRootWorkspace'] = true;
expect(wsService.isMultiRootWorkspaceOpened).to.be.true;
expect(wsService.isMultiRootWorkspaceEnabled).to.be.true;

mockPreferenceValues['workspace.supportMultiRootWorkspace'] = false;
expect(wsService.isMultiRootWorkspaceEnabled).to.be.false;
});
});

describe('isMultiRootWorkspaceOpened status', () => {
it('should be true if there is an opened workspace and the workspace is not a directory, otherwise false', () => {
expect(wsService.isMultiRootWorkspaceOpened).to.be.false;

const file = <FileStat>{
uri: 'file:///home/file',
lastModification: 0,
isDirectory: false
};
wsService['_workspace'] = file;
expect(wsService.isMultiRootWorkspaceOpened).to.be.true;

const dir = <FileStat>{
uri: 'file:///home/dir',
lastModification: 0,
isDirectory: true
};
wsService['_workspace'] = dir;
expect(wsService.isMultiRootWorkspaceOpened).to.be.false;
});
});
Expand Down
10 changes: 9 additions & 1 deletion packages/workspace/src/browser/workspace-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -261,10 +261,18 @@ export class WorkspaceService implements FrontendApplicationContribution {
}

/**
* Returns `true` if there is an opened workspace in theia, and the workspace has more than one root.
* Returns `true` if a multiple-root workspace is currently open.
* @returns {boolean}
*/
get isMultiRootWorkspaceOpened(): boolean {
return !!this.workspace && !this.workspace.isDirectory;
elaihau marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Returns `true` if there is an opened workspace, and multi root workspace support is enabled.
* @returns {boolean}
*/
get isMultiRootWorkspaceEnabled(): boolean {
return this.opened && this.preferences['workspace.supportMultiRootWorkspace'];
}

Expand Down