Factor GitHub URL parsing into FindContextFromUrl

OpenFromUrlCommand now works with GitHubContext objects rather than
directly with URLs.

src/GitHub.App/Services/GitHubContext.cs

@@ -1,4 +1,6 @@
-namespace GitHub.App.Services
+using System;
+
+namespace GitHub.App.Services
 {
     public class GitHubContext
     {
@@ -9,5 +11,6 @@ public class GitHubContext
         public int? PullRequest { get; set; }
         public int? Issue { get; set; }
         public string Path { get; set; }
+        public int? Line { get; set; }
     }
 }

src/GitHub.App/Services/GitHubContextService.cs

@@ -35,13 +35,27 @@ public class GitHubContextService
         static readonly Regex windowTitlePathRegex = new Regex($"{path} at {branch} · {owner}/{repo} - ", RegexOptions.Compiled);
         static readonly Regex windowTitleBranchesRegex = new Regex($"Branches · {owner}/{repo} - ", RegexOptions.Compiled);
 
-        public GitHubContext FindContextFromUrl(UriString url)
+        public GitHubContext FindContextFromUrl(string url)
         {
+            var uri = new UriString(url);
+            if (!uri.IsValidUri)
+            {
+                return null;
+            }
+
+            if (!uri.IsHypertextTransferProtocol)
+            {
+                return null;
+            }
+
             return new GitHubContext
             {
-                Host = url.Host,
-                Owner = url.Owner,
-                RepositoryName = url.RepositoryName,
+                Host = uri.Host,
+                Owner = uri.Owner,
+                RepositoryName = uri.RepositoryName,
+                Path = FindSubPath(uri, "/blob/master/"),
+                PullRequest = FindPullRequest(uri),
+                Line = FindLine(uri)
             };
         }
 
@@ -68,6 +82,13 @@ public IEnumerable<string> FindWindowTitlesForClass(string className = "Chrome_W
             }
         }
 
+        public Uri ToRepositoryUrl(GitHubContext context)
+        {
+            var builder = new UriBuilder("https", context.Host ?? "github.com");
+            builder.Path = $"{context.Owner}/{context.RepositoryName}";
+            return builder.Uri;
+        }
+
         public GitHubContext FindContextFromWindowTitle(string windowTitle)
         {
             var (success, owner, repo, branch, pullRequest, issue, path) = MatchWindowTitle(windowTitle);
@@ -130,6 +151,60 @@ public GitHubContext FindContextFromWindowTitle(string windowTitle)
             return (match.Success, null, null, null, null, null, null);
         }
 
+
+        static int? FindLine(UriString gitHubUrl)
+        {
+            var prefix = "#L";
+            var url = gitHubUrl.ToString();
+            var index = url.LastIndexOf(prefix);
+            if (index == -1)
+            {
+                return null;
+            }
+
+            if (!int.TryParse(url.Substring(index + prefix.Length), out int lineNumber))
+            {
+                return null;
+            }
+
+            return lineNumber; // 1 based
+        }
+
+        static int? FindPullRequest(UriString gitHubUrl)
+        {
+            var pullRequest = FindSubPath(gitHubUrl, "/pull/")?.Split('/').First();
+            if (pullRequest == null)
+            {
+                return null;
+            }
+
+            if (!int.TryParse(pullRequest, out int number))
+            {
+                return null;
+            }
+
+            return number;
+        }
+
+        static string FindSubPath(UriString gitHubUrl, string matchPath)
+        {
+            var url = gitHubUrl.ToString();
+            var prefix = gitHubUrl.ToRepositoryUrl() + matchPath;
+            if (!url.StartsWith(prefix))
+            {
+                return null;
+            }
+
+            var endIndex = url.IndexOf('#');
+            if (endIndex == -1)
+            {
+                endIndex = gitHubUrl.Length;
+            }
+
+            var path = url.Substring(prefix.Length, endIndex - prefix.Length);
+            return path;
+        }
+
         static class User32
         {
             [DllImport("user32.dll", SetLastError = true)]

src/GitHub.VisualStudio/Commands/OpenFromUrlCommand.cs

@@ -6,7 +6,6 @@
 using System.ComponentModel.Composition;
 using GitHub.Commands;
 using GitHub.Services;
-using GitHub.Primitives;
 using GitHub.App.Services;
 using GitHub.Services.Vssdk.Commands;
 using EnvDTE;
@@ -65,34 +64,20 @@ public override async Task Execute(string url)
                 url = Clipboard.GetText(TextDataFormat.Text);
             }
 
-            var gitHubUrl = new UriString(url);
-            if (!gitHubUrl.IsValidUri || !gitHubUrl.IsHypertextTransferProtocol)
-            {
-                gitHubUrl = null;
-            }
-
-            GitHubContext context = null;
-            if (gitHubUrl == null)
-            {
-                // HACK: Reconstruct a GitHub URL from the topmost browser window
-                context = gitHubContextService.Value.FindContextFromBrowser();
-                if (context != null)
-                {
-                    gitHubUrl = $"https://github.com/{context.Owner}/{context.RepositoryName}";
-                }
-            }
+            var context = gitHubContextService.Value.FindContextFromUrl(url);
+            context = context ?? gitHubContextService.Value.FindContextFromBrowser();
 
-            if (gitHubUrl == null)
+            if (context == null)
             {
                 return;
             }
 
             // Keep repos in unique dir while testing
             var defaultSubPath = "GitHubCache";
 
-            var cloneUrl = gitHubUrl.ToRepositoryUrl().ToString();
-            var targetDir = Path.Combine(repositoryCloneService.Value.DefaultClonePath, defaultSubPath, gitHubUrl.Owner);
-            var repositoryDirName = gitHubUrl.RepositoryName;
+            var cloneUrl = gitHubContextService.Value.ToRepositoryUrl(context).ToString();
+            var targetDir = Path.Combine(repositoryCloneService.Value.DefaultClonePath, defaultSubPath, context.Owner);
+            var repositoryDirName = context.RepositoryName;
             var repositoryDir = Path.Combine(targetDir, repositoryDirName);
 
             if (!Directory.Exists(repositoryDir))
@@ -139,8 +124,8 @@ public override async Task Execute(string url)
                 }
             }
 
-            await TryOpenPullRequest(gitHubUrl);
-            TryOpenFile(gitHubUrl, context, repositoryDir);
+            await TryOpenPullRequest(context);
+            TryOpenFile(context, repositoryDir);
         }
 
         VSConstants.MessageBoxResult ShowInfoMessage(string message)
@@ -180,9 +165,9 @@ static string FindSolutionDirectory(Solution solution)
             return null;
         }
 
-        bool TryOpenFile(UriString gitHubUrl, GitHubContext context, string repositoryDir)
+        bool TryOpenFile(GitHubContext context, string repositoryDir)
         {
-            var path = FindSubPath(gitHubUrl, "/blob/master/") ?? context?.Path;
+            var path = context.Path;
             if (path == null)
             {
                 return false;
@@ -204,70 +189,28 @@ bool TryOpenFile(UriString gitHubUrl, GitHubContext context, string repositoryDi
 
             dte.Value.ItemOperations.OpenFile(fullPath);
 
-            var lineNumber = FindLineNumber(gitHubUrl);
-            if (lineNumber != -1)
+            var lineNumber = context.Line;
+            if (lineNumber == null)
             {
                 var activeView = pullRequestEditorService.Value.FindActiveView();
-                ErrorHandler.ThrowOnFailure(activeView.SetCaretPos(lineNumber, 0));
-                ErrorHandler.ThrowOnFailure(activeView.CenterLines(lineNumber, 1));
+                ErrorHandler.ThrowOnFailure(activeView.SetCaretPos(lineNumber.Value, 0));
+                ErrorHandler.ThrowOnFailure(activeView.CenterLines(lineNumber.Value, 1));
             }
 
             return true;
         }
 
-        async Task<bool> TryOpenPullRequest(UriString gitHubUrl)
+        async Task<bool> TryOpenPullRequest(GitHubContext context)
         {
-            var pullRequest = FindSubPath(gitHubUrl, "/pull/");
+            var pullRequest = context.PullRequest;
             if (pullRequest == null)
             {
                 return false;
             }
 
-            if (!int.TryParse(pullRequest, out int number))
-            {
-                return false;
-            }
-
             var host = await gitHubToolWindowManager.Value.ShowGitHubPane();
-            await host.ShowPullRequest(gitHubUrl.Owner, gitHubUrl.RepositoryName, number);
+            await host.ShowPullRequest(context.Owner, context.RepositoryName, pullRequest.Value);
             return true;
         }
-
-        static string FindSubPath(UriString gitHubUrl, string matchPath)
-        {
-            var url = gitHubUrl.ToString();
-            var prefix = gitHubUrl.ToRepositoryUrl() + matchPath;
-            if (!url.StartsWith(prefix))
-            {
-                return null;
-            }
-
-            var endIndex = url.IndexOf('#');
-            if (endIndex == -1)
-            {
-                endIndex = gitHubUrl.Length;
-            }
-
-            var path = url.Substring(prefix.Length, endIndex - prefix.Length);
-            return path;
-        }
-
-        static int FindLineNumber(UriString gitHubUrl)
-        {
-            var prefix = "#L";
-            var url = gitHubUrl.ToString();
-            var index = url.LastIndexOf(prefix);
-            if (index == -1)
-            {
-                return -1;
-            }
-
-            if (!int.TryParse(url.Substring(index + prefix.Length), out int lineNumber))
-            {
-                return -1;
-            }
-
-            return lineNumber - 1;
-        }
     }
 }

test/GitHub.App.UnitTests/Services/GitHubContextServiceTests.cs

@@ -1,4 +1,5 @@
-using GitHub.App.Services;
+using System;
+using GitHub.App.Services;
 using NUnit.Framework;
 
 public class GitHubContextServiceTests
@@ -43,6 +44,78 @@ public void Host(string url, string expectHost)
 
             Assert.That(context.Host, Is.EqualTo(expectHost));
         }
+
+        [TestCase("https://github.com", null)]
+        [TestCase("https://github.com/github", null)]
+        [TestCase("https://github.com/github/VisualStudio", null)]
+        [TestCase("https://github.com/github/VisualStudio/blob/master/README.md", null)]
+        [TestCase("https://github.com/github/VisualStudio/pull/1763", 1763)]
+        [TestCase("https://github.com/github/VisualStudio/pull/1763/commits", 1763)]
+        [TestCase("https://github.com/github/VisualStudio/pull/1763/files#diff-7384294e6c288e13bad0293bae232754R1", 1763)]
+        [TestCase("https://github.com/github/VisualStudio/pull/NaN", null)]
+        public void PullRequest(string url, int? expectPullRequest)
+        {
+            var target = new GitHubContextService();
+
+            var context = target.FindContextFromUrl(url);
+
+            Assert.That(context?.PullRequest, Is.EqualTo(expectPullRequest));
+        }
+
+        [TestCase("https://github.com", null)]
+        [TestCase("https://github.com/github", null)]
+        [TestCase("https://github.com/github/VisualStudio", null)]
+        [TestCase("https://github.com/github/VisualStudio/blob/master/README.md", "README.md")]
+        [TestCase("https://github.com/github/VisualStudio/blob/master/README.md#notices", "README.md")]
+        public void Path(string url, string expectPath)
+        {
+            var target = new GitHubContextService();
+
+            var context = target.FindContextFromUrl(url);
+
+            Assert.That(context.Path, Is.EqualTo(expectPath));
+        }
+
+        [TestCase("https://github.com", null)]
+        [TestCase("https://github.com/github", null)]
+        [TestCase("https://github.com/github/VisualStudio", null)]
+        [TestCase("https://github.com/github/VisualStudio/blob/master/README.md", null)]
+        [TestCase("https://github.com/github/VisualStudio/blob/master/README.md#notices", null)]
+        [TestCase("https://github.com/github/VisualStudio/blob/master/src/GitHub.VisualStudio/GitHubPackage.cs#L38", 38)]
+        public void Line(string url, int? expectLine)
+        {
+            var target = new GitHubContextService();
+
+            var context = target.FindContextFromUrl(url);
+
+            Assert.That(context.Line, Is.EqualTo(expectLine));
+        }
+
+        [TestCase("foo", true)]
+        [TestCase("ssh://[email protected]:443/benstraub/libgit2", true)]
+        [TestCase("https://github.com/github/VisualStudio", false)]
+        public void IsNull(string url, bool expectNull)
+        {
+            var target = new GitHubContextService();
+
+            var context = target.FindContextFromUrl(url);
+
+            Assert.That(context, expectNull ? Is.Null : Is.Not.Null);
+        }
+    }
+
+    public class TheToMethod
+    {
+        [Test]
+        public void DefaultGitHubDotCom()
+        {
+            var context = new GitHubContext { Host = "github.com", Owner = "github", RepositoryName = "VisualStudio" };
+            var target = new GitHubContextService();
+
+            var uri = target.ToRepositoryUrl(context);
+
+            Assert.That(uri, Is.EqualTo(new Uri("https://github.com/github/VisualStudio")));
+        }
     }
 
     public class TheFindContextFromWindowTitleMethod