Merge pull request #1011 from github/fixes/1010-diff-encoding

Save PR merge base file as UTF-8 if working file is UTF-8

Author: jcansdale

src/GitHub.App/GlobalSuppressions.cs

@@ -5,6 +5,7 @@
 [assembly: SuppressMessage("Microsoft.Performance", "CA1800:DoNotCastUnnecessarily", Scope = "member", Target = "GitHub.Caches.CredentialCache.#InsertObject`1(System.String,!!0,System.Nullable`1<System.DateTimeOffset>)")]
 [assembly: SuppressMessage("Microsoft.Naming", "CA1703:ResourceStringsShouldBeSpelledCorrectly", MessageId = "Git", Scope = "resource", Target = "GitHub.App.Resources.resources")]
 [assembly: SuppressMessage("Microsoft.Globalization", "CA1305:SpecifyIFormatProvider", MessageId = "System.String.Format(System.String,System.Object,System.Object,System.Object)", Scope = "member", Target = "GitHub.Services.PullRequestService.#CreateTempFile(System.String,System.String,System.String)")]
+[assembly: SuppressMessage("Microsoft.Globalization", "CA1305:SpecifyIFormatProvider", MessageId = "System.String.Format(System.String,System.Object,System.Object,System.Object)", Scope = "member", Target = "GitHub.Services.PullRequestService.#CreateTempFile(System.String,System.String,System.String,System.Text.Encoding)")]
 // This file is used by Code Analysis to maintain SuppressMessage 
 // attributes that are applied to this project.
 // Project-level suppressions either have no target or are given 

src/GitHub.App/Services/PullRequestService.cs

@@ -299,15 +299,53 @@ public IObservable<Tuple<string, string>> ExtractDiffFiles(
                     throw new FileNotFoundException($"Couldn't find merge base between {baseSha} and {headSha}.");
                 }
 
-                // We've found the merge base so these should already be fetched.
-                var left = await ExtractToTempFile(repo, mergeBase, fileName);
-                var right = isPullRequestBranchCheckedOut ?
-                    Path.Combine(repository.LocalPath, fileName) : await ExtractToTempFile(repo, headSha, fileName);
+                string left;
+                string right;
+                if (isPullRequestBranchCheckedOut)
+                {
+                    right = Path.Combine(repository.LocalPath, fileName);
+                    left = await ExtractToTempFile(repo, mergeBase, fileName, GetEncoding(right));
+                }
+                else
+                {
+                    left = await ExtractToTempFile(repo, mergeBase, fileName, Encoding.Default);
+                    right = await ExtractToTempFile(repo, headSha, fileName, Encoding.Default);
+                }
 
                 return Observable.Return(Tuple.Create(left, right));
             });
         }
 
+        static Encoding GetEncoding(string file)
+        {
+            if (File.Exists(file))
+            {
+                var encoding = Encoding.UTF8;
+                if (IsEncoding(file, encoding))
+                {
+                    return encoding;
+                }
+            }
+
+            return Encoding.Default;
+        }
+
+        static bool IsEncoding(string file, Encoding encoding)
+        {
+            using (var stream = File.OpenRead(file))
+            {
+                foreach (var b in encoding.GetPreamble())
+                {
+                    if(b != stream.ReadByte())
+                    {
+                        return false;
+                    }
+                }
+            }
+
+            return true;
+        }
+
         public IObservable<Unit> RemoveUnusedRemotes(ILocalRepositoryModel repository)
         {
             return Observable.Defer(async () =>
@@ -362,20 +400,20 @@ string CreateUniqueRemoteName(IRepository repo, string name)
             return uniqueName;
         }
 
-        async Task<string> ExtractToTempFile(IRepository repo, string commitSha, string fileName)
+        async Task<string> ExtractToTempFile(IRepository repo, string commitSha, string fileName, Encoding encoding)
         {
             var contents = await gitClient.ExtractFile(repo, commitSha, fileName) ?? string.Empty;
-            return CreateTempFile(fileName, commitSha, contents);
+            return CreateTempFile(fileName, commitSha, contents, encoding);
         }
 
-        static string CreateTempFile(string fileName, string commitSha, string contents)
+        static string CreateTempFile(string fileName, string commitSha, string contents, Encoding encoding)
         {
             var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
             var tempFileName = $"{Path.GetFileNameWithoutExtension(fileName)}@{commitSha}{Path.GetExtension(fileName)}";
             var tempFile = Path.Combine(tempDir, tempFileName);
 
             Directory.CreateDirectory(tempDir);
-            File.WriteAllText(tempFile, contents, Encoding.UTF8);
+            File.WriteAllText(tempFile, contents, encoding);
             return tempFile;
         }
 

src/UnitTests/GitHub.App/Services/PullRequestServiceTests.cs

@@ -1,5 +1,6 @@
 using System;
 using System.IO;
+using System.Text;
 using System.Collections.Generic;
 using System.Linq;
 using System.Reactive.Linq;
@@ -128,6 +129,32 @@ public async Task CheckedOut_BaseFromWorkingFile()
             Assert.Equal(workingFile, files.Item2);
         }
 
+        // https://github.com/github/VisualStudio/issues/1010
+        [Theory]
+        [InlineData("utf-8")]        // Unicode (UTF-8)
+        [InlineData("Windows-1252")] // Western European (Windows)        
+        public async Task CheckedOut_DifferentEncodings(string encodingName)
+        {
+            var encoding = Encoding.GetEncoding(encodingName);
+            var repoDir = Path.GetTempPath();
+            var baseFileContent = "baseFileContent";
+            var headFileContent = null as string;
+            var fileName = "fileName.txt";
+            var baseSha = "baseSha";
+            var headSha = "headSha";
+            var baseRef = new GitReferenceModel("ref", "label", baseSha, "uri");
+            var checkedOut = true;
+            var workingFile = Path.Combine(repoDir, fileName);
+            var workingFileContent = baseFileContent;
+            File.WriteAllText(workingFile, workingFileContent, encoding);
+
+            var files = await ExtractDiffFiles(baseSha, baseFileContent, headSha, headFileContent,
+                baseSha, baseFileContent, fileName, checkedOut, repoDir);
+
+            Assert.Equal(File.ReadAllText(files.Item1), File.ReadAllText(files.Item2));
+            Assert.Equal(File.ReadAllBytes(files.Item1), File.ReadAllBytes(files.Item2));
+        }
+
         [Fact]
         public async Task HeadBranchNotAvailable_ThrowsFileNotFoundException()
         {