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

Commit 6b81f3f

Browse files
grokysgrokys
committed
Handle BOM in files.
We need to add explicit handling for the BOM in text files otherwise the first line of the file will erroniously be signalled as changed.
1 parent 7f76c47 commit 6b81f3f

File tree

6 files changed

+80
-64
lines changed

6 files changed

+80
-64
lines changed

‎src/GitHub.App/Services/GitClient.cs‎

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,8 @@
77
using GitHub.Extensions;
88
using GitHub.Primitives;
99
using LibGit2Sharp;
10-
using NullGuard;
11-
using System.Diagnostics;
1210
using NLog;
13-
using System.Security.Cryptography;
14-
using System.Text;
11+
using NullGuard;
1512

1613
namespace GitHub.Services
1714
{
@@ -191,20 +188,19 @@ public Task<Patch> Compare(
191188
});
192189
}
193190

194-
public Task<ContentChanges> CompareWithString(IRepository repository, string sha, string path, string contents)
191+
public Task<ContentChanges> CompareWith(IRepository repository, string sha, string path, [AllowNull] byte[] contents)
195192
{
196193
Guard.ArgumentNotNull(repository, nameof(repository));
197194
Guard.ArgumentNotEmptyString(sha, nameof(sha));
198195
Guard.ArgumentNotEmptyString(path, nameof(path));
199-
Guard.ArgumentNotEmptyString(contents, nameof(contents));
200196

201197
return Task.Factory.StartNew(() =>
202198
{
203199
var commit = repository.Lookup<Commit>(sha);
204200

205201
if (commit != null)
206202
{
207-
var contentStream = new MemoryStream(Encoding.UTF8.GetBytes(contents));
203+
var contentStream = contents != null ? new MemoryStream(contents) : new MemoryStream();
208204
var blob1 = commit[path]?.Target as Blob ?? repository.ObjectDatabase.CreateBlob(new MemoryStream());
209205
var blob2 = repository.ObjectDatabase.CreateBlob(contentStream, path);
210206
return repository.Diff.Compare(blob1, blob2);
@@ -304,14 +300,20 @@ public Task<string> ExtractFile(IRepository repository, string commitSha, string
304300
});
305301
}
306302

307-
public Task<bool> IsModified(IRepository repository, string path, [AllowNull] string contents)
303+
public Task<bool> IsModified(IRepository repository, string path, [AllowNull] byte[] contents)
308304
{
309305
return Task.Factory.StartNew(() =>
310306
{
311307
if (repository.RetrieveStatus(path) == FileStatus.Unaltered)
312308
{
313-
var repoContents = (repository.Head[path].Target as Blob)?.GetContentText();
314-
return !contents.EqualsIgnoringLineEndings(repoContents);
309+
var blob1 = (Blob)repository.Head[path].Target;
310+
311+
using (var s = contents != null ? new MemoryStream(contents) : new MemoryStream())
312+
{
313+
var blob2 = repository.ObjectDatabase.CreateBlob(s, path);
314+
var diff = repository.Diff.Compare(blob1, blob2);
315+
return diff.LinesAdded != 0 || diff.LinesDeleted != 0;
316+
}
315317
}
316318

317319
return true;

‎src/GitHub.Exports.Reactive/Services/IGitClient.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,11 +86,11 @@ public interface IGitClient
8686
/// <param name="repository">The repository</param>
8787
/// <param name="sha">The SHA of the first commit.</param>
8888
/// <param name="path">The relative path to the file.</param>
89-
/// <param name="contents">The string to compare with the file.</param>
89+
/// <param name="contents">The contents to compare with the file.</param>
9090
/// <returns>
9191
/// A <see cref="Patch"/> object or null if the commit could not be found in the repository.
9292
/// </returns>
93-
Task<ContentChanges> CompareWithString(IRepository repository, string sha, string path, string contents);
93+
Task<ContentChanges> CompareWith(IRepository repository, string sha, string path, byte[] contents);
9494

9595
/// Gets the value of a configuration key.
9696
/// </summary>
@@ -155,6 +155,6 @@ public interface IGitClient
155155
/// <param name="path">The relative path to the file.</param>
156156
/// <param name="contents">The file contents to test.</param>
157157
/// <returns></returns>
158-
Task<bool> IsModified(IRepository repository, string path, string contents);
158+
Task<bool> IsModified(IRepository repository, string path, byte[] contents);
159159
}
160160
}

‎src/GitHub.Exports.Reactive/Services/IPullRequestSession.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -58,19 +58,19 @@ public interface IPullRequestSession
5858
/// Gets a file touched by the pull request.
5959
/// </summary>
6060
/// <param name="relativePath">The relative path to the file.</param>
61-
/// <param name="snapshot">The current snapshot of the file in an editor.</param>
61+
/// <param name="snapshot">The editor file contents.</param>
6262
/// <returns>
6363
/// A <see cref="IPullRequestSessionFile"/> object or null if the file was not touched by
6464
/// the pull request.
6565
/// </returns>
66-
Task<IPullRequestSessionFile> GetFile(string relativePath, ITextSnapshot snapshot);
66+
Task<IPullRequestSessionFile> GetFile(string relativePath, byte[] contents);
6767

6868
/// <summary>
6969
/// Updates the line numbers of the inline comments of a file.
7070
/// </summary>
7171
/// <param name="relativePath">The relative path to the file.</param>
7272
/// <param name="contents">The new file contents.</param>
7373
/// <returns>A tack which completes when the operation has completed.</returns>
74-
Task RecaluateLineNumbers(string relativePath, string contents);
74+
Task RecaluateLineNumbers(string relativePath, byte[] contents);
7575
}
7676
}

‎src/GitHub.InlineReviews/Services/PullRequestSession.cs‎

Lines changed: 40 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -91,19 +91,45 @@ public async Task<IReactiveList<IPullRequestSessionFile>> GetAllFiles()
9191
/// <inheritdoc/>
9292
public async Task<IPullRequestSessionFile> GetFile(string relativePath)
9393
{
94-
var contents = await ReadAllTextAsync(GetFullPath(relativePath));
94+
var contents = await ReadAsync(GetFullPath(relativePath));
9595
return await GetFile(relativePath, contents);
9696
}
9797

9898
/// <inheritdoc/>
99-
public Task<IPullRequestSessionFile> GetFile(string relativePath, ITextSnapshot snapshot)
99+
public async Task<IPullRequestSessionFile> GetFile(string relativePath, byte[] contents)
100100
{
101-
var contents = snapshot.GetText();
102-
return GetFile(relativePath, contents);
101+
await getFilesLock.WaitAsync();
102+
103+
try
104+
{
105+
PullRequestSessionFile file;
106+
107+
relativePath = relativePath.Replace("\\", "/");
108+
109+
if (!fileIndex.TryGetValue(relativePath, out file))
110+
{
111+
// TODO: Check for binary files.
112+
if (FilePaths.Any(x => x == relativePath))
113+
{
114+
file = await CreateFile(relativePath, contents);
115+
fileIndex.Add(relativePath, file);
116+
}
117+
else
118+
{
119+
fileIndex.Add(relativePath, null);
120+
}
121+
}
122+
123+
return file;
124+
}
125+
finally
126+
{
127+
getFilesLock.Release();
128+
}
103129
}
104130

105131
/// <inheritdoc/>
106-
public async Task RecaluateLineNumbers(string relativePath, string contents)
132+
public async Task RecaluateLineNumbers(string relativePath, byte[] contents)
107133
{
108134
relativePath = relativePath.Replace("\\", "/");
109135

@@ -115,7 +141,7 @@ public async Task RecaluateLineNumbers(string relativePath, string contents)
115141
{
116142
var repo = gitService.GetRepository(Repository.LocalPath);
117143
var result = new Dictionary<IInlineCommentThreadModel, int>();
118-
var diff = await gitClient.CompareWithString(repo, file.BaseSha, relativePath, contents);
144+
var diff = await gitClient.CompareWith(repo, file.BaseSha, relativePath, contents);
119145

120146
file.Diff = diffService.ParseFragment(diff.Patch).ToList();
121147

@@ -140,43 +166,11 @@ public async Task RecaluateLineNumbers(string relativePath, string contents)
140166
}
141167
}
142168

143-
async Task<IPullRequestSessionFile> GetFile(string relativePath, string contents)
144-
{
145-
await getFilesLock.WaitAsync();
146-
147-
try
148-
{
149-
PullRequestSessionFile file;
150-
151-
relativePath = relativePath.Replace("\\", "/");
152-
153-
if (!fileIndex.TryGetValue(relativePath, out file))
154-
{
155-
// TODO: Check for binary files.
156-
if (FilePaths.Any(x => x == relativePath))
157-
{
158-
file = await CreateFile(relativePath, contents);
159-
fileIndex.Add(relativePath, file);
160-
}
161-
else
162-
{
163-
fileIndex.Add(relativePath, null);
164-
}
165-
}
166-
167-
return file;
168-
}
169-
finally
170-
{
171-
getFilesLock.Release();
172-
}
173-
}
174-
175-
async Task<PullRequestSessionFile> CreateFile(string relativePath, string contents)
169+
async Task<PullRequestSessionFile> CreateFile(string relativePath, byte[] contents)
176170
{
177171
var file = new PullRequestSessionFile();
178172
var repository = gitService.GetRepository(Repository.LocalPath);
179-
var changes = await gitClient.CompareWithString(repository, PullRequest.Base.Sha, relativePath, contents);
173+
var changes = await gitClient.CompareWith(repository, PullRequest.Base.Sha, relativePath, contents);
180174

181175
file.RelativePath = relativePath;
182176
file.BaseSha = PullRequest.Base.Sha;
@@ -211,7 +205,7 @@ async Task<ReactiveList<IPullRequestSessionFile>> CreateAllFiles()
211205

212206
foreach (var path in FilePaths)
213207
{
214-
var contents = await ReadAllTextAsync(GetFullPath(path));
208+
var contents = await ReadAsync(GetFullPath(path));
215209
result.Add(await CreateFile(path, contents));
216210
}
217211

@@ -259,15 +253,17 @@ static DiffLine Match(IEnumerable<DiffChunk> diff, IList<DiffLine> target)
259253
return null;
260254
}
261255

262-
async Task<string> ReadAllTextAsync(string path)
256+
async Task<byte[]> ReadAsync(string path)
263257
{
264258
if (os.File.Exists(path))
265259
{
266260
try
267261
{
268-
using (var reader = os.File.OpenText(path))
262+
using (var stream = os.File.OpenRead(path))
269263
{
270-
return await reader.ReadToEndAsync();
264+
var result = new byte[stream.Length];
265+
await stream.ReadAsync(result, 0, result.Length);
266+
return result;
271267
}
272268
}
273269
catch { }

‎src/GitHub.InlineReviews/Tags/InlineCommentTagger.cs‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ sealed class InlineCommentTagger : ITagger<InlineCommentTag>, IDisposable
3232
readonly Dictionary<IInlineCommentThreadModel, ITrackingPoint> trackingPoints;
3333
readonly int? tabsToSpaces;
3434
bool initialized;
35+
ITextDocument document;
3536
string fullPath;
3637
string relativePath;
3738
bool leftHandSide;
@@ -157,6 +158,8 @@ void Initialize()
157158
var bufferTag = buffer.Properties.GetProperty<PullRequestBufferTag>(typeof(PullRequestBufferTag), null);
158159
IPullRequestSession session = null;
159160

161+
document = buffer.Properties.GetProperty<ITextDocument>(typeof(ITextDocument));
162+
160163
if (bufferTag != null)
161164
{
162165
fullPath = bufferTag.RelativePath;
@@ -169,7 +172,6 @@ void Initialize()
169172
}
170173
else
171174
{
172-
var document = buffer.Properties.GetProperty<ITextDocument>(typeof(ITextDocument));
173175
fullPath = document.FilePath;
174176
}
175177

@@ -228,7 +230,7 @@ async void SessionChanged(IPullRequestSession session)
228230
if (snapshot == null) return;
229231

230232
var repository = gitService.GetRepository(session.Repository.LocalPath);
231-
file = await session.GetFile(relativePath, snapshot);
233+
file = await session.GetFile(relativePath, GetContents(snapshot));
232234

233235
NotifyTagsChanged();
234236
}
@@ -261,11 +263,27 @@ void Buffer_Changed(object sender, TextContentChangedEventArgs e)
261263
}
262264
}
263265

266+
byte[] GetContents(ITextSnapshot snapshot)
267+
{
268+
var currentText = snapshot.GetText();
269+
270+
var content = document.Encoding.GetBytes(currentText);
271+
272+
var preamble = document.Encoding.GetPreamble();
273+
if (preamble.Length == 0) return content;
274+
275+
var completeContent = new byte[preamble.Length + content.Length];
276+
Buffer.BlockCopy(preamble, 0, completeContent, 0, preamble.Length);
277+
Buffer.BlockCopy(content, 0, completeContent, preamble.Length, content.Length);
278+
279+
return completeContent;
280+
}
281+
264282
async Task Rebuild(ITextSnapshot snapshot)
265283
{
266284
if (buffer.CurrentSnapshot == snapshot)
267285
{
268-
await session.RecaluateLineNumbers(relativePath, buffer.CurrentSnapshot.GetText());
286+
await session.RecaluateLineNumbers(relativePath, GetContents(buffer.CurrentSnapshot));
269287

270288
foreach (var thread in file.InlineCommentThreads)
271289
{

‎submodules/rothko‎

0 commit comments

Comments
 (0)