Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Commit fbf5a4f

Browse files
committed
Fix PR base diff file when checked out file is UTF-8
It appears Visual Studio likes to add a UTF-8 header to files it touches. When creating a temp file to diff against, save as UTF-8 if working file is UTF-8. Fixes #1010
1 parent 0d04613 commit fbf5a4f

File tree

2 files changed

+95
-8
lines changed

2 files changed

+95
-8
lines changed

src/GitHub.App/Services/PullRequestService.cs

+46-8
Original file line numberDiff line numberDiff line change
@@ -299,15 +299,53 @@ public IObservable<Tuple<string, string>> ExtractDiffFiles(
299299
throw new FileNotFoundException($"Couldn't find merge base between {baseSha} and {headSha}.");
300300
}
301301

302-
// We've found the merge base so these should already be fetched.
303-
var left = await ExtractToTempFile(repo, mergeBase, fileName);
304-
var right = isPullRequestBranchCheckedOut ?
305-
Path.Combine(repository.LocalPath, fileName) : await ExtractToTempFile(repo, headSha, fileName);
302+
string left;
303+
string right;
304+
if (isPullRequestBranchCheckedOut)
305+
{
306+
right = Path.Combine(repository.LocalPath, fileName);
307+
left = await ExtractToTempFile(repo, mergeBase, fileName, GetEncoding(right));
308+
}
309+
else
310+
{
311+
left = await ExtractToTempFile(repo, mergeBase, fileName, Encoding.Default);
312+
right = await ExtractToTempFile(repo, headSha, fileName, Encoding.Default);
313+
}
306314

307315
return Observable.Return(Tuple.Create(left, right));
308316
});
309317
}
310318

319+
static Encoding GetEncoding(string file)
320+
{
321+
if (File.Exists(file))
322+
{
323+
var encoding = Encoding.UTF8;
324+
if (IsEncoding(file, encoding))
325+
{
326+
return encoding;
327+
}
328+
}
329+
330+
return Encoding.Default;
331+
}
332+
333+
static bool IsEncoding(string file, Encoding encoding)
334+
{
335+
using (var stream = File.OpenRead(file))
336+
{
337+
foreach (var b in encoding.GetPreamble())
338+
{
339+
if(b != stream.ReadByte())
340+
{
341+
return false;
342+
}
343+
}
344+
}
345+
346+
return true;
347+
}
348+
311349
public IObservable<Unit> RemoveUnusedRemotes(ILocalRepositoryModel repository)
312350
{
313351
return Observable.Defer(async () =>
@@ -362,20 +400,20 @@ string CreateUniqueRemoteName(IRepository repo, string name)
362400
return uniqueName;
363401
}
364402

365-
async Task<string> ExtractToTempFile(IRepository repo, string commitSha, string fileName)
403+
async Task<string> ExtractToTempFile(IRepository repo, string commitSha, string fileName, Encoding encoding)
366404
{
367405
var contents = await gitClient.ExtractFile(repo, commitSha, fileName) ?? string.Empty;
368-
return CreateTempFile(fileName, commitSha, contents);
406+
return CreateTempFile(fileName, commitSha, contents, encoding);
369407
}
370408

371-
static string CreateTempFile(string fileName, string commitSha, string contents)
409+
static string CreateTempFile(string fileName, string commitSha, string contents, Encoding encoding)
372410
{
373411
var tempDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString());
374412
var tempFileName = $"{Path.GetFileNameWithoutExtension(fileName)}@{commitSha}{Path.GetExtension(fileName)}";
375413
var tempFile = Path.Combine(tempDir, tempFileName);
376414

377415
Directory.CreateDirectory(tempDir);
378-
File.WriteAllText(tempFile, contents, Encoding.UTF8);
416+
File.WriteAllText(tempFile, contents, encoding);
379417
return tempFile;
380418
}
381419

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

+49
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.IO;
3+
using System.Text;
34
using System.Collections.Generic;
45
using System.Linq;
56
using System.Reactive.Linq;
@@ -128,6 +129,54 @@ public async Task CheckedOut_BaseFromWorkingFile()
128129
Assert.Equal(workingFile, files.Item2);
129130
}
130131

132+
[Fact]
133+
public async Task CheckedOut_BaseFromWorkingFile_UTF8()
134+
{
135+
var repoDir = Path.GetTempPath();
136+
var baseFileContent = "baseFileContent";
137+
var headFileContent = null as string;
138+
var fileName = "fileName.txt";
139+
var baseSha = "baseSha";
140+
var headSha = "headSha";
141+
var baseRef = new GitReferenceModel("ref", "label", baseSha, "uri");
142+
var checkedOut = true;
143+
var workingFile = Path.Combine(repoDir, fileName);
144+
var workingFileContent = baseFileContent;
145+
File.WriteAllText(workingFile, workingFileContent, Encoding.UTF8);
146+
147+
var files = await ExtractDiffFiles(baseSha, baseFileContent, headSha, headFileContent, baseSha, baseFileContent,
148+
fileName, checkedOut, repoDir);
149+
150+
Assert.Equal(File.ReadAllText(files.Item1), File.ReadAllText(files.Item2));
151+
Assert.Equal(File.ReadAllBytes(files.Item1), File.ReadAllBytes(files.Item2));
152+
}
153+
154+
// https://github.com/github/VisualStudio/issues/1010
155+
[Theory]
156+
[InlineData("utf-8")] // Unicode (UTF-8)
157+
[InlineData("Windows-1252")] // Western European (Windows)
158+
public async Task CheckedOut_DifferentEncodings(string encodingName)
159+
{
160+
var encoding = Encoding.GetEncoding(encodingName);
161+
var repoDir = Path.GetTempPath();
162+
var baseFileContent = "baseFileContent";
163+
var headFileContent = null as string;
164+
var fileName = "fileName.txt";
165+
var baseSha = "baseSha";
166+
var headSha = "headSha";
167+
var baseRef = new GitReferenceModel("ref", "label", baseSha, "uri");
168+
var checkedOut = true;
169+
var workingFile = Path.Combine(repoDir, fileName);
170+
var workingFileContent = baseFileContent;
171+
File.WriteAllText(workingFile, workingFileContent, encoding);
172+
173+
var files = await ExtractDiffFiles(baseSha, baseFileContent, headSha, headFileContent,
174+
baseSha, baseFileContent, fileName, checkedOut, repoDir);
175+
176+
Assert.Equal(File.ReadAllText(files.Item1), File.ReadAllText(files.Item2));
177+
Assert.Equal(File.ReadAllBytes(files.Item1), File.ReadAllBytes(files.Item2));
178+
}
179+
131180
[Fact]
132181
public async Task HeadBranchNotAvailable_ThrowsFileNotFoundException()
133182
{

0 commit comments

Comments
 (0)