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.

Sign up for GitHub

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

Use workspace.findFiles instead of glob #3681

Merged
merged 4 commits into from
Mar 19, 2020
Merged

Use workspace.findFiles instead of glob #3681

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e3bfd3a
Use workspace.findFiles instead of glob
JoeRobich Mar 18, 2020
a13f2f3
Add tests for getExcludedPaths
JoeRobich Mar 18, 2020
4be30aa
Optionally include search.exclude with excluded paths
JoeRobich Mar 18, 2020
bf46264
Pass null URI when getting excluded paths
JoeRobich Mar 18, 2020
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
49 changes: 30 additions & 19 deletions src/omnisharp/options.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,8 @@ export class Options {
public defaultLaunchSolution?: string,
public monoPath?: string,
public excludePaths?: string[],
public maxProjectFileCountForDiagnosticAnalysis?: number | null)
{
}
public maxProjectFileCountForDiagnosticAnalysis?: number | null) {
}

public static Read(vscode: vscode): Options {
// Extra effort is taken below to ensure that legacy versions of options
Expand Down Expand Up @@ -65,7 +64,7 @@ export class Options {
const maxProjectResults = omnisharpConfig.get<number>('maxProjectResults', 250);
const defaultLaunchSolution = omnisharpConfig.get<string>('defaultLaunchSolution', undefined);
const useEditorFormattingSettings = omnisharpConfig.get<boolean>('useEditorFormattingSettings', true);

const enableRoslynAnalyzers = omnisharpConfig.get<boolean>('enableRoslynAnalyzers', false);
const enableEditorConfigSupport = omnisharpConfig.get<boolean>('enableEditorConfigSupport', false);

Expand All @@ -89,21 +88,7 @@ export class Options {

const maxProjectFileCountForDiagnosticAnalysis = csharpConfig.get<number | null>('maxProjectFileCountForDiagnosticAnalysis', 1000);

let workspaceConfig = vscode.workspace.getConfiguration();
let excludePaths = [];
if (workspaceConfig)
{
let excludeFilesOption = workspaceConfig.get<{ [i: string]: boolean }>('files.exclude');
if (excludeFilesOption)
{
for (let field in excludeFilesOption) {
if (excludeFilesOption[field]) {
excludePaths.push(field);
}
}
}
}

const excludePaths = this.getExcludedPaths(vscode);

return new Options(
path,
Expand Down Expand Up @@ -134,6 +119,32 @@ export class Options {
);
}

public static getExcludedPaths(vscode: vscode, includeSearchExcludes: boolean = false): string[] {
let workspaceConfig = vscode.workspace.getConfiguration(undefined, null);
if (!workspaceConfig) {
return [];
}

let excludePaths = getExcludes(workspaceConfig, 'files.exclude');

if (includeSearchExcludes) {
excludePaths = excludePaths.concat(getExcludes(workspaceConfig, 'search.exclude'));
}

return excludePaths;

function getExcludes(config: WorkspaceConfiguration, option: string): string[] {
let optionValue = config.get<{ [i: string]: boolean }>(option);
if (!optionValue) {
return [];
}

return Object.entries(optionValue)
.filter(([key, value]) => value)
.map(([key, value]) => key);
}
}

private static readPathOption(csharpConfig: WorkspaceConfiguration, omnisharpConfig: WorkspaceConfiguration): string | null {
if (omnisharpConfig.has('path')) {
// If 'omnisharp.path' setting was found, use it.
Expand Down
28 changes: 18 additions & 10 deletions src/omnisharp/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
*--------------------------------------------------------------------------------------------*/

import * as fs from 'fs-extra';
import * as glob from 'glob';
import { OmniSharpServer } from './server';
import * as path from 'path';
import * as protocol from './protocol';
import * as vscode from 'vscode';
import { MSBuildProject } from './protocol';
import { Options } from './options';

export async function autoComplete(server: OmniSharpServer, request: protocol.AutoCompleteRequest, token: vscode.CancellationToken) {
return server.makeRequest<protocol.AutoCompleteResponse[]>(protocol.Requests.AutoComplete, request, token);
Expand Down Expand Up @@ -71,14 +71,20 @@ export async function requestWorkspaceInformation(server: OmniSharpServer) {
const response = await server.makeRequest<protocol.WorkspaceInformationResponse>(protocol.Requests.Projects);
if (response.MsBuild && response.MsBuild.Projects) {
const blazorDetectionEnabled = hasBlazorWebAssemblyDebugPrerequisites();
let blazorWebAssemblyProjectFound = false;

for (const project of response.MsBuild.Projects) {
const isProjectBlazorWebAssemblyProject = await isBlazorWebAssemblyProject(project);
const isProjectBlazorWebAssemblyHosted = isBlazorWebAssemblyHosted(project, isProjectBlazorWebAssemblyProject);

project.IsWebProject = isWebProject(project);
project.IsBlazorWebAssemblyHosted = blazorDetectionEnabled && isBlazorWebAssemblyHosted(project);
project.IsBlazorWebAssemblyStandalone = blazorDetectionEnabled && !project.IsBlazorWebAssemblyHosted && isBlazorWebAssemblyProject(project);
project.IsBlazorWebAssemblyHosted = blazorDetectionEnabled && isProjectBlazorWebAssemblyHosted;
project.IsBlazorWebAssemblyStandalone = blazorDetectionEnabled && isProjectBlazorWebAssemblyProject && !project.IsBlazorWebAssemblyHosted;

blazorWebAssemblyProjectFound = blazorWebAssemblyProjectFound || isProjectBlazorWebAssemblyProject;
}

if (!blazorDetectionEnabled && response.MsBuild.Projects.some(project => isBlazorWebAssemblyProject(project))) {
if (!blazorDetectionEnabled && blazorWebAssemblyProjectFound) {
// There's a Blazor Web Assembly project but VSCode isn't configured to debug the WASM code, show a notification
// to help the user configure their VSCode appropriately.
vscode.window.showInformationMessage('Additional setup is required to debug Blazor WebAssembly applications.', 'Learn more', 'Close')
Expand Down Expand Up @@ -150,8 +156,8 @@ export async function isNetCoreProject(project: protocol.MSBuildProject) {
return project.TargetFrameworks.find(tf => tf.ShortName.startsWith('netcoreapp') || tf.ShortName.startsWith('netstandard')) !== undefined;
}

function isBlazorWebAssemblyHosted(project: protocol.MSBuildProject): boolean {
if (!isBlazorWebAssemblyProject(project)) {
function isBlazorWebAssemblyHosted(project: protocol.MSBuildProject, isProjectBlazorWebAssemblyProject: boolean): boolean {
if (!isProjectBlazorWebAssemblyProject) {
return false;
}

Expand All @@ -170,17 +176,19 @@ function isBlazorWebAssemblyHosted(project: protocol.MSBuildProject): boolean {
return true;
}

function isBlazorWebAssemblyProject(project: MSBuildProject): boolean {
async function isBlazorWebAssemblyProject(project: MSBuildProject): Promise<boolean> {
const projectDirectory = path.dirname(project.Path);
const launchSettings = glob.sync('**/launchSettings.json', { cwd: projectDirectory });
const launchSettingsPattern = new vscode.RelativePattern(projectDirectory, '**/launchSettings.json');
const excludedPathPattern = `{${Options.getExcludedPaths(vscode, true).join(',')}}`;

const launchSettings = await vscode.workspace.findFiles(launchSettingsPattern, excludedPathPattern);
if (!launchSettings) {
return false;
}

for (const launchSetting of launchSettings) {
try {
const absoluteLaunchSetting = path.join(projectDirectory, launchSetting);
const launchSettingContent = fs.readFileSync(absoluteLaunchSetting);
const launchSettingContent = fs.readFileSync(launchSetting.fsPath);
if (!launchSettingContent) {
continue;
}
Expand Down
65 changes: 45 additions & 20 deletions test/unitTests/options.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import { getVSCodeWithConfig, updateConfig } from './testAssets/Fakes';
suite("Options tests", () => {
suiteSetup(() => should());

test('Verify defaults', () =>
{
test('Verify defaults', () => {
const vscode = getVSCodeWithConfig();
const options = Options.Read(vscode);
expect(options.path).to.be.null;
Expand All @@ -36,8 +35,41 @@ suite("Options tests", () => {
expect(options.defaultLaunchSolution).to.be.undefined;
});

test('BACK-COMPAT: "omnisharp.loggingLevel": "verbose" == "omnisharp.loggingLevel": "debug"', () =>
{
test('Verify return no excluded paths when files.exclude empty', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, null, 'files.exclude', {});

const excludedPaths = Options.getExcludedPaths(vscode);
expect(excludedPaths).to.be.empty;
});

test('Verify return excluded paths when files.exclude populated', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, null, 'files.exclude', { "**/node_modules": true, "**/assets": false });

const excludedPaths = Options.getExcludedPaths(vscode);
expect(excludedPaths).to.equalTo(["**/node_modules"]);
});

test('Verify return no excluded paths when files.exclude and search.exclude empty', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, null, 'files.exclude', {});
updateConfig(vscode, null, 'search.exclude', {});

const excludedPaths = Options.getExcludedPaths(vscode, true);
expect(excludedPaths).to.be.empty;
});

test('Verify return excluded paths when files.exclude and search.exclude populated', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, null, 'files.exclude', { "/Library": true });
updateConfig(vscode, null, 'search.exclude', { "**/node_modules": true, "**/assets": false });

const excludedPaths = Options.getExcludedPaths(vscode, true);
expect(excludedPaths).to.be.equalTo(["/Library", "**/node_modules"]);
});

test('BACK-COMPAT: "omnisharp.loggingLevel": "verbose" == "omnisharp.loggingLevel": "debug"', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, 'omnisharp', 'loggingLevel', "verbose");

Expand All @@ -46,28 +78,25 @@ suite("Options tests", () => {
options.loggingLevel.should.equal("debug");
});

test('BACK-COMPAT: "omnisharp.useMono": true == "omnisharp.useGlobalMono": "always"', () =>
{
test('BACK-COMPAT: "omnisharp.useMono": true == "omnisharp.useGlobalMono": "always"', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, 'omnisharp', 'useMono', true);

const options = Options.Read(vscode);

options.useGlobalMono.should.equal("always");
});

test('BACK-COMPAT: "omnisharp.useMono": false == "omnisharp.useGlobalMono": "auto"', () =>
{
test('BACK-COMPAT: "omnisharp.useMono": false == "omnisharp.useGlobalMono": "auto"', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, 'omnisharp', 'useMono', false);

const options = Options.Read(vscode);

options.useGlobalMono.should.equal("auto");
});

test('BACK-COMPAT: "csharp.omnisharpUsesMono": true == "omnisharp.useGlobalMono": "always"', () =>
{
test('BACK-COMPAT: "csharp.omnisharpUsesMono": true == "omnisharp.useGlobalMono": "always"', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, 'csharp', 'omnisharpUsesMono', true);

Expand All @@ -76,8 +105,7 @@ suite("Options tests", () => {
options.useGlobalMono.should.equal("always");
});

test('BACK-COMPAT: "csharp.omnisharpUsesMono": false == "omnisharp.useGlobalMono": "auto"', () =>
{
test('BACK-COMPAT: "csharp.omnisharpUsesMono": false == "omnisharp.useGlobalMono": "auto"', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, 'csharp', 'omnisharpUsesMono', false);

Expand All @@ -86,8 +114,7 @@ suite("Options tests", () => {
options.useGlobalMono.should.equal("auto");
});

test('BACK-COMPAT: "csharp.omnisharp" is used if it is set and "omnisharp.path" is not', () =>
{
test('BACK-COMPAT: "csharp.omnisharp" is used if it is set and "omnisharp.path" is not', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, 'csharp', 'omnisharp', 'OldPath');

Expand All @@ -96,8 +123,7 @@ suite("Options tests", () => {
options.path.should.equal("OldPath");
});

test('BACK-COMPAT: "csharp.omnisharp" is not used if "omnisharp.path" is set', () =>
{
test('BACK-COMPAT: "csharp.omnisharp" is not used if "omnisharp.path" is set', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, 'omnisharp', 'path', 'NewPath');
updateConfig(vscode, 'csharp', 'omnisharp', 'OldPath');
Expand All @@ -107,8 +133,7 @@ suite("Options tests", () => {
options.path.should.equal("NewPath");
});

test('"omnisharp.defaultLaunchSolution" is used if set', () =>
{
test('"omnisharp.defaultLaunchSolution" is used if set', () => {
const vscode = getVSCodeWithConfig();
updateConfig(vscode, 'omnisharp', 'defaultLaunchSolution', 'some_valid_solution.sln');

Expand Down
5 changes: 5 additions & 0 deletions test/unitTests/testAssets/Fakes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,10 +185,15 @@ export function getWorkspaceInformationUpdated(msbuild: protocol.MsBuildWorkspac
export function getVSCodeWithConfig() {
const vscode = getFakeVsCode();

const _vscodeConfig = getWorkspaceConfiguration();
const _omnisharpConfig = getWorkspaceConfiguration();
const _csharpConfig = getWorkspaceConfiguration();

vscode.workspace.getConfiguration = (section?, resource?) => {
if (!section) {
return _vscodeConfig;
}

if (section === 'omnisharp') {
return _omnisharpConfig;
}
Expand Down