Open from clipboard url #1771
Open from clipboard url #1771
Conversation
Use `GitHub.OpenFromUrl <url>` from the VS Command Window to clone a target URL. This spike uses existing `ShowReCloneDialog` method.
Expect Owner="github", RepositoryName="VisualStudio" from: https://github.com/github/VisualStudio/blob/master/README.md Not Owner="master", RepositoryName="README.md"
Only files on master branch are currently supported.
Trim line number when using FindPath.
Prototype this using the happy path where we use the default repository location.
If we're already in an appropriate context, simply open the target file.
The following URL will now have an owner of "owner": https://github.com/owner Previously it would make a repository name of "owner".
Add code to parse window titles and find the GitHub context from the topmost Chrome browser.
If there is no URL in the paste buffer, use the GitHub context from the topmost browser.
When browser is in the context of a file, open a file with the same name after repository is opened.
OpenFromUrlCommand now works with GitHubContext objects rather than directly with URLs.
Visual Studio line numbers are zero based.
Assume that branch names don't contain a '/' (sic).
Navigate to line when line number information is available.
Search for Chrome then Firefox windows. Need to find a way to search in order of height.
Previously highlighting only worked when a range was selected. Use VsShellUtilities.OpenDocument instead of DTE.ItemOperations.OpenFile. Factor TryOpenFile into GitHubContextService.
Don't continue to column 0 on the line below.
GitHub URLs combine branches/SHAs and tree paths. For example: https://github.com/github/visualstudio/tree/master/src Git would represent "master/src" as the tree-ish "master:src". Because looking at the URL we don't know where the branch stops and the tree starts, "master/src" is being stored as the TreeishPath property.
There was a problem hiding this comment.
This seems to work well! I personally think the error messages are good enough for now. This feature could really use a UI to help ease people into it, but this is good as a MVP!
Got a few questions on the code, and also it would be good to get some XML documentation in there.
| [TestCase("https://git01.codeplex.com/nuget", "git01.codeplex.com", "nuget", null, | ||
| Description = "We assume the first component is the owner")] | ||
| [TestCase("https://example.com/vpath/foo/bar", "example.com", "vpath", "foo")] | ||
| [TestCase("https://example.com/vpath/foo/bar.git", "example.com", "vpath", "foo")] |
There was a problem hiding this comment.
vpath here suggests "virtual path" which I assume is because enterprise installations can have a virtual path before the owner? I this PR changes the parsing of this to not work with vpaths, but maybe we should change the string vpath to something else here?
| void InvokeService<T1, T2, T3, T4>(Type serviceType, string method, T1 arg1, T2 arg2, T3 arg3, T4 arg4) | ||
| { | ||
| var service = serviceProvider.GetService(serviceType); | ||
| var action = (Action<T1, T2, T3, T4>)Delegate.CreateDelegate(typeof(Action<T1, T2, T3, T4>), service, method); |
There was a problem hiding this comment.
Ohh, magic! Could you explain what's happening here? :)
There was a problem hiding this comment.
The IGitExt2 interface was introduced in an update of Visual Studio 2017, so we can't be sure it exists. I'm grabbing the IGitExt2 type by name and if it exists constructing and invoking a delegate on the AnnotateFile method.
My helper method was retrieving the service and calling the method. I've simplified it to only create and invoke the delegate. Using reflection was just too ugly. ;-)
The method is now:
void Invoke<T1, T2, T3, T4>(object target, string method, T1 arg1, T2 arg2, T3 arg3, T4 arg4)
{
var action = (Action<T1, T2, T3, T4>)Delegate.CreateDelegate(typeof(Action<T1, T2, T3, T4>), target, method);
action.Invoke(arg1, arg2, arg3, arg4);
}| namespace GitHub.App.Services | ||
| { | ||
| [Export(typeof(IGitHubContextService))] | ||
| public class GitHubContextService : IGitHubContextService |
There was a problem hiding this comment.
We now have GitHub.Services.Vssdk - seeing as this service mostly deals with the VSSDK, would that be a better place for it?
There was a problem hiding this comment.
Given its dependencies on the Microsoft.VisualStudio.* assemblies, I think this makes sense.
| using LibGit2Sharp; | ||
| using Task = System.Threading.Tasks.Task; | ||
|
|
||
| namespace GitHub.App.Services |
There was a problem hiding this comment.
Remove the .App from the namespace.
| return context; | ||
| } | ||
|
|
||
| public GitHubContext FindContextFromBrowser() |
There was a problem hiding this comment.
I thought we'd decided not to put this feature in?
There was a problem hiding this comment.
This feature is on a back-burner, but the code and the hope is to eventually reactivate it.
The command that uses it GitHub.OpenFromUrl has been removed from the UI. It was previously surfaced via File > Open > Open from GitHub.... The command that is surfaced via GitHub > Open from clipboard is GitHub.OpenFromClipboard`.
It does make sense to remove it from GitHub.OpenFromUrl (which needs to be explicitly bound to a keyboard shortcut).
"Couldn't open from 'https://github.com/owner/name'. Only URLs that link to repository files are currently supported."
Can't find anything about vpaths being a thing on GitHub Enterprise.
The `GitHub.OpenFromUrl` command isn't exposed on the UI but still shouldn't be calling FindContextFromBrowser .
|
I started moving What do you think? |
|
@jcansdale this feature is working as expected ✨ . It's nice being able to easily open files from what's pasted in the clipboard! I think the error messages are good as they are. Whenever I saw a message it was usually always clear what was happening. The name/icon As for the code from the original spike, it's probably okay to keep around if you think you'll refer back to it in the future. Otherwise it might be good to remove any unused code. |
Yeah, I generally prefer to keep unused code out of master, as it tends to rot. You can always keep it in a branch for easy recall later! I'll leave this to be your call though. |
✨ ✨ ✨ I like this idea! I'm a fan of explicit actions over convenience (e.g., reading the clipboard contents without user input) since it gives a level of control back to the user by letting them double check if they're opening the right URL. I think you were using the search bar as a general example, but what are your thoughts of having a dialog with a text field instead? Maybe something similar to:
Only thing I'm not sure about is having the entry point for opening a URL in the editor context menu. This is primarily because I expect actions in the editor context menu to be related to the file at hand. While I understand the reasoning of "Open from clipboard" being a complimentary action to "Copy link to clipboard," I think the times when someone will want to copy a link to their clipboard vs open a link from GitHub are so far apart that the connection between the actions won't be obvious enough (Holy run-on-sentence, batman! 😄) I think having this action apart of the File > Open from GitHub... menu makes more sense. Is the expectation that we'll move this command to there and deprecate the context menu? |
This was also a specific suggestion because users are now very familiar with the combined address/search field. I think it was originally pioneered in Chrome, but Firefox and Edge have followed. The Having a dialog with a text field will greatly expand our options when it comes to communicating to the user what kind of URLs are supported and what action they want to take. E.g. when opening a URL from another a different fork, do they want to navigate in the current solution or open/clone the specific fork? We'll definitely need this for the
I'd be fine with that. TBH, I think the discoverability of the We should maybe consider moving some of these command to a main |
Since `Open from clipboard` requires a repository to navigate, we only want to advertise the command when in the context of a repository.
jcansdale
force-pushed
the
feature/open-from-clipboard-url
branch
from
eca86c0 to
bec5eca
Compare
This broke unit tests and probably should be done on UI thread.
This could be something for the GitHubHub maybe? I don't think the GitHub pane filter would be a very obvious place to put that functionality, but it might work in the GitHubHub.
You also need to have a text editor open, if you don't you have to open a random file in order to paste the URL! |
What this PR does
This PR is the first component of some more more general - Open from GitHub URL - functionality. It enables navigation from a URL that's in the clipboard to a file in the active solution. It won't offer to change branches or open a different solution or folder. The idea is to hand over to future clone/open UI when necessary before continuing the navigate operation.
It is designed to be complimentary with existing
Copy link to clipboardfunctionality (available from theGitHubcode context menu). You should be able to navigate back to any URL created withGitHub > Copy link to clipboardby using theGitHub > Open from clipboardcommand.To do
GitHub > Open from clipboardwhen there's an active repositoryWhat works
Warnings
Known Issues
Reviewers
Open from clipboardOpen from GitHub.../OpenFromUrlCommandspike still exists, but isn't surfaced on the UI anywhere. It is only accessible by biding a keyboard shortcut toGitHub.OpenFromUrl. Is this okay or should it be removed?