diff --git a/src/GitHub.App/GlobalSuppressions.cs b/src/GitHub.App/GlobalSuppressions.cs index a08222f3b2..f635f8ea1d 100644 --- a/src/GitHub.App/GlobalSuppressions.cs +++ b/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)")] [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 diff --git a/src/GitHub.App/Services/PullRequestService.cs b/src/GitHub.App/Services/PullRequestService.cs index adecfcc853..6b8b1ee1e0 100644 --- a/src/GitHub.App/Services/PullRequestService.cs +++ b/src/GitHub.App/Services/PullRequestService.cs @@ -299,15 +299,53 @@ public IObservable> 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 RemoveUnusedRemotes(ILocalRepositoryModel repository) { return Observable.Defer(async () => @@ -362,20 +400,20 @@ string CreateUniqueRemoteName(IRepository repo, string name) return uniqueName; } - async Task ExtractToTempFile(IRepository repo, string commitSha, string fileName) + async Task 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; } diff --git a/src/UnitTests/GitHub.App/Services/PullRequestServiceTests.cs b/src/UnitTests/GitHub.App/Services/PullRequestServiceTests.cs index 1e7ea187dc..47813703ca 100644 --- a/src/UnitTests/GitHub.App/Services/PullRequestServiceTests.cs +++ b/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() {