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

Fixing a couple vscode issues #1035

Merged
merged 1 commit into from
Nov 21, 2023
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
4 changes: 4 additions & 0 deletions Src/CSharpier.VSCode/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## [1.5.1]
- Fixed bug with being unable to setup CSharpier when trying to use a global version of csharpier >= 0.26.0
- Fixed bug with being unable to setup CSharpier with username containing space

## [1.5.0]
- Improved error handling and reporting around csharpier failing to install or run

Expand Down
4 changes: 2 additions & 2 deletions Src/CSharpier.VSCode/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
"name": "csharpier-vscode",
"displayName": "CSharpier - Code formatter",
"description": "Code formatter using csharpier",
"version": "1.5.0",
"version": "1.5.1",
"publisher": "csharpier",
"author": "CSharpier",
"homepage": "https://marketplace.visualstudio.com/items?itemName=csharpier.csharpier-vscode",
Expand Down Expand Up @@ -81,4 +81,4 @@
"webpack-cli": "4.9.1",
"xml-js": "1.6.11"
}
}
}
13 changes: 11 additions & 2 deletions Src/CSharpier.VSCode/src/CSharpierProcess.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,24 @@
import { Disposable } from "vscode";
import { Logger } from "./Logger";

export interface ICSharpierProcess extends Disposable {
formatFile(content: string, filePath: string): Promise<string>;
}

export class NullCSharpierProcess implements ICSharpierProcess {
static instance = new NullCSharpierProcess();
static instance: NullCSharpierProcess;
private logger: Logger;

private constructor() {}
static create(logger: Logger) {
this.instance = new NullCSharpierProcess(logger);
}

private constructor(logger: Logger) {
this.logger = logger;
}

formatFile(content: string, filePath: string): Promise<string> {
this.logger.debug("Skipping formatting because this is a NullCSharpierProcess. This generally indicates there was a problem starting the CSharpier process");
return Promise.resolve("");
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ export class CSharpierProcessPipeMultipleFiles implements ICSharpierProcess {
private callbacks: ((result: string) => void)[] = [];
private logger: Logger;
private nextFile: string = "";
private processFailedToStart = false;
public processFailedToStart = false;

constructor(logger: Logger, csharpierPath: string, workingDirectory: string) {
this.logger = logger;
Expand Down
24 changes: 18 additions & 6 deletions Src/CSharpier.VSCode/src/CSharpierProcessProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -145,8 +145,10 @@ export class CSharpierProcessProvider implements Disposable {
.toString()
.trim();

this.logger.debug(`dotnet csharpier --version output ${outputFromCsharpier}`);
return outputFromCsharpier;
this.logger.debug(`dotnet csharpier --version output: ${outputFromCsharpier}`);
const versionWithoutHash = outputFromCsharpier.split("+")[0]
this.logger.debug(`Using ${versionWithoutHash} as the version number.`)
return versionWithoutHash;
} catch (error: any) {
const message = !error.stderr ? error.toString() : error.stderr.toString();

Expand Down Expand Up @@ -210,9 +212,8 @@ export class CSharpierProcessProvider implements Disposable {
}

if (!this.customPathInstaller.ensureVersionInstalled(version)) {
window.showErrorMessage(
"CSharpier could not be set up properly so formatting is not currently supported. See Output - CSharpier for details.",
);
this.logger.debug(`Unable to validate install of version ${version}`)
this.displayFailureMessage();
return NullCSharpierProcess.instance;
}

Expand All @@ -229,7 +230,12 @@ export class CSharpierProcessProvider implements Disposable {
}
return new CSharpierProcessSingleFile(this.logger, customPath);
} else {
return new CSharpierProcessPipeMultipleFiles(this.logger, customPath, directory);
const csharpierProcess = new CSharpierProcessPipeMultipleFiles(this.logger, customPath, directory);
if (csharpierProcess.processFailedToStart) {
this.displayFailureMessage();
}

return csharpierProcess;
}
} catch (ex: any) {
this.logger.error(ex.output.toString());
Expand All @@ -253,4 +259,10 @@ export class CSharpierProcessProvider implements Disposable {
this.csharpierVersionByDirectory = {};
this.csharpierProcessesByVersion = {};
};

private displayFailureMessage() {
window.showErrorMessage(
"CSharpier could not be set up properly so formatting is not currently supported. See Output - CSharpier for details.",
);
}
}
12 changes: 9 additions & 3 deletions Src/CSharpier.VSCode/src/CustomPathInstaller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export class CustomPathInstaller {

const pathToDirectoryForVersion = this.getDirectoryForVersion(version);
if (fs.existsSync(pathToDirectoryForVersion)) {
this.logger.debug("Validating install at " + pathToDirectoryForVersion);
if (this.validateInstall(pathToDirectoryForVersion, version)) {
return true;
}
Expand All @@ -44,18 +45,23 @@ export class CustomPathInstaller {

private validateInstall(pathToDirectoryForVersion: string, version: string): boolean {
try {
const output = execSync(`${this.getPathForVersion(version)} --version`, {
const output = execSync(`"${this.getPathForVersion(version)}" --version`, {
env: { ...process.env, DOTNET_NOLOGO: "1" },
})
.toString()
.trim();

this.logger.debug("dotnet csharpier --version output: " + output);
this.logger.debug(`"${this.getPathForVersion(version)}" --version output: ${output}`);
const versionWithoutHash = output.split("+")[0]
this.logger.debug(`Using ${versionWithoutHash} as the version number.`)

if (output.split("+")[0] === version) {
if (versionWithoutHash === version) {
this.logger.debug("CSharpier at " + pathToDirectoryForVersion + " already exists");
return true;
}
else {
this.logger.warn("Version of " + versionWithoutHash + " did not match expected version of " + version)
}
} catch (error: any) {
const message = !error.stderr ? error.toString() : error.stderr.toString();
this.logger.warn(
Expand Down
2 changes: 2 additions & 0 deletions Src/CSharpier.VSCode/src/Extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { ExtensionContext, workspace } from "vscode";
import { CSharpierProcessProvider } from "./CSharpierProcessProvider";
import { FormattingService } from "./FormattingService";
import { Logger } from "./Logger";
import { NullCSharpierProcess } from "./CSharpierProcess";

export async function activate(context: ExtensionContext) {
if (!workspace.isTrusted) {
Expand All @@ -17,6 +18,7 @@ const initPlugin = async (context: ExtensionContext) => {
workspace.getConfiguration("csharpier").get<boolean>("enableDebugLogs") ?? false;

const logger = new Logger(enableDebugLogs);
NullCSharpierProcess.create(logger);

const isDevelopment = (process.env as any).MODE === "development";

Expand Down
Loading