New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open file from GitHub (MVP) #2060

Merged
merged 16 commits into from Nov 30, 2018

Conversation

Projects
None yet
4 participants
@jcansdale
Copy link
Collaborator

jcansdale commented Nov 16, 2018

This is a MVP implementation of open file from GitHub functionality.

With this functionality the user can paste a /blob/ style URL into the URL tab of the Open from GitHub dialog and be taken to the file/line/range after the target repository is opened or cloned.

When the target /blob/ isn't available in the working directory, a dialog will appear letting the user know about the problem. There is no functionality to assist navigation to an alternative remote branch or pull request (if that is where the /blob/ URL is from).

This functionality isn't advertised/discoverable and will be primarily used for gathering user feedback and testing when the functionality is explicitly pointed out.

Scope of MVP

  • Support https://github.com/owner/repo/blob/<branch>/<path>/<file> style URLs
  • Support https://github.com/owner/repo/blob/<SHA>/<path>/<file> style URLs
  • Support navigation to individual lines or ranges (e.g. URL ends with #L10 and #L25-L33)
  • Open any file that hasn't changed on the current branch
  • Show dialog if target file can't be found in local repository
  • Show dialog if working directory file is different to target file

Next iteration

  • Advertise functionality on Open from GitHub / URL page
  • Find pull request associated with permalink-URLs
  • Find remote branch associated with permalink-URLs
  • Automatically fetch when target file can't be found in local repository
  • Offer to open pull request when permalink points to a pull request file
  • Offer to checkout branch when link points to a remote branch

What this PR does

  • Bring together the Open from GuitHub and Open from Clipboard functionality
  • Always show URL tab of Open from GitHub dialog when there is a GitHub URL in the clipboard
  • Add .GitHubContextService.TryNavigateToContext for syncing Visual Studio context with context from a /blob/URL
  • Show warning dialog when working directory file is different to file pointed at by /blob/ URL
    • This can be established using the commit SHA or permalink URLs and the latest fetched commit of remote branches
  • Show warning dialog when file pointed at by /blob/ URL doesn't exist in local repository
    • This dialog is more helpful when the remote branch name is included in the URL (the more common case when browsing GitHub)
    • Then the /blob/ URL is a permalink, we don't currently know the remote branch/PR it is associated with

Notes and Issues

  • When a user opens a blob permalink, we don't know the name of the branch they're targeting

  • When a user opens a blob from a named branch, files on the current branch might be different to the target branch

    • The current implementation compares the working file with the remote branch version
  • If the user hasn't recently done a fetch, the targeted commit might not exist or the link might be out of date (if it uses a named branch)

    • The current implementation doesn't automatically do a fetch. A future version should automatically fetch if the file can't be found.
  • When a range of lines is selected and a solution is opened using Team Explorer - Home, the selected text is lost but the caret position remains the same

    • When selecting a range, we have the option to start the selection at the top or the bottom. The current implementation starts at the top, which means the caret is left at the bottom of the range when the solution opens. I think this might work better if we start the selection at the bottom, leaving the caret at the top of the range. My guess is that the top position of a range is generally more important.
    • This would be a nice refinement, but isn't currently implemented

Discoverability

This feature isn't being advertised in the MVP

With this new functionality a user can paste a blob style URL into the Open from GitHub dialog and have the target file and line number/range selected as soon as the clone or Open completes. We need to find a way to make this functionality discoverable.

Currently is a user copies a blob style URL into their clipboard clicks File > Open > Open from GitHub..., they will be presented with this:

image

If there is no URL in the clipboard, they will see this:

image

  • There is plenty of space below the repository field to let the user know what kind of URLs and shortcuts are supported here
  • There is also the possibility of showing elements of the deconstructed URL (owner/repo/branch/line number/range)
  • We could even check the repository exists and fetch its description

For the MVP I think it would make sense to work on these incrementally. Start off with a static description and only show elements of the deconstructed URL if they're deemed to be particularly useful.

What do you think @donokuda?

Use GitHubContextService to navigate to files
When user clones a /blob/ type URL, navigate to the file after
cloning/opening the repository.

@jcansdale jcansdale force-pushed the feature/open-file-from-github branch from 990782b to cadf19a Nov 20, 2018

jcansdale added some commits Nov 20, 2018

Always default to clipboard URL for clone dialog
Default to clipboard URL when cloning form Start Page and Connect page.
This makes it consistent with current File > Open > Open from GitHub
behavior.

@jcansdale jcansdale changed the title [spike] Open file from GitHub [wip] Open file from GitHub (MVP) Nov 21, 2018

@jcansdale jcansdale force-pushed the feature/open-file-from-github branch from b32ad70 to 999e41b Nov 21, 2018

Remove unused field gitHubContextService
Remove gitHubContextService from OpenFromUrlCommand.

@jcansdale jcansdale force-pushed the feature/open-file-from-github branch from 75164dc to d3228ab Nov 22, 2018

jcansdale added some commits Nov 22, 2018

Add TryNavigateToContext to IGitHubContextService
Move context changing responsibilities away from RepositoryCloneService.
Add warnings when blob URL file can't be found
Warn when blob can't be found in the repository.
Warn when the working file is different blob URL file.

@jcansdale jcansdale force-pushed the feature/open-file-from-github branch from f9f7bd8 to 030e2da Nov 29, 2018

@jcansdale jcansdale changed the title [wip] Open file from GitHub (MVP) Open file from GitHub (MVP) Nov 30, 2018

jcansdale added some commits Nov 30, 2018

@StanleyGoldman

This comment has been minimized.

Copy link
Contributor

StanleyGoldman commented Nov 30, 2018

I opened the dialog with some random markdown in my clipboard.

image

2018-11-30 07:54:17.636 [22924] EROR [01] RepositorySelectViewModel Error reading repository list from GitHub.Primitives.HostAddress
System.ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index
   at System.ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument argument, ExceptionResource resource)
   at Octokit.GraphQL.Core.PagedQuery`1.Runner.<RunPage>d__15.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at Octokit.GraphQL.ConnectionExtensions.<Run>d__2`1.MoveNext()
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at GitHub.Services.RepositoryCloneService.<ReadViewerRepositories>d__10.MoveNext() in C:\Users\Spade\Projects\GitHub\VisualStudio\src\GitHub.App\Services\RepositoryCloneService.cs:line 107
--- End of stack trace from previous location where exception was thrown ---
   at System.Runtime.CompilerServices.TaskAwaiter.ThrowForNonSuccess(Task task)
   at System.Runtime.CompilerServices.TaskAwaiter.HandleNonSuccessAndDebuggerNotification(Task task)
   at System.Runtime.CompilerServices.ConfiguredTaskAwaitable`1.ConfiguredTaskAwaiter.GetResult()
   at GitHub.ViewModels.Dialog.Clone.RepositorySelectViewModel.<Activate>d__37.MoveNext() in C:\Users\Spade\Projects\GitHub\VisualStudio\src\GitHub.App\ViewModels\Dialog\Clone\RepositorySelectViewModel.cs:line 111
@grokys

grokys approved these changes Nov 30, 2018

jcansdale added some commits Nov 30, 2018

Don't open folder when solution part of repository
Don't close solution and open folder view when solution is already in
the targeted repository.
@jcansdale

This comment has been minimized.

Copy link
Collaborator

jcansdale commented Nov 30, 2018

Thanks @StanleyGoldman, will fix #2060 (comment) next week in a separate PR.

@jcansdale jcansdale merged commit 865845e into master Nov 30, 2018

2 checks passed

continuous-integration/appveyor/branch AppVeyor build succeeded
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details

@jcansdale jcansdale deleted the feature/open-file-from-github branch Nov 30, 2018

@meaghanlewis meaghanlewis added this to the 2.6.0 milestone Nov 30, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment