Skip to content
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

Enable navigation from historical file to solution #1783

Merged
merged 4 commits into from Aug 4, 2018

Conversation

Projects
None yet
3 participants
@jcansdale
Copy link
Collaborator

jcansdale commented Jul 17, 2018

What this PR does

Extend the GoToSolutionOrPullRequestFile command to support navigation from a file opened using the history view to the same file in the users solution/working directory.

How to test

The history of a file

  1. Right click on a source file from a repository and Code context > View History...
    image

  2. Double click on a revision in the history
    image

  3. Right click on the read-only historical file and Open File in Solution
    image

  4. It should navigate to the equivalent position in your solution / working directory

The commit history

  1. Select View History... from the status bar
    image

  2. Double click on a commit in the history
    image

  3. Double click on a changed file in the commit
    image

  4. Right click on the commit diff view and Open File in Solution
    image

  5. It should navigate to the equivalent position in your solution / working directory

Enable navigation from historical file to solution
Extend the GoToSolutionOrPullRequestFile command to support navigation
from a file opened using the history view to the same file in the users
solution/working directory.

@jcansdale jcansdale changed the title [wip] Enable navigation from historical file to solution Enable navigation from historical file to solution Jul 18, 2018

@StanleyGoldman StanleyGoldman self-requested a review Jul 27, 2018

var match = tempFileObjectishRegex.Match(tempFile);
if (match.Success)
{
return match.Groups["objectish"].Value;

This comment has been minimized.

@StanleyGoldman

StanleyGoldman Jul 27, 2018

Contributor

Why did you name the group objectish?

This comment has been minimized.

@StanleyGoldman

StanleyGoldman Jul 27, 2018

Contributor

I guess it's not you, that's just what the value is called in RepoExtensions.Lookup(...). Nvm me.

This comment has been minimized.

@StanleyGoldman

StanleyGoldman Jul 27, 2018

Contributor

(first 8 chars of a blob SHA) ahh

This comment has been minimized.

@jcansdale

jcansdale Jul 27, 2018

Author Collaborator

It started off being called blobish, but I stole the name from the Lookup method you found. 😄

foreach (var commit in repo.Commits)
{
var trees = new Stack<Tree>();
trees.Push(commit.Tree);

This comment has been minimized.

@StanleyGoldman

StanleyGoldman Jul 27, 2018

Contributor

I now know what a Git blob sha is, I've yet to wrap my head around a Commit Tree.

This comment has been minimized.

@StanleyGoldman

StanleyGoldman Jul 27, 2018

Contributor

I have to read the tests more

@StanleyGoldman
Copy link
Contributor

StanleyGoldman left a comment

Something small to refactor..

if (objectish == null)
{
return false;
}

This comment has been minimized.

@StanleyGoldman

StanleyGoldman Jul 27, 2018

Contributor

The first three if statements from this function and the function above are meant to be the same.
The only minor minor difference (took me a moment to spot) is you are saving the repository dir in the second implementation.

The intent might be clearer if you combine both to one function that returns repositoryDir if the conditions are right or null if they fail.

This comment has been minimized.

@StanleyGoldman

StanleyGoldman Jul 27, 2018

Contributor

GetRepositoryDirectoryForSourceViewAndPath or something..

@StanleyGoldman

This comment has been minimized.

Copy link
Contributor

StanleyGoldman commented Jul 27, 2018

Everything works here. Just one thing that can be clarified i spotted.
I still feel like I need to read the tests more to understand what a Commit Tree is.

Refactored to make more readable
Factored out FindObjectishForTFSTempFile and added comments.

@StanleyGoldman StanleyGoldman merged commit dffdf45 into master Aug 4, 2018

2 checks passed

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

@StanleyGoldman StanleyGoldman deleted the feature/navigate-from-history-to-working-directory branch Aug 4, 2018

@meaghanlewis meaghanlewis added this to the 2.5.5 milestone Aug 6, 2018

@jcansdale

This comment has been minimized.

Copy link
Collaborator Author

jcansdale commented on src/GitHub.App/Services/GitHubContextService.cs in 3081e24 Nov 22, 2018

I think this should have been repo.Head.Commits.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.