Fix garbled characters in history view with non-ascii characters (fixes#124)

src/GitHub.Api/Git/Tasks/GitCommitTask.cs

@@ -15,7 +15,7 @@ public GitCommitTask(string message, string body,
             Guard.ArgumentNotNullOrWhiteSpace(message, "message");
 
             Name = TaskName;
-            arguments = "commit ";
+            arguments = "-c i18n.commitencoding=utf8 commit ";
             arguments += String.Format(" -m \"{0}\"", message);
             if (!String.IsNullOrEmpty(body))
                 arguments += String.Format(" -m \"{0}\"", body);

src/GitHub.Api/Git/Tasks/GitLogTask.cs

@@ -15,7 +15,7 @@ public GitLogTask(IGitObjectFactory gitObjectFactory,
 
         public override string ProcessArguments
         {
-            get { return @"log --pretty=format:""%H%n%P%n%aN%n%aE%n%aI%n%cN%n%cE%n%cI%n%B---GHUBODYEND---"" --name-status"; }
+            get { return @"-c i18n.logoutputencoding=utf8 -c core.quotepath=false log --pretty=format:""%H%n%P%n%aN%n%aE%n%aI%n%cN%n%cE%n%cI%n%B---GHUBODYEND---"" --name-status"; }
         }
     }
 }

src/GitHub.Api/Git/Tasks/GitStatusTask.cs

@@ -15,7 +15,7 @@ public GitStatusTask(IGitObjectFactory gitObjectFactory,
 
         public override string ProcessArguments
         {
-            get { return "status -b -u --ignored --porcelain"; }
+            get { return "-c i18n.logoutputencoding=utf8 -c core.quotepath=false status -b -u --ignored --porcelain"; }
         }
         public override TaskAffinity Affinity { get { return TaskAffinity.Exclusive; } }
     }

src/GitHub.Api/GitHub.Api.csproj

@@ -108,6 +108,7 @@
     <Compile Include="Application\ApplicationManagerBase.cs" />
     <Compile Include="Helpers\Constants.cs" />
     <Compile Include="Cache\IBranchCache.cs" />
+    <Compile Include="Helpers\NewlineSplitStringBuilder.cs" />
     <Compile Include="Helpers\Validation.cs" />
     <Compile Include="Platform\DefaultEnvironment.cs" />
     <Compile Include="Extensions\EnvironmentExtensions.cs" />

src/GitHub.Api/Helpers/NewlineSplitStringBuilder.cs

@@ -0,0 +1,57 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+
+namespace GitHub.Unity.Helpers
+{
+    class NewlineSplitStringBuilder
+    {
+        private StringBuilder builder;
+
+        public string[] Append(string value)
+        {
+            if (value == null)
+            {
+                var singleResult = builder?.ToString();
+                if (singleResult == null)
+                {
+                    return new string[0];
+                }
+                return new[] { singleResult };
+            }
+
+            var splitValues = value.Split(new[] { "\r\n", "\n" }, StringSplitOptions.None);
+
+            if (splitValues.Length == 0)
+            {
+                throw new ArgumentOutOfRangeException();
+            }
+
+            if (builder == null)
+            {
+                builder = new StringBuilder();
+            }
+
+            builder.Append(splitValues[0]);
+
+            if (splitValues.Length == 1)
+            {
+                return new string[0];
+            }
+
+            var results = new string[splitValues.Length - 1];
+            results[0] = builder.ToString();
+
+            for (var index = 1; index < splitValues.Length - 1; index++)
+            {
+                results[index] = splitValues[index];
+            }
+
+            builder = new StringBuilder(splitValues[splitValues.Length - 1]);
+
+            return results;
+        }
+    }
+}
+

src/GitHub.Api/NewTaskSystem/ProcessTask.cs

@@ -3,9 +3,11 @@
 using System.ComponentModel;
 using System.Diagnostics;
 using System.IO;
+using System.Linq;
 using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
+using GitHub.Unity.Helpers;
 
 namespace GitHub.Unity
 {
@@ -56,7 +58,6 @@ class ProcessWrapper
         private readonly Action onEnd;
         private readonly Action<Exception, string> onError;
         private readonly CancellationToken token;
-        private readonly List<string> errors = new List<string>();
 
         public Process Process { get; }
         public StreamWriter Input { get; private set; }
@@ -78,39 +79,6 @@ public ProcessWrapper(Process process, IOutputProcessor outputProcessor,
 
         public void Run()
         {
-            if (Process.StartInfo.RedirectStandardOutput)
-            {
-                Process.OutputDataReceived += (s, e) =>
-                {
-                    //Logger.Trace("OutputData \"" + (e.Data == null ? "'null'" : e.Data) + "\"");
-
-                    string encodedData = null;
-                    if (e.Data != null)
-                    {
-                        encodedData = Encoding.UTF8.GetString(Encoding.Default.GetBytes(e.Data));
-                    }
-                    outputProcessor.LineReceived(encodedData);
-                };
-            }
-
-            if (Process.StartInfo.RedirectStandardError)
-            {
-                Process.ErrorDataReceived += (s, e) =>
-                {
-                    //if (e.Data != null)
-                    //{
-                    //    Logger.Trace("ErrorData \"" + (e.Data == null ? "'null'" : e.Data) + "\"");
-                    //}
-
-                    string encodedData = null;
-                    if (e.Data != null)
-                    {
-                        encodedData = Encoding.UTF8.GetString(Encoding.Default.GetBytes(e.Data));
-                        errors.Add(encodedData);
-                    }
-                };
-            }
-
             try
             {
                 Process.Start();
@@ -128,54 +96,103 @@ public void Run()
                     sb.AppendFormat("{0}:{1}", env, Process.StartInfo.EnvironmentVariables[env]);
                     sb.AppendLine();
                 }
-                onError?.Invoke(ex, String.Format("{0} {1}", ex.Message, sb.ToString()));
+                onError?.Invoke(ex, $"{ex.Message} {sb}");
                 onEnd?.Invoke();
                 return;
             }
 
-            if (Process.StartInfo.RedirectStandardOutput)
-                Process.BeginOutputReadLine();
-            if (Process.StartInfo.RedirectStandardError)
-                Process.BeginErrorReadLine();
             if (Process.StartInfo.RedirectStandardInput)
                 Input = new StreamWriter(Process.StandardInput.BaseStream, new UTF8Encoding(false));
 
             onStart?.Invoke();
-
             if (Process.StartInfo.CreateNoWindow)
             {
-                while (!WaitForExit(500))
+                // buffer size refers to https://github.com/Unity-Technologies/mono/blob/unity-5.6-staging/mcs/class/System/System.Diagnostics/Process.cs#L1149-L1157
+                const int bufferSize = 8182;
+
+                if (Process.StartInfo.RedirectStandardOutput)
                 {
-                    if (token.IsCancellationRequested)
+                    var outputStream = Process.StandardOutput.BaseStream;
+                    var outputBuffer = new byte[bufferSize];
+                    var outputEncoding = Process.StartInfo.StandardOutputEncoding;
+                    var splitStringbuilder = new NewlineSplitStringBuilder();
+                    string[] splitLines = null;
+
+                    var bytesRead = outputStream.Read(outputBuffer, 0, bufferSize);
+                    while (bytesRead > 0)
                     {
-                        if (!Process.HasExited)
-                            Process.Kill();
-                        Process.Close();
-                        onEnd?.Invoke();
-                        token.ThrowIfCancellationRequested();
+                        var encoded = outputEncoding.GetString(outputBuffer, 0, bytesRead);
+                        splitLines = splitStringbuilder.Append(encoded);
+                        foreach (var splitLine in splitLines)
+                        {
+                            outputProcessor.LineReceived(splitLine);
+                        }
+
+                        if (token.IsCancellationRequested)
+                        {
+                            if (!Process.HasExited)
+                                Process.Kill();
+
+                            Process.Close();
+                            onEnd?.Invoke();
+                            token.ThrowIfCancellationRequested();
+                        }
+
+                        bytesRead = outputStream.Read(outputBuffer, 0, bufferSize);
                     }
+
+                    splitLines = splitStringbuilder.Append(null);
+                    //All but the last line, which will always be empty
+                    for (var index = 0; index < splitLines.Length - 1; index++)
+                    {
+                        var splitLine = splitLines[index];
+                        outputProcessor.LineReceived(splitLine);
+                    }
+
+                    outputProcessor.LineReceived(null);
                 }
 
-                if (Process.ExitCode != 0 && errors.Count > 0)
+                if (Process.StartInfo.RedirectStandardError)
                 {
-                    onError?.Invoke(null, String.Join(Environment.NewLine, errors.ToArray()));
-                }
-            }
-            onEnd?.Invoke();
-        }
+                    var errorStream = Process.StandardError.BaseStream;
+                    var errorBuffer = new byte[bufferSize];
+                    var errorEncoding = Process.StartInfo.StandardErrorEncoding;
+                    var errorStringBuilder = new StringBuilder();
 
-        private bool WaitForExit(int milliseconds)
-        {
-            //Logger.Debug("WaitForExit - time: {0}ms", milliseconds);
+                    var bytesRead = errorStream.Read(errorBuffer, 0, bufferSize);
+                    while (bytesRead > 0)
+                    {
+                        var encoded = errorEncoding.GetString(errorBuffer, 0, bytesRead);
+                        errorStringBuilder.Append(encoded);
 
-            // Workaround for a bug in which some data may still be processed AFTER this method returns true, thus losing the data.
-            // http://connect.microsoft.com/VisualStudio/feedback/details/272125/waitforexit-and-waitforexit-int32-provide-different-and-undocumented-implementations
-            bool waitSucceeded = Process.WaitForExit(milliseconds);
-            if (waitSucceeded)
-            {
-                Process.WaitForExit();
+                        if (token.IsCancellationRequested)
+                        {
+                            if (!Process.HasExited)
+                                Process.Kill();
+
+                            Process.Close();
+                            onEnd?.Invoke();
+                            token.ThrowIfCancellationRequested();
+                        }
+
+                        bytesRead = errorStream.Read(errorBuffer, 0, bufferSize);
+                    }
+
+                    var errors = errorStringBuilder.ToString().Split(new[] { "\r\n", "\n" }, StringSplitOptions.None).ToArray();
+                    if (errors.Length > 1)
+                    {
+                        //All but the last line, which will always be empty
+                        errors = errors.Take(errors.Length - 1).ToArray();
+                    }
+
+                    if (Process.ExitCode != 0 && errors.Length > 0)
+                    {
+                        onError?.Invoke(null, string.Join(Environment.NewLine, errors));
+                    }
+                }
             }
-            return waitSucceeded;
+
+            onEnd?.Invoke();
         }
     }
 

src/tests/IntegrationTests/Process/ProcessManagerIntegrationTests.cs

@@ -81,7 +81,7 @@ public async Task LogEntriesTest()
             });
         }
 
-        [Test, Category("DoNotRunOnAppVeyor")]
+        [Test]
         public async Task RussianLogEntriesTest()
         {
             await Initialize(TestRepoMasterCleanUnsynchronizedRussianLanguage);

src/tests/UnitTests/Helpers/NewlineSplitStringBuilderTests.cs

@@ -0,0 +1,40 @@
+using System;
+using System.Collections.Generic;
+using FluentAssertions;
+using GitHub.Unity.Helpers;
+using NUnit.Framework;
+
+namespace UnitTests
+{
+    [TestFixture]
+    public class NewlineSplitStringBuilderTests
+    {
+        public static TestCaseData[] GetNewlineSplitTestData()
+        {
+            return new[] {
+                new TestCaseData(new string[] {null}, new string[0]).SetName("Null returns nothing"),
+                new TestCaseData(new string[] { $"ASDF{Environment.NewLine}Hello", null}, new[] {"ASDF", "Hello"}).SetName("Can split whole string"),
+                new TestCaseData(new string[] { $"ASDF", $"{Environment.NewLine}Hello", null}, new[] {"ASDF", "Hello"}).SetName("Can split string with second beginning newline"),
+                new TestCaseData(new string[] { $"ASDF{Environment.NewLine}", "Hello", null}, new[] {"ASDF", "Hello"}).SetName("Can split string with first ending newline"),
+                new TestCaseData(new string[] { $"AS", $"DF{Environment.NewLine}Hello", null}, new[] {"ASDF", "Hello"}).SetName("Can split string with newline contained in second"),
+            };
+        }
+
+        [TestCaseSource("GetNewlineSplitTestData")]
+        public void NewlineSplitStringBuilderTest(string[] expectedInputs, string[] expectedOutputs)
+        {
+            var results = new List<string>();
+            var newlineSplitStringBuilder = new NewlineSplitStringBuilder();
+            foreach (var expectedInput in expectedInputs)
+            {
+                var output = newlineSplitStringBuilder.Append(expectedInput);
+                if (output != null)
+                {
+                    results.AddRange(output);
+                }
+            }
+
+            results.ShouldAllBeEquivalentTo(expectedOutputs);
+        }
+    }
+}

src/tests/UnitTests/UnitTests.csproj

@@ -80,6 +80,7 @@
     <Compile Include="Extensions\ListExtensionTests.cs" />
     <Compile Include="Git\ValidationTests.cs" />
     <Compile Include="Git\GitConfigTests.cs" />
+    <Compile Include="Helpers\NewlineSplitStringBuilderTests.cs" />
     <Compile Include="IO\BranchListOutputProcessorTests.cs" />
     <Compile Include="Extensions\EnvironmentExtensionTests.cs" />
     <Compile Include="IO\LockOutputProcessorTests.cs" />