From 8c55f6d83689a0cebc190e1c9e5a171e33962990 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 6 Mar 2017 12:48:25 +0000 Subject: [PATCH 1/4] Show compared branches as tooltip Make consistent with 'Team Explorer' diffs. Show diffed filename as caption with Diff - prefix. --- .../UI/Views/PullRequestDetailView.xaml.cs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs index 9bce7a980..33c255292 100644 --- a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs +++ b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs @@ -86,14 +86,16 @@ namespace GitHub.VisualStudio.UI.Views try { var fileNames = await ViewModel.ExtractDiffFiles(file); - var leftLabel = $"{file.FileName};{ViewModel.TargetBranchDisplayName}"; - var rightLabel = $"{file.FileName};PR {ViewModel.Model.Number}"; + var leftLabel = $"{file.FileName};{ViewModel.TargetBranchDisplayName}"; + var rightLabel = $"{file.FileName};PR {ViewModel.Model.Number}"; + var caption = $"Diff - {file.FileName}"; + var tooltip = $"{leftLabel}\nvs.\n{rightLabel}"; - Services.DifferenceService.OpenComparisonWindow2( + Services.DifferenceService.OpenComparisonWindow2( fileNames.Item1, fileNames.Item2, - $"{leftLabel} vs {rightLabel}", - file.DirectoryPath, + caption, + tooltip, leftLabel, rightLabel, string.Empty, From 437ae0973ed4aef878559c74ded11b0660b7f70d Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 21 Mar 2017 11:59:14 +0100 Subject: [PATCH 2/4] Only enable Open file when branch checked out. In the Pull Request Details view, disable the Open File context menu for files unless the PR branch is checked out and when opening a file, open the file in the working directory. Fixes #801 Fixes #817 --- .../PullRequestDetailViewModelDesigner.cs | 4 +- .../ViewModels/PullRequestDetailViewModel.cs | 38 ++++++++++++------- .../ViewModels/IPullRequestDetailViewModel.cs | 14 +++---- .../UI/Views/PullRequestDetailView.xaml | 2 +- .../UI/Views/PullRequestDetailView.xaml.cs | 11 ++---- 5 files changed, 38 insertions(+), 31 deletions(-) diff --git a/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs b/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs index a7f84838f..225b9c9a0 100644 --- a/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs +++ b/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs @@ -84,12 +84,12 @@ This requires that errors be propagated from the viewmodel to the view and from public ReactiveCommand OpenFile { get; } public ReactiveCommand DiffFile { get; } - public Task ExtractFile(IPullRequestFileNode file) + public Task> ExtractDiffFiles(IPullRequestFileNode file) { return null; } - public Task> ExtractDiffFiles(IPullRequestFileNode file) + public string GetLocalFilePath(IPullRequestFileNode file) { return null; } diff --git a/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs b/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs index e04cb3baf..51eaf1756 100644 --- a/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs @@ -39,6 +39,7 @@ namespace GitHub.ViewModels IPullRequestCheckoutState checkoutState; IPullRequestUpdateState updateState; string operationError; + bool isCheckedOut; bool isFromFork; bool isInCheckout; @@ -103,7 +104,7 @@ namespace GitHub.ViewModels SubscribeOperationError(Push); OpenOnGitHub = ReactiveCommand.Create(); - OpenFile = ReactiveCommand.Create(); + OpenFile = ReactiveCommand.Create(this.WhenAnyValue(x => x.IsCheckedOut)); DiffFile = ReactiveCommand.Create(); } @@ -145,6 +146,15 @@ namespace GitHub.ViewModels private set { this.RaiseAndSetIfChanged(ref targetBranchDisplayName, value); } } + /// + /// Gets a value indicating whether the pull request branch is checked out. + /// + public bool IsCheckedOut + { + get { return isCheckedOut; } + private set { this.RaiseAndSetIfChanged(ref isCheckedOut, value); } + } + /// /// Gets a value indicating whether the pull request comes from a fork. /// @@ -269,9 +279,10 @@ namespace GitHub.ViewModels } var localBranches = await pullRequestsService.GetLocalBranches(repository, pullRequest).ToList(); - var isCheckedOut = localBranches.Contains(repository.CurrentBranch); - if (isCheckedOut) + IsCheckedOut = localBranches.Contains(repository.CurrentBranch); + + if (IsCheckedOut) { var divergence = await pullRequestsService.CalculateHistoryDivergence(repository, Model.Number); var pullEnabled = divergence.BehindBy > 0; @@ -339,17 +350,6 @@ namespace GitHub.ViewModels } } - /// - /// Gets the specified file as it appears in the pull request. - /// - /// The file or directory node. - /// The path to the extracted file. - public Task ExtractFile(IPullRequestFileNode file) - { - var path = Path.Combine(file.DirectoryPath, file.FileName); - return pullRequestsService.ExtractFile(repository, modelService, model.Head.Sha, path, file.Sha).ToTask(); - } - /// /// Gets the before and after files needed for viewing a diff. /// @@ -361,6 +361,16 @@ namespace GitHub.ViewModels return pullRequestsService.ExtractDiffFiles(repository, modelService, model, path, file.Sha).ToTask(); } + /// + /// Gets the full path to a file in the working directory. + /// + /// The file. + /// The full path to the file in the working directory. + public string GetLocalFilePath(IPullRequestFileNode file) + { + return Path.Combine(repository.LocalPath, file.DirectoryPath, file.FileName); + } + void SubscribeOperationError(ReactiveCommand command) { command.ThrownExceptions.Subscribe(x => OperationError = x.Message); diff --git a/src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs b/src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs index 9e1fada96..bd90fcce8 100644 --- a/src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs +++ b/src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs @@ -139,18 +139,18 @@ namespace GitHub.ViewModels /// ReactiveCommand DiffFile { get; } - /// - /// Gets the specified file as it appears in the pull request. - /// - /// The file or directory node. - /// The path to the extracted file. - Task ExtractFile(IPullRequestFileNode file); - /// /// Gets the before and after files needed for viewing a diff. /// /// The changed file. /// A tuple containing the full path to the before and after files. Task> ExtractDiffFiles(IPullRequestFileNode file); + + /// + /// Gets the full path to a file in the working directory. + /// + /// The file. + /// The full path to the file in the working directory. + string GetLocalFilePath(IPullRequestFileNode file); } } diff --git a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml index d1d0e49b7..5f508e2f1 100644 --- a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml +++ b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml @@ -286,8 +286,8 @@ - + diff --git a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs index 33c255292..64a1fadc0 100644 --- a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs +++ b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs @@ -41,7 +41,7 @@ namespace GitHub.VisualStudio.UI.Views this.WhenActivated(d => { d(ViewModel.OpenOnGitHub.Subscribe(_ => DoOpenOnGitHub())); - d(ViewModel.OpenFile.Subscribe(x => DoOpenFile((IPullRequestFileNode)x).Forget())); + d(ViewModel.OpenFile.Subscribe(x => DoOpenFile((IPullRequestFileNode)x))); d(ViewModel.DiffFile.Subscribe(x => DoDiffFile((IPullRequestFileNode)x).Forget())); }); } @@ -65,15 +65,12 @@ namespace GitHub.VisualStudio.UI.Views browser.OpenUrl(url); } - async Task DoOpenFile(IPullRequestFileNode file) + void DoOpenFile(IPullRequestFileNode file) { try { - var fileName = await ViewModel.ExtractFile(file); - var window = Services.Dte.ItemOperations.OpenFile(fileName); - - // If the file we extracted isn't the current file on disk, make the window read-only. - window.Document.ReadOnly = fileName != file.DirectoryPath; + var fileName = ViewModel.GetLocalFilePath(file); + Services.Dte.ItemOperations.OpenFile(fileName); } catch (Exception e) { From 0d0ad94133d075fcd12e88353fabdd780883c5ef Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Tue, 21 Mar 2017 12:39:57 +0100 Subject: [PATCH 3/4] Make compare view editable. Make PR file compare view editable if PR branch is checked out. --- .../PullRequestDetailViewModelDesigner.cs | 1 + src/GitHub.App/Services/PullRequestService.cs | 7 +++-- .../ViewModels/PullRequestDetailViewModel.cs | 2 +- .../Services/IPullRequestService.cs | 9 +++++-- .../ViewModels/IPullRequestDetailViewModel.cs | 5 ++++ .../UI/Views/PullRequestDetailView.xaml.cs | 27 +++++++++++-------- 6 files changed, 35 insertions(+), 16 deletions(-) diff --git a/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs b/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs index 225b9c9a0..97bbd454a 100644 --- a/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs +++ b/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs @@ -70,6 +70,7 @@ This requires that errors be propagated from the viewmodel to the view and from public IPullRequestModel Model { get; } public string SourceBranchDisplayName { get; set; } public string TargetBranchDisplayName { get; set; } + public bool IsCheckedOut { get; } public bool IsFromFork { get; } public string Body { get; } public IReactiveList ChangedFilesTree { get; } diff --git a/src/GitHub.App/Services/PullRequestService.cs b/src/GitHub.App/Services/PullRequestService.cs index c054ddbc1..be7adb4fa 100644 --- a/src/GitHub.App/Services/PullRequestService.cs +++ b/src/GitHub.App/Services/PullRequestService.cs @@ -287,7 +287,8 @@ namespace GitHub.Services IModelService modelService, IPullRequestModel pullRequest, string fileName, - string fileSha) + string fileSha, + bool isPullRequestBranchCheckedOut) { return Observable.Defer(async () => { @@ -300,7 +301,9 @@ namespace GitHub.Services // The right file - if it comes from a fork - may not be fetched so fall back to // getting the file contents from the model service. - var right = await GetFileFromRepositoryOrApi(repository, repo, modelService, pullRequest.Head.Sha, fileName, fileSha); + var right = isPullRequestBranchCheckedOut ? + Path.Combine(repository.LocalPath, fileName) : + await GetFileFromRepositoryOrApi(repository, repo, modelService, pullRequest.Head.Sha, fileName, fileSha); if (left == null) { diff --git a/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs b/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs index 51eaf1756..7900b0070 100644 --- a/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs @@ -358,7 +358,7 @@ namespace GitHub.ViewModels public Task> ExtractDiffFiles(IPullRequestFileNode file) { var path = Path.Combine(file.DirectoryPath, file.FileName); - return pullRequestsService.ExtractDiffFiles(repository, modelService, model, path, file.Sha).ToTask(); + return pullRequestsService.ExtractDiffFiles(repository, modelService, model, path, file.Sha, IsCheckedOut).ToTask(); } /// diff --git a/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs b/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs index 0c7a8bd91..2cece4d4e 100644 --- a/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs +++ b/src/GitHub.Exports.Reactive/Services/IPullRequestService.cs @@ -120,13 +120,18 @@ namespace GitHub.Services /// The pull request details. /// The filename relative to the repository root. /// The SHA of the file in the pull request. - /// The filenames of the left and right files for the diff. + /// + /// Whether the pull request branch is currently checked out. If so the right file returned + /// will be the path to the file in the working directory. + /// + /// The paths of the left and right files for the diff. IObservable> ExtractDiffFiles( ILocalRepositoryModel repository, IModelService modelService, IPullRequestModel pullRequest, string fileName, - string fileSha); + string fileSha, + bool isPullRequestBranchCheckedOut); /// /// Remotes all unused remotes that were created by GitHub for Visual Studio to track PRs diff --git a/src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs b/src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs index bd90fcce8..233fd9aa3 100644 --- a/src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs +++ b/src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs @@ -79,6 +79,11 @@ namespace GitHub.ViewModels /// string TargetBranchDisplayName { get; } + /// + /// Gets a value indicating whether the pull request branch is checked out. + /// + bool IsCheckedOut { get; } + /// /// Gets a value indicating whether the pull request comes from a fork. /// diff --git a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs index 64a1fadc0..ff1c41c22 100644 --- a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs +++ b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs @@ -87,19 +87,24 @@ namespace GitHub.VisualStudio.UI.Views var rightLabel = $"{file.FileName};PR {ViewModel.Model.Number}"; var caption = $"Diff - {file.FileName}"; var tooltip = $"{leftLabel}\nvs.\n{rightLabel}"; + var options = __VSDIFFSERVICEOPTIONS.VSDIFFOPT_DetectBinaryFiles | + __VSDIFFSERVICEOPTIONS.VSDIFFOPT_LeftFileIsTemporary; + + if (!ViewModel.IsCheckedOut) + { + options |= __VSDIFFSERVICEOPTIONS.VSDIFFOPT_RightFileIsTemporary; + } Services.DifferenceService.OpenComparisonWindow2( - fileNames.Item1, - fileNames.Item2, - caption, - tooltip, - leftLabel, - rightLabel, - string.Empty, - string.Empty, - (int)(__VSDIFFSERVICEOPTIONS.VSDIFFOPT_DetectBinaryFiles | - __VSDIFFSERVICEOPTIONS.VSDIFFOPT_LeftFileIsTemporary | - __VSDIFFSERVICEOPTIONS.VSDIFFOPT_RightFileIsTemporary)); + fileNames.Item1, + fileNames.Item2, + caption, + tooltip, + leftLabel, + rightLabel, + string.Empty, + string.Empty, + (uint)options); } catch (Exception e) { From 8965fa1d41deebd4b5e29b10765c310025ef79e6 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 22 Mar 2017 12:13:35 +0000 Subject: [PATCH 4/4] Give GitHubVSAutomationIDs a unique project guid --- submodules/GitHubVSAutomationIDs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/submodules/GitHubVSAutomationIDs b/submodules/GitHubVSAutomationIDs index e3a4c2ed7..3105a88ca 160000 --- a/submodules/GitHubVSAutomationIDs +++ b/submodules/GitHubVSAutomationIDs @@ -1 +1 @@ -Subproject commit e3a4c2ed78980e4a2379716062e9e4ae04c44e8a +Subproject commit 3105a88caaef9b7059f3779843b8b2c8feb5390a