Fixing a couple vscode issues

closes #1003
belav · Nov 21, 2023 · 17b7b6d · 17b7b6d
1 parent 14d9a79
commit 17b7b6d
Show file tree

Hide file tree

Showing 7 changed files with 47 additions and 14 deletions.
diff --git a/Src/CSharpier.VSCode/CHANGELOG.md b/Src/CSharpier.VSCode/CHANGELOG.md
@@ -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
 

diff --git a/Src/CSharpier.VSCode/package.json b/Src/CSharpier.VSCode/package.json
@@ -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",
@@ -81,4 +81,4 @@
         "webpack-cli": "4.9.1",
         "xml-js": "1.6.11"
     }
-}
+}
diff --git a/Src/CSharpier.VSCode/src/CSharpierProcess.ts b/Src/CSharpier.VSCode/src/CSharpierProcess.ts
@@ -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("");
     }
 

diff --git a/Src/CSharpier.VSCode/src/CSharpierProcessPipeMultipleFiles.ts b/Src/CSharpier.VSCode/src/CSharpierProcessPipeMultipleFiles.ts
@@ -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;

diff --git a/Src/CSharpier.VSCode/src/CSharpierProcessProvider.ts b/Src/CSharpier.VSCode/src/CSharpierProcessProvider.ts
@@ -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();
 
@@ -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;
             }
 
@@ -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());
@@ -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.",
+        );
+    }
 }
diff --git a/Src/CSharpier.VSCode/src/CustomPathInstaller.ts b/Src/CSharpier.VSCode/src/CustomPathInstaller.ts
@@ -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;
             }
@@ -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(

diff --git a/Src/CSharpier.VSCode/src/Extension.ts b/Src/CSharpier.VSCode/src/Extension.ts
@@ -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) {
@@ -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";