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 issue with csharpier server not using dotnet root #1239

Merged
merged 1 commit into from
Apr 26, 2024
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
3 changes: 3 additions & 0 deletions Src/CSharpier.VSCode/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
## [1.7.1]
- Fix issue with csharpier server not supporting dotnet root

## [1.7.0]
- Use CSharpier Http Server for 0.28.0+
- Log version of CSharpier used to format a given file
Expand Down
2 changes: 1 addition & 1 deletion 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.7.0",
"version": "1.7.1",
"publisher": "csharpier",
"author": "CSharpier",
"homepage": "https://marketplace.visualstudio.com/items?itemName=csharpier.csharpier-vscode",
Expand Down
20 changes: 16 additions & 4 deletions Src/CSharpier.VSCode/src/CSharpierProcessServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { ChildProcessWithoutNullStreams, spawn } from "child_process";
import { Logger } from "./Logger";
import { FormatFileParameter, FormatFileResult, ICSharpierProcess2 } from "./ICSharpierProcess";
import fetch from "node-fetch";
import { getDotNetRoot } from "./DotNetProvider";

export class CSharpierProcessServer implements ICSharpierProcess2 {
private csharpierPath: string;
Expand All @@ -19,8 +20,9 @@ export class CSharpierProcessServer implements ICSharpierProcess2 {

this.logger.debug("Warm CSharpier with initial format");
// warm by formatting a file twice, the 3rd time is when it gets really fast
this.formatFile("public class ClassName { }", "/Temp/Test.cs").then(() => {
this.formatFile("public class ClassName { }", "/Temp/Test.cs");
// make sure we give a path that should not exist to avoid any errors when trying to find config files etc.
this.formatFile("public class ClassName { }", `/${Date.now()}/Test.cs`).then(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the goal is for the directory to not exists, why not just use ""?

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I remember correctly, csharpier is unhappy if you do that. There should probably just be an option for "no file path" and 0.28.0 would have been the time to add it. Dammit.

this.formatFile("public class ClassName { }", `/${Date.now()}/Test.cs`);
});
}

Expand All @@ -32,7 +34,7 @@ export class CSharpierProcessServer implements ICSharpierProcess2 {
const csharpierProcess = spawn(csharpierPath, ["--server"], {
stdio: "pipe",
cwd: workingDirectory,
env: { ...process.env, DOTNET_NOLOGO: "1" },
env: { ...process.env, DOTNET_NOLOGO: "1", DOTNET_ROOT: getDotNetRoot() },
});

csharpierProcess.on("error", data => {
Expand All @@ -43,6 +45,16 @@ export class CSharpierProcessServer implements ICSharpierProcess2 {
this.processFailedToStart = true;
});

csharpierProcess.on("exit", () => {
if (csharpierProcess.exitCode !== null && csharpierProcess.exitCode > 0) {
this.processFailedToStart = true;
}
});

csharpierProcess.stderr.on("data", data => {
this.logger.warn("CSharpier process return the following error: ", data.toString());
});

let output = "";
const regex = /^Started on (\d+)/;

Expand Down Expand Up @@ -72,7 +84,7 @@ export class CSharpierProcessServer implements ICSharpierProcess2 {
}

if (typeof this.process === "undefined") {
await new Promise(r => setTimeout(r, 2000));
await new Promise(r => setTimeout(r, 1000));
}

if (this.processFailedToStart || typeof this.process === "undefined") {
Expand Down
Loading