Skip to content

Commit

Permalink
Fixing issue with unicode characters when piping standard in. (#550)
Browse files Browse the repository at this point in the history
* 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
  • Loading branch information
belav authored Jan 22, 2022
1 parent 1b1f451 commit 8a1dfc7
Show file tree
Hide file tree
Showing 12 changed files with 91 additions and 57 deletions.
27 changes: 23 additions & 4 deletions Src/CSharpier.Cli.Tests/CliTests.cs
Original file line number Diff line number Diff line change
@@ -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;

Expand Down Expand Up @@ -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()
{
Expand Down Expand Up @@ -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");
Expand All @@ -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)
Expand All @@ -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<ProcessResult> ExecuteAsync()
{
var result = await this.command.ExecuteAsync();
var result = await this.command.ExecuteBufferedAsync(this.encoding);
return new ProcessResult(
this.output.ToString(),
this.errorOutput.ToString(),
Expand Down
7 changes: 6 additions & 1 deletion Src/CSharpier.Cli/IConsole.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -45,5 +50,5 @@ public void ResetColor()
Console.ResetColor();
}

public Encoding InputEncoding => Console.InputEncoding;
public Encoding InputEncoding => Encoding.UTF8;
}
6 changes: 3 additions & 3 deletions Src/CSharpier.Cli/Program.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
using System.CommandLine;
using System.CommandLine;
using System.CommandLine.Invocation;
using System.Diagnostics;
using System.IO.Abstractions;
Expand Down Expand Up @@ -44,7 +44,7 @@ CancellationToken cancellationToken
{
using var streamReader = new StreamReader(
Console.OpenStandardInput(),
Console.InputEncoding
console.InputEncoding
);
standardInFileContents = await streamReader.ReadToEndAsync();

Expand Down Expand Up @@ -90,7 +90,7 @@ CancellationToken cancellationToken
{
using var streamReader = new StreamReader(
Console.OpenStandardInput(),
Console.InputEncoding
console.InputEncoding
);

var stringBuilder = new StringBuilder();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,26 +6,30 @@
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 {
Logger LOG = Logger.getInstance(CSharpierProcessPipeMultipleFiles.class);
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);
}
Expand All @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}
Expand Down Expand Up @@ -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")
Expand All @@ -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);
Expand Down
1 change: 0 additions & 1 deletion Src/CSharpier.VisualStudio/CSharpier.VisualStudio.sln
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ IProgress<ServiceProgressData> progress
{
var outputPane = await this.GetServiceAsync<IVsOutputWindow>();
var logger = new Logger(outputPane);
logger.Log("Starting");
logger.Info("Starting");

await this.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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");
}
Expand All @@ -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);
Expand All @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
Expand All @@ -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)
{
Expand All @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.IO;
using Microsoft.VisualStudio.Shell;
using Microsoft.VisualStudio.Shell.Interop;

Expand All @@ -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);
}
Expand Down

0 comments on commit 8a1dfc7

Please sign in to comment.