Be consistent about how we use relative paths and Git paths

src/GitHub.App/SampleData/GitServiceDesigner.cs

@@ -15,7 +15,7 @@ class GitServiceDesigner : IGitService
         public IRepository GetRepository(string path) => null;
         public UriString GetUri(string path, string remote = "origin") => null;
         public UriString GetUri(IRepository repository, string remote = "origin") => null;
-        public Task<Patch> Compare(IRepository repository, string sha1, string sha2, string path) => null;
+        public Task<Patch> Compare(IRepository repository, string sha1, string sha2, string relativePath) => null;
         public Task<ContentChanges> CompareWith(IRepository repository, string sha1, string sha2, string path, byte[] contents) => null;
         public Task<TreeChanges> Compare(IRepository repository, string sha1, string sha2, bool detectRenames = false) => null;
     }

src/GitHub.App/Services/GitClient.cs

@@ -259,40 +259,42 @@ public Task<Remote> GetHttpRemote(IRepository repo, string remote)
             });
         }
 
-        public Task<string> ExtractFile(IRepository repository, string commitSha, string fileName)
+        public Task<string> ExtractFile(IRepository repository, string commitSha, string relativePath)
         {
             Guard.ArgumentNotNull(repository, nameof(repository));
             Guard.ArgumentNotEmptyString(commitSha, nameof(commitSha));
-            Guard.ArgumentIsGitPath(fileName, nameof(fileName));
+            Guard.ArgumentIsRelativePath(relativePath, nameof(relativePath));
 
+            var gitPath = Paths.ToGitPath(relativePath);
             return Task.Run(() =>
             {
                 var commit = repository.Lookup<Commit>(commitSha);
                 if (commit == null)
                 {
-                    throw new FileNotFoundException("Couldn't find '" + fileName + "' at commit " + commitSha + ".");
+                    throw new FileNotFoundException("Couldn't find '" + gitPath + "' at commit " + commitSha + ".");
                 }
 
-                var blob = commit[fileName]?.Target as Blob;
+                var blob = commit[gitPath]?.Target as Blob;
                 return blob?.GetContentText();
             });
         }
 
-        public Task<byte[]> ExtractFileBinary(IRepository repository, string commitSha, string fileName)
+        public Task<byte[]> ExtractFileBinary(IRepository repository, string commitSha, string relativePath)
         {
             Guard.ArgumentNotNull(repository, nameof(repository));
             Guard.ArgumentNotEmptyString(commitSha, nameof(commitSha));
-            Guard.ArgumentIsGitPath(fileName, nameof(fileName));
+            Guard.ArgumentIsRelativePath(relativePath, nameof(relativePath));
 
+            var gitPath = Paths.ToGitPath(relativePath);
             return Task.Run(() =>
             {
                 var commit = repository.Lookup<Commit>(commitSha);
                 if (commit == null)
                 {
-                    throw new FileNotFoundException("Couldn't find '" + fileName + "' at commit " + commitSha + ".");
+                    throw new FileNotFoundException("Couldn't find '" + gitPath + "' at commit " + commitSha + ".");
                 }
 
-                var blob = commit[fileName]?.Target as Blob;
+                var blob = commit[gitPath]?.Target as Blob;
 
                 if (blob != null)
                 {
@@ -308,16 +310,17 @@ public Task<byte[]> ExtractFileBinary(IRepository repository, string commitSha,
             });
         }
 
-        public Task<bool> IsModified(IRepository repository, string path, byte[] contents)
+        public Task<bool> IsModified(IRepository repository, string relativePath, byte[] contents)
         {
             Guard.ArgumentNotNull(repository, nameof(repository));
-            Guard.ArgumentIsGitPath(path, nameof(path));
+            Guard.ArgumentIsRelativePath(relativePath, nameof(relativePath));
 
+            var gitPath = Paths.ToGitPath(relativePath);
             return Task.Run(() =>
             {
-                if (repository.RetrieveStatus(path) == FileStatus.Unaltered)
+                if (repository.RetrieveStatus(gitPath) == FileStatus.Unaltered)
                 {
-                    var treeEntry = repository.Head[path];
+                    var treeEntry = repository.Head[gitPath];
                     if (treeEntry?.TargetType != TreeEntryTargetType.Blob)
                     {
                         return false;
@@ -326,7 +329,7 @@ public Task<bool> IsModified(IRepository repository, string path, byte[] content
                     var blob1 = (Blob)treeEntry.Target;
                     using (var s = contents != null ? new MemoryStream(contents) : new MemoryStream())
                     {
-                        var blob2 = repository.ObjectDatabase.CreateBlob(s, path);
+                        var blob2 = repository.ObjectDatabase.CreateBlob(s, gitPath);
                         var diff = repository.Diff.Compare(blob1, blob2);
                         return diff.LinesAdded != 0 || diff.LinesDeleted != 0;
                     }

src/GitHub.App/Services/GitHubContextService.cs

@@ -282,7 +282,8 @@ public bool TryOpenFile(string repositoryDir, GitHubContext context)
                 return false;
             }
 
-            var fullPath = Path.Combine(repositoryDir, path.Replace('/', '\\'));
+            var relativePath = Paths.ToWindowsPath(path);
+            var fullPath = Path.Combine(repositoryDir, relativePath);
             var textView = OpenDocument(fullPath);
             SetSelection(textView, context);
             return true;
@@ -404,16 +405,17 @@ public string FindObjectishForTFSTempFile(string tempFile)
         }
 
         /// <inheritdoc/>
-        public bool HasChangesInWorkingDirectory(string repositoryDir, string commitish, string path)
+        public bool HasChangesInWorkingDirectory(string repositoryDir, string commitish, string relativePath)
         {
-            Guard.ArgumentNotNull(path, nameof(repositoryDir));
-            Guard.ArgumentNotNull(path, nameof(commitish));
-            Guard.ArgumentIsGitPath(path, nameof(path));
+            Guard.ArgumentNotNull(repositoryDir, nameof(repositoryDir));
+            Guard.ArgumentNotNull(commitish, nameof(commitish));
+            Guard.ArgumentIsRelativePath(relativePath, nameof(relativePath));
 
+            var gitPath = Paths.ToGitPath(relativePath);
             using (var repo = gitService.GetRepository(repositoryDir))
             {
                 var commit = repo.Lookup<Commit>(commitish);
-                var paths = new[] { path };
+                var paths = new[] { gitPath };
 
                 return repo.Diff.Compare<Patch>(commit.Tree, DiffTargets.WorkingDirectory, paths).Count() > 0;
             }

src/GitHub.App/Services/PullRequestEditorService.cs

@@ -7,6 +7,7 @@
 using System.Threading.Tasks;
 using EnvDTE;
 using GitHub.Commands;
+using GitHub.Primitives;
 using GitHub.Extensions;
 using GitHub.Models;
 using GitHub.Models.Drafts;
@@ -89,13 +90,12 @@ public async Task<ITextView> OpenFile(
 
             try
             {
-                var fullPath = GetAbsolutePath(session.LocalRepository, relativePath);
                 string fileName;
                 string commitSha;
 
                 if (workingDirectory)
                 {
-                    fileName = fullPath;
+                    fileName = Path.Combine(session.LocalRepository.LocalPath, relativePath);
                     commitSha = null;
                 }
                 else
@@ -119,7 +119,7 @@ public async Task<ITextView> OpenFile(
 
                     if (!workingDirectory)
                     {
-                        AddBufferTag(wpfTextView.TextBuffer, session, fullPath, commitSha, null);
+                        AddBufferTag(wpfTextView.TextBuffer, session, relativePath, commitSha, null);
                         EnableNavigateToEditor(textView, session);
                     }
                 }
@@ -478,13 +478,6 @@ public static int FindNearestMatchingLine(IList<string> fromLines, IList<string>
             return matchingLine;
         }
 
-        static string GetAbsolutePath(LocalRepositoryModel localRepository, string relativePath)
-        {
-            var localPath = localRepository.LocalPath;
-            relativePath = relativePath.Replace('/', Path.DirectorySeparatorChar);
-            return Path.Combine(localPath, relativePath);
-        }
-
         string GetText(IVsTextView textView)
         {
             IVsTextLines buffer;
@@ -561,21 +554,21 @@ void ShowErrorInStatusBar(string message, Exception e)
         void AddBufferTag(
             ITextBuffer buffer,
             IPullRequestSession session,
-            string path,
+            string relativePath,
             string commitSha,
             DiffSide? side)
         {
             buffer.Properties.GetOrCreateSingletonProperty(
                 typeof(PullRequestTextBufferInfo),
-                () => new PullRequestTextBufferInfo(session, path, commitSha, side));
+                () => new PullRequestTextBufferInfo(session, relativePath, commitSha, side));
 
             var projection = buffer as IProjectionBuffer;
 
             if (projection != null)
             {
                 foreach (var source in projection.SourceBuffers)
                 {
-                    AddBufferTag(source, session, path, commitSha, side);
+                    AddBufferTag(source, session, relativePath, commitSha, side);
                 }
             }
         }
@@ -642,9 +635,10 @@ async Task<string> GetBaseFileName(IPullRequestSession session, IPullRequestSess
                 session.LocalRepository,
                 session.PullRequest))
             {
-                var fileChange = changes.FirstOrDefault(x => x.Path == file.RelativePath);
+                var gitPath = Paths.ToGitPath(file.RelativePath);
+                var fileChange = changes.FirstOrDefault(x => x.Path == gitPath);
                 return fileChange?.Status == LibGit2Sharp.ChangeKind.Renamed ?
-                    fileChange.OldPath : file.RelativePath;
+                    Paths.ToWindowsPath(fileChange.OldPath) : file.RelativePath;
             }
         }
 

src/GitHub.App/Services/PullRequestService.cs

@@ -779,7 +779,7 @@ public async Task<string> ExtractToTempFile(
             Encoding encoding)
         {
             var tempFilePath = CalculateTempFileName(relativePath, commitSha, encoding);
-            var gitPath = relativePath.TrimStart('/').Replace('\\', '/');
+            var gitPath = Paths.ToGitPath(relativePath);
 
             if (!File.Exists(tempFilePath))
             {
@@ -922,24 +922,22 @@ async Task ExtractToTempFile(
             IRepository repo,
             int pullRequestNumber,
             string commitSha,
-            string path,
+            string relativePath,
             Encoding encoding,
             string tempFilePath)
         {
-            Guard.ArgumentIsGitPath(path, nameof(path));
-
             string contents;
 
             try
             {
-                contents = await gitClient.ExtractFile(repo, commitSha, path) ?? string.Empty;
+                contents = await gitClient.ExtractFile(repo, commitSha, relativePath) ?? string.Empty;
             }
             catch (FileNotFoundException)
             {
                 var pullHeadRef = $"refs/pull/{pullRequestNumber}/head";
                 var remote = await gitClient.GetHttpRemote(repo, "origin");
                 await gitClient.Fetch(repo, remote.Name, commitSha, pullHeadRef);
-                contents = await gitClient.ExtractFile(repo, commitSha, path) ?? string.Empty;
+                contents = await gitClient.ExtractFile(repo, commitSha, relativePath) ?? string.Empty;
             }
 
             Directory.CreateDirectory(Path.GetDirectoryName(tempFilePath));

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

@@ -1,6 +1,7 @@
 using System;
 using System.Collections.Generic;
 using System.Linq;
+using GitHub.Primitives;
 
 namespace GitHub.ViewModels.GitHubPane
 {
@@ -12,11 +13,11 @@ public class PullRequestDirectoryNode : IPullRequestDirectoryNode
         /// <summary>
         /// Initializes a new instance of the <see cref="PullRequestDirectoryNode"/> class.
         /// </summary>
-        /// <param name="relativePath">The path to the directory, relative to the repository.</param>
-        public PullRequestDirectoryNode(string relativePath)
+        /// <param name="relativeOrGitPath">The path to the directory, relative to the repository.</param>
+        public PullRequestDirectoryNode(string relativeOrGitPath)
         {
-            DirectoryName = System.IO.Path.GetFileName(relativePath);
-            RelativePath = relativePath.Replace("/", "\\");
+            DirectoryName = System.IO.Path.GetFileName(relativeOrGitPath);
+            RelativePath = Paths.ToWindowsPath(relativeOrGitPath);
             Directories = new List<IPullRequestDirectoryNode>();
             Files = new List<IPullRequestFileNode>();
         }

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

@@ -3,6 +3,7 @@
 using GitHub.App;
 using GitHub.Extensions;
 using GitHub.Models;
+using GitHub.Primitives;
 using ReactiveUI;
 
 namespace GitHub.ViewModels.GitHubPane
@@ -21,7 +22,7 @@ public class PullRequestFileNode : ReactiveObject, IPullRequestFileNode
         /// Initializes a new instance of the <see cref="PullRequestFileNode"/> class.
         /// </summary>
         /// <param name="repositoryPath">The absolute path to the repository.</param>
-        /// <param name="relativePath">The path to the file, relative to the repository.</param>
+        /// <param name="relativeOrGitPath">The path to the file, relative to the repository.</param>
         /// <param name="sha">The SHA of the file.</param>
         /// <param name="status">The way the file was changed.</param>
         /// <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
         /// </param>
         public PullRequestFileNode(
             string repositoryPath,
-            string relativePath,
+            string relativeOrGitPath,
             string sha,
             PullRequestFileStatus status,
             string oldPath)
         {
             Guard.ArgumentNotEmptyString(repositoryPath, nameof(repositoryPath));
-            Guard.ArgumentNotEmptyString(relativePath, nameof(relativePath));
+            Guard.ArgumentNotEmptyString(relativeOrGitPath, nameof(relativeOrGitPath));
             Guard.ArgumentNotEmptyString(sha, nameof(sha));
 
-            FileName = Path.GetFileName(relativePath);
-            RelativePath = relativePath.Replace("/", "\\");
+            FileName = Path.GetFileName(relativeOrGitPath);
+            RelativePath = Paths.ToWindowsPath(relativeOrGitPath);
             Sha = sha;
             Status = status;
             OldPath = oldPath;
@@ -54,7 +55,7 @@ public PullRequestFileNode(
             {
                 if (oldPath != null)
                 {
-                    StatusDisplay = Path.GetDirectoryName(oldPath) == Path.GetDirectoryName(relativePath) ?
+                    StatusDisplay = Path.GetDirectoryName(oldPath) == Path.GetDirectoryName(relativeOrGitPath) ?
                             Path.GetFileName(oldPath) : oldPath;
                 }
                 else

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

@@ -12,6 +12,7 @@
 using System.Windows.Input;
 using GitHub.Extensions;
 using GitHub.Models;
+using GitHub.Primitives;
 using GitHub.Services;
 using LibGit2Sharp;
 using ReactiveUI;
@@ -224,8 +225,8 @@ static string GetOldFileName(PullRequestFileModel file, TreeChanges changes)
         {
             if (file.Status == PullRequestFileStatus.Renamed)
             {
-                var fileName = file.FileName.Replace("/", "\\");
-                return changes?.Renamed.FirstOrDefault(x => x.Path == fileName)?.OldPath;
+                var gitPath = Paths.ToGitPath(file.FileName);
+                return changes?.Renamed.FirstOrDefault(x => x.Path == gitPath)?.OldPath;
             }
 
             return null;

src/GitHub.App/ViewModels/PullRequestReviewCommentThreadViewModel.cs

@@ -2,6 +2,7 @@
 using System.Collections.Generic;
 using System.ComponentModel.Composition;
 using System.Globalization;
+using System.IO;
 using System.Linq;
 using System.Reactive.Linq;
 using System.Threading.Tasks;
@@ -197,7 +198,7 @@ public override async Task PostComment(ICommentViewModel comment)
                     await Session.PostReviewComment(
                         comment.Body,
                         File.CommitSha,
-                        File.RelativePath.Replace("\\", "/"),
+                        Paths.ToGitPath(File.RelativePath),
                         File.Diff,
                         diffPosition.DiffLineNumber).ConfigureAwait(false);
                 }
@@ -234,8 +235,8 @@ public static (string key, string secondaryKey) GetDraftKeys(
             string relativePath,
             int lineNumber)
         {
-            relativePath = relativePath.Replace("\\", "/");
-            var key = Invariant($"pr-review-comment|{cloneUri}|{pullRequestNumber}|{relativePath}");
+            var gitPath = Paths.ToGitPath(relativePath);
+            var key = Invariant($"pr-review-comment|{cloneUri}|{pullRequestNumber}|{gitPath}");
             return (key, lineNumber.ToString(CultureInfo.InvariantCulture));
         }
 

src/GitHub.Exports/Primitives/Paths.cs

@@ -0,0 +1,33 @@
+using System;
+using System.IO;
+
+namespace GitHub.Primitives
+{
+    /// <summary>
+    /// Convert to and from Git paths.
+    /// </summary>
+    public static class Paths
+    {
+        public const char GitDirectorySeparatorChar = '/';
+
+        /// <summary>
+        /// Convert from a relative path to a Git path.
+        /// </summary>
+        /// <param name="relativePath">A relative path.</param>
+        /// <returns>A working directory relative path which uses the '/' directory separator.</returns>
+        public static string ToGitPath(string relativePath)
+        {
+            return relativePath.Replace(Path.DirectorySeparatorChar, GitDirectorySeparatorChar);
+        }
+
+        /// <summary>
+        /// Convert from a Git path to a path that uses the Windows directory separator ('\').
+        /// </summary>
+        /// <param name="gitPath">A relative path that uses the '/' directory separator.</param>
+        /// <returns>A relative path that uses the <see cref="Path.DirectorySeparatorChar"/> directory separator ('\' on Windows).</returns>
+        public static string ToWindowsPath(string gitPath)
+        {
+            return gitPath.Replace(GitDirectorySeparatorChar, Path.DirectorySeparatorChar);
+        }
+    }
+}