From 8a1dfc78867fc58ec1cc8238d7ac366c4db126a5 Mon Sep 17 00:00:00 2001 From: Bela VanderVoort Date: Sat, 22 Jan 2022 09:44:14 -0600 Subject: [PATCH] Fixing issue with unicode characters when piping standard in. (#550) * Adding documentation for husky.net + implement it. Clean up the solution a bit with notarget projects. * self code review * More self code review * rename file * Make sure the pre-commit hook is opt in * Update task-runner.json * Narrowing down where the unicode encoding issue is for #545 * Adding some notes and logging. * Getting UTF8 working on vs and vscode * Cleanup plus getting rider working with utf8 * some cleanup * Self code review * Update CliTests.cs --- Src/CSharpier.Cli.Tests/CliTests.cs | 27 ++++++++++++++++--- Src/CSharpier.Cli/IConsole.cs | 7 ++++- Src/CSharpier.Cli/Program.cs | 6 ++--- .../CSharpierProcessPipeMultipleFiles.java | 18 ++++++++----- .../intellij/csharpier/CSharpierService.java | 20 +++++--------- .../CSharpier.VisualStudio.sln | 1 - .../CSharpierPackage.cs | 2 +- .../CSharpierProcessPipeMultipleFiles.cs | 24 +++++++++++------ .../CSharpierProcessSingleFile.cs | 4 +-- .../CSharpierService.cs | 21 +++++---------- .../FormattingService.cs | 2 +- .../CSharpier.VisualStudioShared/Logger.cs | 16 ++++++++++- 12 files changed, 91 insertions(+), 57 deletions(-) diff --git a/Src/CSharpier.Cli.Tests/CliTests.cs b/Src/CSharpier.Cli.Tests/CliTests.cs index 3064afd2f..fede92660 100644 --- a/Src/CSharpier.Cli.Tests/CliTests.cs +++ b/Src/CSharpier.Cli.Tests/CliTests.cs @@ -1,8 +1,10 @@ using System; +using System.Globalization; using System.IO; using System.Text; using System.Threading.Tasks; using CliWrap; +using CliWrap.Buffered; using FluentAssertions; using NUnit.Framework; @@ -100,6 +102,19 @@ public async Task Should_Format_Piped_File(string lineEnding) result.ExitCode.Should().Be(0); } + [Test] + public async Task Should_Format_Unicode() + { + // use the \u so that we don't accidentally reformat this to be '?' + var unicodeContent = $"var test = '{'\u3002'}';\n"; + + var result = await new CsharpierProcess().WithPipedInput(unicodeContent).ExecuteAsync(); + + result.ErrorOutput.Should().BeEmpty(); + result.Output.Should().Be(unicodeContent); + result.ExitCode.Should().Be(0); + } + [Test] public async Task Should_Print_NotFound() { @@ -261,6 +276,8 @@ private class CsharpierProcess private readonly StringBuilder errorOutput = new(); private Command command; + private readonly Encoding encoding = Encoding.UTF8; + public CsharpierProcess() { var path = Path.Combine(Directory.GetCurrentDirectory(), "dotnet-csharpier.dll"); @@ -270,8 +287,8 @@ public CsharpierProcess() .WithArguments(path) .WithWorkingDirectory(testFileDirectory) .WithValidation(CommandResultValidation.None) - .WithStandardOutputPipe(PipeTarget.ToStringBuilder(this.output)) - .WithStandardErrorPipe(PipeTarget.ToStringBuilder(this.errorOutput)); + .WithStandardOutputPipe(PipeTarget.ToStringBuilder(this.output, this.encoding)) + .WithStandardErrorPipe(PipeTarget.ToStringBuilder(this.errorOutput, this.encoding)); } public CsharpierProcess WithArguments(string arguments) @@ -282,14 +299,16 @@ public CsharpierProcess WithArguments(string arguments) public CsharpierProcess WithPipedInput(string input) { - this.command = this.command.WithStandardInputPipe(PipeSource.FromString(input)); + this.command = this.command.WithStandardInputPipe( + PipeSource.FromString(input, this.encoding) + ); return this; } public async Task ExecuteAsync() { - var result = await this.command.ExecuteAsync(); + var result = await this.command.ExecuteBufferedAsync(this.encoding); return new ProcessResult( this.output.ToString(), this.errorOutput.ToString(), diff --git a/Src/CSharpier.Cli/IConsole.cs b/Src/CSharpier.Cli/IConsole.cs index acf31b44d..056c50eae 100644 --- a/Src/CSharpier.Cli/IConsole.cs +++ b/Src/CSharpier.Cli/IConsole.cs @@ -15,6 +15,11 @@ public interface IConsole public class SystemConsole : IConsole { + public SystemConsole() + { + Console.OutputEncoding = Encoding.UTF8; + } + public void WriteLine(string? line = null) { Console.WriteLine(line); @@ -45,5 +50,5 @@ public void ResetColor() Console.ResetColor(); } - public Encoding InputEncoding => Console.InputEncoding; + public Encoding InputEncoding => Encoding.UTF8; } diff --git a/Src/CSharpier.Cli/Program.cs b/Src/CSharpier.Cli/Program.cs index 1263a0472..5ab51998b 100644 --- a/Src/CSharpier.Cli/Program.cs +++ b/Src/CSharpier.Cli/Program.cs @@ -1,4 +1,4 @@ -using System.CommandLine; +using System.CommandLine; using System.CommandLine.Invocation; using System.Diagnostics; using System.IO.Abstractions; @@ -44,7 +44,7 @@ CancellationToken cancellationToken { using var streamReader = new StreamReader( Console.OpenStandardInput(), - Console.InputEncoding + console.InputEncoding ); standardInFileContents = await streamReader.ReadToEndAsync(); @@ -90,7 +90,7 @@ CancellationToken cancellationToken { using var streamReader = new StreamReader( Console.OpenStandardInput(), - Console.InputEncoding + console.InputEncoding ); var stringBuilder = new StringBuilder(); diff --git a/Src/CSharpier.Rider/src/main/java/com/intellij/csharpier/CSharpierProcessPipeMultipleFiles.java b/Src/CSharpier.Rider/src/main/java/com/intellij/csharpier/CSharpierProcessPipeMultipleFiles.java index 9f60e8e03..e35567cd0 100644 --- a/Src/CSharpier.Rider/src/main/java/com/intellij/csharpier/CSharpierProcessPipeMultipleFiles.java +++ b/Src/CSharpier.Rider/src/main/java/com/intellij/csharpier/CSharpierProcessPipeMultipleFiles.java @@ -6,6 +6,8 @@ import java.io.BufferedReader; import java.io.InputStreamReader; import java.io.OutputStream; +import java.io.OutputStreamWriter; +import java.nio.charset.Charset; import java.util.concurrent.atomic.AtomicBoolean; public class CSharpierProcessPipeMultipleFiles implements ICSharpierProcess, Disposable { @@ -13,19 +15,21 @@ public class CSharpierProcessPipeMultipleFiles implements ICSharpierProcess, Dis String csharpierPath; Process process = null; - OutputStream stdin; + OutputStreamWriter stdin; BufferedReader stdOut; BufferedReader stdError; - public CSharpierProcessPipeMultipleFiles(String csharpierPath) { + public CSharpierProcessPipeMultipleFiles(String csharpierPath, boolean useUtf8) { this.csharpierPath = csharpierPath; try { process = new ProcessBuilder("dotnet", csharpierPath, "--pipe-multiple-files") .start(); - stdin = process.getOutputStream(); - stdOut = new BufferedReader(new InputStreamReader(process.getInputStream())); - stdError = new BufferedReader(new InputStreamReader(process.getErrorStream())); + String charset = useUtf8 ? "utf-8" : Charset.defaultCharset().toString(); + + stdin = new OutputStreamWriter(process.getOutputStream(), charset); + stdOut = new BufferedReader(new InputStreamReader(process.getInputStream(), charset)); + stdError = new BufferedReader(new InputStreamReader(process.getErrorStream(), charset)); } catch (Exception e) { LOG.error("error", e); } @@ -39,9 +43,9 @@ public String formatFile(String content, String filePath) { try { LOG.info(filePath); - stdin.write(filePath.getBytes()); + stdin.write(filePath); stdin.write('\u0003'); - stdin.write(content.getBytes()); + stdin.write(content); stdin.write('\u0003'); stdin.flush(); diff --git a/Src/CSharpier.Rider/src/main/java/com/intellij/csharpier/CSharpierService.java b/Src/CSharpier.Rider/src/main/java/com/intellij/csharpier/CSharpierService.java index 8564aa5af..3350a8d8a 100644 --- a/Src/CSharpier.Rider/src/main/java/com/intellij/csharpier/CSharpierService.java +++ b/Src/CSharpier.Rider/src/main/java/com/intellij/csharpier/CSharpierService.java @@ -32,18 +32,7 @@ public CSharpierService(@NotNull Project project) { public String getCSharpierPath() { // TODO make this some kind of build property so it only works when testing the plugin // or maybe make it a setting? -// try { -// String csharpierDebugPath = "C:\\projects\\csharpier\\Src\\CSharpier.Cli\\bin\\Debug\\net6.0\\dotnet-csharpier.dll"; -// String csharpierReleasePath = csharpierDebugPath.replace("Debug", "Release"); -// -// if (new File(csharpierDebugPath).exists()) { -// return csharpierDebugPath; -// } else if (new File(csharpierReleasePath).exists()) { -// return csharpierReleasePath; -// } -// } catch (Exception ex) { -// Log.debug("Could not find local csharpier " + ex.getMessage()); -// } + // return "C:\\projects\\csharpier\\Src\\CSharpier.Cli\\bin\\Debug\\net6.0\\dotnet-csharpier.dll"; return "csharpier"; } @@ -76,6 +65,7 @@ private ICSharpierProcess setupCSharpierProcess() { else { ComparableVersion installedVersion = new ComparableVersion(version); ComparableVersion pipeFilesVersion = new ComparableVersion("0.12.0"); + ComparableVersion utf8Version = new ComparableVersion("0.14.0"); if (installedVersion.compareTo(pipeFilesVersion) < 0) { String content = "Please upgrade to CSharpier >= 0.12.0 for bug fixes and improved formatting speed."; NotificationGroupManager.getInstance().getNotificationGroup("CSharpier") @@ -84,7 +74,11 @@ private ICSharpierProcess setupCSharpierProcess() { return new CSharpierProcessSingleFile(this.csharpierPath); } - return new CSharpierProcessPipeMultipleFiles(this.csharpierPath); + + boolean useUtf8 = installedVersion.compareTo(utf8Version) >= 0; + + return new CSharpierProcessPipeMultipleFiles(this.csharpierPath, useUtf8); + } } catch (Exception ex) { LOG.error(ex); diff --git a/Src/CSharpier.VisualStudio/CSharpier.VisualStudio.sln b/Src/CSharpier.VisualStudio/CSharpier.VisualStudio.sln index 96fb78d78..ddefca16d 100644 --- a/Src/CSharpier.VisualStudio/CSharpier.VisualStudio.sln +++ b/Src/CSharpier.VisualStudio/CSharpier.VisualStudio.sln @@ -31,7 +31,6 @@ Global {3F1D185A-BBA4-45AA-8B12-23060E529C73}.Release|x86.ActiveCfg = Release|Any CPU {3F1D185A-BBA4-45AA-8B12-23060E529C73}.Release|x86.Build.0 = Release|Any CPU {72BB9FC2-30F0-479F-850F-D67ACA557E9E}.Debug|Any CPU.ActiveCfg = Debug|Any CPU - {72BB9FC2-30F0-479F-850F-D67ACA557E9E}.Debug|Any CPU.Build.0 = Debug|Any CPU {72BB9FC2-30F0-479F-850F-D67ACA557E9E}.Debug|x86.ActiveCfg = Debug|x86 {72BB9FC2-30F0-479F-850F-D67ACA557E9E}.Debug|x86.Build.0 = Debug|x86 {72BB9FC2-30F0-479F-850F-D67ACA557E9E}.Release|Any CPU.ActiveCfg = Release|Any CPU diff --git a/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierPackage.cs b/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierPackage.cs index 6278256aa..d8720d076 100644 --- a/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierPackage.cs +++ b/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierPackage.cs @@ -23,7 +23,7 @@ IProgress progress { var outputPane = await this.GetServiceAsync(); var logger = new Logger(outputPane); - logger.Log("Starting"); + logger.Info("Starting"); await this.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken); diff --git a/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierProcessPipeMultipleFiles.cs b/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierProcessPipeMultipleFiles.cs index 9f4a0d5d6..5a3bc26ee 100644 --- a/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierProcessPipeMultipleFiles.cs +++ b/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierProcessPipeMultipleFiles.cs @@ -15,6 +15,7 @@ public class CSharpierProcessPipeMultipleFiles : ICSharpierProcess private readonly AutoResetEvent autoEvent = new AutoResetEvent(false); private readonly StringBuilder output = new StringBuilder(); private readonly StringBuilder errorOutput = new StringBuilder(); + private readonly StreamWriter standardIn; public CSharpierProcessPipeMultipleFiles(string csharpierPath, Logger logger) { @@ -28,11 +29,17 @@ public CSharpierProcessPipeMultipleFiles(string csharpierPath, Logger logger) RedirectStandardInput = true, RedirectStandardOutput = true, RedirectStandardError = true, + StandardOutputEncoding = Encoding.UTF8, + StandardErrorEncoding = Encoding.UTF8, UseShellExecute = false, CreateNoWindow = true }; this.process = new Process { StartInfo = processStartInfo }; this.process.Start(); + this.standardIn = new StreamWriter( + this.process.StandardInput.BaseStream, + Encoding.UTF8 + ); this.FormatFile("public class ClassName { }", "Test.cs"); } @@ -44,13 +51,14 @@ public string FormatFile(string content, string filePath) this.output.Clear(); this.errorOutput.Clear(); - this.logger.Log("Formatting " + filePath); + this.logger.Info("Formatting " + filePath); var stopwatch = Stopwatch.StartNew(); - this.process.StandardInput.Write(filePath); - this.process.StandardInput.Write('\u0003'); - this.process.StandardInput.Write(content); - this.process.StandardInput.Write('\u0003'); + this.standardIn.Write(filePath); + this.standardIn.Write('\u0003'); + this.standardIn.Write(content); + this.standardIn.Write('\u0003'); + this.standardIn.Flush(); ThreadPool.QueueUserWorkItem(this.ReadOutput, this.autoEvent); ThreadPool.QueueUserWorkItem(this.ReadError, this.autoEvent); @@ -63,17 +71,17 @@ public string FormatFile(string content, string filePath) { if (string.IsNullOrEmpty(result)) { - this.logger.Log("File is ignored by .csharpierignore"); + this.logger.Info("File is ignored by .csharpierignore"); return null; } else { - this.logger.Log("Formatted in " + stopwatch.ElapsedMilliseconds + "ms"); + this.logger.Info("Formatted in " + stopwatch.ElapsedMilliseconds + "ms"); return this.output.ToString(); } } - this.logger.Log("Got error output: " + errorResult); + this.logger.Info("Got error output: " + errorResult); return null; } diff --git a/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierProcessSingleFile.cs b/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierProcessSingleFile.cs index a88db40e1..bd5ef9d74 100644 --- a/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierProcessSingleFile.cs +++ b/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierProcessSingleFile.cs @@ -49,8 +49,8 @@ public string FormatFile(string content, string fileName) return result; } - this.logger.Log(errorOutput.ToString()); - this.logger.Log(result); + this.logger.Info(errorOutput.ToString()); + this.logger.Info(result); return null; } diff --git a/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierService.cs b/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierService.cs index 347c8a50a..882c742dd 100644 --- a/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierService.cs +++ b/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/CSharpierService.cs @@ -16,7 +16,7 @@ public CSharpierService(Logger logger) this.csharpierPath = this.GetCSharpierPath(); - logger.Log("Using command dotnet " + this.csharpierPath); + logger.Debug("Using command dotnet " + this.csharpierPath); this.csharpierProcess = this.SetupCSharpierProcess(); } @@ -25,23 +25,14 @@ public string GetCSharpierPath() { // TODO make this some kind of build property so it only works when testing the plugin // or maybe make it a setting? - // try { - // String csharpierDebugPath = "C:\\projects\\csharpier\\Src\\CSharpier.Cli\\bin\\Debug\\net6.0\\dotnet-csharpier.dll"; - // String csharpierReleasePath = csharpierDebugPath.replace("Debug", "Release"); - // - // if (new File(csharpierDebugPath).exists()) { - // return csharpierDebugPath; - // } else if (new File(csharpierReleasePath).exists()) { - // return csharpierReleasePath; - // } - // } catch (Exception ex) { - // Log.debug("Could not find local csharpier " + ex.getMessage()); - // } +#if DEBUG + // return @"C:\projects\csharpier\Src\CSharpier.Cli\bin\Debug\net6.0\dotnet-csharpier.dll"; +#endif return "csharpier"; } - public string ExecuteCommand(string cmd, string arguments) + private string ExecuteCommand(string cmd, string arguments) { var processStartInfo = new ProcessStartInfo(cmd, arguments) { @@ -68,7 +59,7 @@ private ICSharpierProcess SetupCSharpierProcess() try { var version = this.ExecuteCommand("dotnet", this.csharpierPath + " --version"); - this.logger.Log("CSharpier version: " + version); + this.logger.Info("CSharpier version: " + version); if (string.IsNullOrEmpty(version)) { this.DisplayInstallNeededMessage(); diff --git a/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/FormattingService.cs b/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/FormattingService.cs index fe305576a..d6f8a0b4b 100644 --- a/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/FormattingService.cs +++ b/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/FormattingService.cs @@ -26,7 +26,7 @@ public void Format(Document document) if (!(document.Object("TextDocument") is TextDocument textDocument)) { - this.logger.Log("There was no TextDocument for the current Document"); + this.logger.Info("There was no TextDocument for the current Document"); return; } diff --git a/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/Logger.cs b/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/Logger.cs index 0180346ab..7e8c7edb8 100644 --- a/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/Logger.cs +++ b/Src/CSharpier.VisualStudio/CSharpier.VisualStudioShared/Logger.cs @@ -1,4 +1,5 @@ using System; +using System.IO; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Shell.Interop; @@ -16,7 +17,20 @@ public Logger(IVsOutputWindow outputWindow) outputWindow.GetPane(ref guid, out this.pane); } - public void Log(string message) + // TODO make a proper setting for this so real users can see debug messages + public void Debug(string message) + { +#if DEBUG + this.Log(message); +#endif + } + + public void Info(string message) + { + this.Log(message); + } + + private void Log(string message) { this.pane.OutputStringThreadSafe(message + Environment.NewLine); }