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

Commit ce9e5c4

Browse files
jcansdalejcansdale
authored
Merge pull request #2386 from github/refactor/relative-and-git-paths
Be consistent about how we use relative paths and Git paths
2 parents 4bd5541 + 52f2bf0 commit ce9e5c4

File tree

17 files changed

+122
-102
lines changed

17 files changed

+122
-102
lines changed

‎src/GitHub.App/SampleData/GitServiceDesigner.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class GitServiceDesigner : IGitService
1515
public IRepository GetRepository(string path) => null;
1616
public UriString GetUri(string path, string remote = "origin") => null;
1717
public UriString GetUri(IRepository repository, string remote = "origin") => null;
18-
public Task<Patch> Compare(IRepository repository, string sha1, string sha2, string path) => null;
18+
public Task<Patch> Compare(IRepository repository, string sha1, string sha2, string relativePath) => null;
1919
public Task<ContentChanges> CompareWith(IRepository repository, string sha1, string sha2, string path, byte[] contents) => null;
2020
public Task<TreeChanges> Compare(IRepository repository, string sha1, string sha2, bool detectRenames = false) => null;
2121
}

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

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -259,40 +259,42 @@ public Task<Remote> GetHttpRemote(IRepository repo, string remote)
259259
});
260260
}
261261

262-
public Task<string> ExtractFile(IRepository repository, string commitSha, string fileName)
262+
public Task<string> ExtractFile(IRepository repository, string commitSha, string relativePath)
263263
{
264264
Guard.ArgumentNotNull(repository, nameof(repository));
265265
Guard.ArgumentNotEmptyString(commitSha, nameof(commitSha));
266-
Guard.ArgumentIsGitPath(fileName, nameof(fileName));
266+
Guard.ArgumentIsRelativePath(relativePath, nameof(relativePath));
267267

268+
var gitPath = Paths.ToGitPath(relativePath);
268269
return Task.Run(() =>
269270
{
270271
var commit = repository.Lookup<Commit>(commitSha);
271272
if (commit == null)
272273
{
273-
throw new FileNotFoundException("Couldn't find '" + fileName + "' at commit " + commitSha + ".");
274+
throw new FileNotFoundException("Couldn't find '" + gitPath + "' at commit " + commitSha + ".");
274275
}
275276

276-
var blob = commit[fileName]?.Target as Blob;
277+
var blob = commit[gitPath]?.Target as Blob;
277278
return blob?.GetContentText();
278279
});
279280
}
280281

281-
public Task<byte[]> ExtractFileBinary(IRepository repository, string commitSha, string fileName)
282+
public Task<byte[]> ExtractFileBinary(IRepository repository, string commitSha, string relativePath)
282283
{
283284
Guard.ArgumentNotNull(repository, nameof(repository));
284285
Guard.ArgumentNotEmptyString(commitSha, nameof(commitSha));
285-
Guard.ArgumentIsGitPath(fileName, nameof(fileName));
286+
Guard.ArgumentIsRelativePath(relativePath, nameof(relativePath));
286287

288+
var gitPath = Paths.ToGitPath(relativePath);
287289
return Task.Run(() =>
288290
{
289291
var commit = repository.Lookup<Commit>(commitSha);
290292
if (commit == null)
291293
{
292-
throw new FileNotFoundException("Couldn't find '" + fileName + "' at commit " + commitSha + ".");
294+
throw new FileNotFoundException("Couldn't find '" + gitPath + "' at commit " + commitSha + ".");
293295
}
294296

295-
var blob = commit[fileName]?.Target as Blob;
297+
var blob = commit[gitPath]?.Target as Blob;
296298

297299
if (blob != null)
298300
{
@@ -308,16 +310,17 @@ public Task<byte[]> ExtractFileBinary(IRepository repository, string commitSha,
308310
});
309311
}
310312

311-
public Task<bool> IsModified(IRepository repository, string path, byte[] contents)
313+
public Task<bool> IsModified(IRepository repository, string relativePath, byte[] contents)
312314
{
313315
Guard.ArgumentNotNull(repository, nameof(repository));
314-
Guard.ArgumentIsGitPath(path, nameof(path));
316+
Guard.ArgumentIsRelativePath(relativePath, nameof(relativePath));
315317

318+
var gitPath = Paths.ToGitPath(relativePath);
316319
return Task.Run(() =>
317320
{
318-
if (repository.RetrieveStatus(path) == FileStatus.Unaltered)
321+
if (repository.RetrieveStatus(gitPath) == FileStatus.Unaltered)
319322
{
320-
var treeEntry = repository.Head[path];
323+
var treeEntry = repository.Head[gitPath];
321324
if (treeEntry?.TargetType != TreeEntryTargetType.Blob)
322325
{
323326
return false;
@@ -326,7 +329,7 @@ public Task<bool> IsModified(IRepository repository, string path, byte[] content
326329
var blob1 = (Blob)treeEntry.Target;
327330
using (var s = contents != null ? new MemoryStream(contents) : new MemoryStream())
328331
{
329-
var blob2 = repository.ObjectDatabase.CreateBlob(s, path);
332+
var blob2 = repository.ObjectDatabase.CreateBlob(s, gitPath);
330333
var diff = repository.Diff.Compare(blob1, blob2);
331334
return diff.LinesAdded != 0 || diff.LinesDeleted != 0;
332335
}

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

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,8 @@ public bool TryOpenFile(string repositoryDir, GitHubContext context)
282282
return false;
283283
}
284284

285-
var fullPath = Path.Combine(repositoryDir, path.Replace('/', '\\'));
285+
var relativePath = Paths.ToWindowsPath(path);
286+
var fullPath = Path.Combine(repositoryDir, relativePath);
286287
var textView = OpenDocument(fullPath);
287288
SetSelection(textView, context);
288289
return true;
@@ -404,16 +405,17 @@ public string FindObjectishForTFSTempFile(string tempFile)
404405
}
405406

406407
/// <inheritdoc/>
407-
public bool HasChangesInWorkingDirectory(string repositoryDir, string commitish, string path)
408+
public bool HasChangesInWorkingDirectory(string repositoryDir, string commitish, string relativePath)
408409
{
409-
Guard.ArgumentNotNull(path, nameof(repositoryDir));
410-
Guard.ArgumentNotNull(path, nameof(commitish));
411-
Guard.ArgumentIsGitPath(path, nameof(path));
410+
Guard.ArgumentNotNull(repositoryDir, nameof(repositoryDir));
411+
Guard.ArgumentNotNull(commitish, nameof(commitish));
412+
Guard.ArgumentIsRelativePath(relativePath, nameof(relativePath));
412413

414+
var gitPath = Paths.ToGitPath(relativePath);
413415
using (var repo = gitService.GetRepository(repositoryDir))
414416
{
415417
var commit = repo.Lookup<Commit>(commitish);
416-
var paths = new[] { path };
418+
var paths = new[] { gitPath };
417419

418420
return repo.Diff.Compare<Patch>(commit.Tree, DiffTargets.WorkingDirectory, paths).Count() > 0;
419421
}

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

Lines changed: 9 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Threading.Tasks;
88
using EnvDTE;
99
using GitHub.Commands;
10+
using GitHub.Primitives;
1011
using GitHub.Extensions;
1112
using GitHub.Models;
1213
using GitHub.Models.Drafts;
@@ -89,13 +90,12 @@ public async Task<ITextView> OpenFile(
8990

9091
try
9192
{
92-
var fullPath = GetAbsolutePath(session.LocalRepository, relativePath);
9393
string fileName;
9494
string commitSha;
9595

9696
if (workingDirectory)
9797
{
98-
fileName = fullPath;
98+
fileName = Path.Combine(session.LocalRepository.LocalPath, relativePath);
9999
commitSha = null;
100100
}
101101
else
@@ -119,7 +119,7 @@ public async Task<ITextView> OpenFile(
119119

120120
if (!workingDirectory)
121121
{
122-
AddBufferTag(wpfTextView.TextBuffer, session, fullPath, commitSha, null);
122+
AddBufferTag(wpfTextView.TextBuffer, session, relativePath, commitSha, null);
123123
EnableNavigateToEditor(textView, session);
124124
}
125125
}
@@ -478,13 +478,6 @@ public static int FindNearestMatchingLine(IList<string> fromLines, IList<string>
478478
return matchingLine;
479479
}
480480

481-
static string GetAbsolutePath(LocalRepositoryModel localRepository, string relativePath)
482-
{
483-
var localPath = localRepository.LocalPath;
484-
relativePath = relativePath.Replace('/', Path.DirectorySeparatorChar);
485-
return Path.Combine(localPath, relativePath);
486-
}
487-
488481
string GetText(IVsTextView textView)
489482
{
490483
IVsTextLines buffer;
@@ -561,21 +554,21 @@ void ShowErrorInStatusBar(string message, Exception e)
561554
void AddBufferTag(
562555
ITextBuffer buffer,
563556
IPullRequestSession session,
564-
string path,
557+
string relativePath,
565558
string commitSha,
566559
DiffSide? side)
567560
{
568561
buffer.Properties.GetOrCreateSingletonProperty(
569562
typeof(PullRequestTextBufferInfo),
570-
() => new PullRequestTextBufferInfo(session, path, commitSha, side));
563+
() => new PullRequestTextBufferInfo(session, relativePath, commitSha, side));
571564

572565
var projection = buffer as IProjectionBuffer;
573566

574567
if (projection != null)
575568
{
576569
foreach (var source in projection.SourceBuffers)
577570
{
578-
AddBufferTag(source, session, path, commitSha, side);
571+
AddBufferTag(source, session, relativePath, commitSha, side);
579572
}
580573
}
581574
}
@@ -642,9 +635,10 @@ async Task<string> GetBaseFileName(IPullRequestSession session, IPullRequestSess
642635
session.LocalRepository,
643636
session.PullRequest))
644637
{
645-
var fileChange = changes.FirstOrDefault(x => x.Path == file.RelativePath);
638+
var gitPath = Paths.ToGitPath(file.RelativePath);
639+
var fileChange = changes.FirstOrDefault(x => x.Path == gitPath);
646640
return fileChange?.Status == LibGit2Sharp.ChangeKind.Renamed ?
647-
fileChange.OldPath : file.RelativePath;
641+
Paths.ToWindowsPath(fileChange.OldPath) : file.RelativePath;
648642
}
649643
}
650644

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

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -779,7 +779,7 @@ public async Task<string> ExtractToTempFile(
779779
Encoding encoding)
780780
{
781781
var tempFilePath = CalculateTempFileName(relativePath, commitSha, encoding);
782-
var gitPath = relativePath.TrimStart('/').Replace('\\', '/');
782+
var gitPath = Paths.ToGitPath(relativePath);
783783

784784
if (!File.Exists(tempFilePath))
785785
{
@@ -922,24 +922,22 @@ async Task ExtractToTempFile(
922922
IRepository repo,
923923
int pullRequestNumber,
924924
string commitSha,
925-
string path,
925+
string relativePath,
926926
Encoding encoding,
927927
string tempFilePath)
928928
{
929-
Guard.ArgumentIsGitPath(path, nameof(path));
930-
931929
string contents;
932930

933931
try
934932
{
935-
contents = await gitClient.ExtractFile(repo, commitSha, path) ?? string.Empty;
933+
contents = await gitClient.ExtractFile(repo, commitSha, relativePath) ?? string.Empty;
936934
}
937935
catch (FileNotFoundException)
938936
{
939937
var pullHeadRef = $"refs/pull/{pullRequestNumber}/head";
940938
var remote = await gitClient.GetHttpRemote(repo, "origin");
941939
await gitClient.Fetch(repo, remote.Name, commitSha, pullHeadRef);
942-
contents = await gitClient.ExtractFile(repo, commitSha, path) ?? string.Empty;
940+
contents = await gitClient.ExtractFile(repo, commitSha, relativePath) ?? string.Empty;
943941
}
944942

945943
Directory.CreateDirectory(Path.GetDirectoryName(tempFilePath));

‎src/GitHub.App/ViewModels/GitHubPane/PullRequestDirectoryNode.cs‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Linq;
4+
using GitHub.Primitives;
45

56
namespace GitHub.ViewModels.GitHubPane
67
{
@@ -12,11 +13,11 @@ public class PullRequestDirectoryNode : IPullRequestDirectoryNode
1213
/// <summary>
1314
/// Initializes a new instance of the <see cref="PullRequestDirectoryNode"/> class.
1415
/// </summary>
15-
/// <param name="relativePath">The path to the directory, relative to the repository.</param>
16-
public PullRequestDirectoryNode(string relativePath)
16+
/// <param name="relativeOrGitPath">The path to the directory, relative to the repository.</param>
17+
public PullRequestDirectoryNode(string relativeOrGitPath)
1718
{
18-
DirectoryName = System.IO.Path.GetFileName(relativePath);
19-
RelativePath = relativePath.Replace("/", "\\");
19+
DirectoryName = System.IO.Path.GetFileName(relativeOrGitPath);
20+
RelativePath = Paths.ToWindowsPath(relativeOrGitPath);
2021
Directories = new List<IPullRequestDirectoryNode>();
2122
Files = new List<IPullRequestFileNode>();
2223
}

‎src/GitHub.App/ViewModels/GitHubPane/PullRequestFileNode.cs‎

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using GitHub.App;
44
using GitHub.Extensions;
55
using GitHub.Models;
6+
using GitHub.Primitives;
67
using ReactiveUI;
78

89
namespace GitHub.ViewModels.GitHubPane
@@ -21,7 +22,7 @@ public class PullRequestFileNode : ReactiveObject, IPullRequestFileNode
2122
/// Initializes a new instance of the <see cref="PullRequestFileNode"/> class.
2223
/// </summary>
2324
/// <param name="repositoryPath">The absolute path to the repository.</param>
24-
/// <param name="relativePath">The path to the file, relative to the repository.</param>
25+
/// <param name="relativeOrGitPath">The path to the file, relative to the repository.</param>
2526
/// <param name="sha">The SHA of the file.</param>
2627
/// <param name="status">The way the file was changed.</param>
2728
/// <param name="statusDisplay">The string to display in the [message] box next to the filename.</param>
@@ -31,17 +32,17 @@ public class PullRequestFileNode : ReactiveObject, IPullRequestFileNode
3132
/// </param>
3233
public PullRequestFileNode(
3334
string repositoryPath,
34-
string relativePath,
35+
string relativeOrGitPath,
3536
string sha,
3637
PullRequestFileStatus status,
3738
string oldPath)
3839
{
3940
Guard.ArgumentNotEmptyString(repositoryPath, nameof(repositoryPath));
40-
Guard.ArgumentNotEmptyString(relativePath, nameof(relativePath));
41+
Guard.ArgumentNotEmptyString(relativeOrGitPath, nameof(relativeOrGitPath));
4142
Guard.ArgumentNotEmptyString(sha, nameof(sha));
4243

43-
FileName = Path.GetFileName(relativePath);
44-
RelativePath = relativePath.Replace("/", "\\");
44+
FileName = Path.GetFileName(relativeOrGitPath);
45+
RelativePath = Paths.ToWindowsPath(relativeOrGitPath);
4546
Sha = sha;
4647
Status = status;
4748
OldPath = oldPath;
@@ -54,7 +55,7 @@ public PullRequestFileNode(
5455
{
5556
if (oldPath != null)
5657
{
57-
StatusDisplay = Path.GetDirectoryName(oldPath) == Path.GetDirectoryName(relativePath) ?
58+
StatusDisplay = Path.GetDirectoryName(oldPath) == Path.GetDirectoryName(relativeOrGitPath) ?
5859
Path.GetFileName(oldPath) : oldPath;
5960
}
6061
else

‎src/GitHub.App/ViewModels/GitHubPane/PullRequestFilesViewModel.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
using System.Windows.Input;
1313
using GitHub.Extensions;
1414
using GitHub.Models;
15+
using GitHub.Primitives;
1516
using GitHub.Services;
1617
using LibGit2Sharp;
1718
using ReactiveUI;
@@ -224,8 +225,8 @@ static string GetOldFileName(PullRequestFileModel file, TreeChanges changes)
224225
{
225226
if (file.Status == PullRequestFileStatus.Renamed)
226227
{
227-
var fileName = file.FileName.Replace("/", "\\");
228-
return changes?.Renamed.FirstOrDefault(x => x.Path == fileName)?.OldPath;
228+
var gitPath = Paths.ToGitPath(file.FileName);
229+
return changes?.Renamed.FirstOrDefault(x => x.Path == gitPath)?.OldPath;
229230
}
230231

231232
return null;

‎src/GitHub.App/ViewModels/PullRequestReviewCommentThreadViewModel.cs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
using System.Collections.Generic;
33
using System.ComponentModel.Composition;
44
using System.Globalization;
5+
using System.IO;
56
using System.Linq;
67
using System.Reactive.Linq;
78
using System.Threading.Tasks;
@@ -197,7 +198,7 @@ public override async Task PostComment(ICommentViewModel comment)
197198
await Session.PostReviewComment(
198199
comment.Body,
199200
File.CommitSha,
200-
File.RelativePath.Replace("\\", "/"),
201+
Paths.ToGitPath(File.RelativePath),
201202
File.Diff,
202203
diffPosition.DiffLineNumber).ConfigureAwait(false);
203204
}
@@ -234,8 +235,8 @@ public static (string key, string secondaryKey) GetDraftKeys(
234235
string relativePath,
235236
int lineNumber)
236237
{
237-
relativePath = relativePath.Replace("\\", "/");
238-
var key = Invariant($"pr-review-comment|{cloneUri}|{pullRequestNumber}|{relativePath}");
238+
var gitPath = Paths.ToGitPath(relativePath);
239+
var key = Invariant($"pr-review-comment|{cloneUri}|{pullRequestNumber}|{gitPath}");
239240
return (key, lineNumber.ToString(CultureInfo.InvariantCulture));
240241
}
241242

‎src/GitHub.Exports/Primitives/Paths.cs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
using System;
2+
using System.IO;
3+
4+
namespace GitHub.Primitives
5+
{
6+
/// <summary>
7+
/// Convert to and from Git paths.
8+
/// </summary>
9+
public static class Paths
10+
{
11+
public const char GitDirectorySeparatorChar = '/';
12+
13+
/// <summary>
14+
/// Convert from a relative path to a Git path.
15+
/// </summary>
16+
/// <param name="relativePath">A relative path.</param>
17+
/// <returns>A working directory relative path which uses the '/' directory separator.</returns>
18+
public static string ToGitPath(string relativePath)
19+
{
20+
return relativePath.Replace(Path.DirectorySeparatorChar, GitDirectorySeparatorChar);
21+
}
22+
23+
/// <summary>
24+
/// Convert from a Git path to a path that uses the Windows directory separator ('\').
25+
/// </summary>
26+
/// <param name="gitPath">A relative path that uses the '/' directory separator.</param>
27+
/// <returns>A relative path that uses the <see cref="Path.DirectorySeparatorChar"/> directory separator ('\' on Windows).</returns>
28+
public static string ToWindowsPath(string gitPath)
29+
{
30+
return gitPath.Replace(GitDirectorySeparatorChar, Path.DirectorySeparatorChar);
31+
}
32+
}
33+
}

0 commit comments

Comments
 (0)