From 069e68221d810c3223b0cd754aad9b9d4ee342e8 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 9 Aug 2017 15:08:02 +0100 Subject: [PATCH 01/56] Add command to show the current PR --- .../Models/PullRequestDetailArgument.cs | 5 ++- .../PullRequestDetailViewModelDesigner.cs | 4 +- .../ViewModels/PullRequestDetailViewModel.cs | 20 +++++----- .../ViewModels/PullRequestListViewModel.cs | 2 +- .../ViewModels/IPullRequestDetailViewModel.cs | 4 +- src/GitHub.Exports/Settings/PkgCmdID.cs | 1 + .../GitHub.VisualStudio.csproj | 1 + .../GitHub.VisualStudio.vsct | 21 +++++++--- src/GitHub.VisualStudio/Menus/MenuProvider.cs | 3 +- .../Menus/ShowCurrentPullRequest.cs | 40 +++++++++++++++++++ .../UI/Views/PullRequestDetailView.xaml.cs | 12 ++++-- 11 files changed, 87 insertions(+), 26 deletions(-) create mode 100644 src/GitHub.VisualStudio/Menus/ShowCurrentPullRequest.cs diff --git a/src/GitHub.App/Models/PullRequestDetailArgument.cs b/src/GitHub.App/Models/PullRequestDetailArgument.cs index 4fa31ad910..3710e75a68 100644 --- a/src/GitHub.App/Models/PullRequestDetailArgument.cs +++ b/src/GitHub.App/Models/PullRequestDetailArgument.cs @@ -1,5 +1,6 @@ using System; using GitHub.ViewModels; +using GitHub.Primitives; namespace GitHub.Models { @@ -9,9 +10,9 @@ namespace GitHub.Models public class PullRequestDetailArgument { /// - /// Gets or sets the repository containing the pull request. + /// Gets or sets the owner of the repository containing the pull request. /// - public IRemoteRepositoryModel Repository { get; set; } + public string RepositoryOwner { get; set; } /// /// Gets or sets the number of the pull request. diff --git a/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs b/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs index 0888ca72a1..c94c5f02c8 100644 --- a/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs +++ b/src/GitHub.App/SampleData/PullRequestDetailViewModelDesigner.cs @@ -36,7 +36,7 @@ public PullRequestDetailViewModelDesigner() { var repoPath = @"C:\Repo"; - Model = new PullRequestModel(419, + Model = new PullRequestModel(419, "Error handling/bubbling from viewmodels to views to viewhosts", new AccountDesigner { Login = "shana", IsUser = true }, DateTime.Now.Subtract(TimeSpan.FromDays(3))) @@ -75,7 +75,7 @@ public PullRequestDetailViewModelDesigner() public IPullRequestModel Model { get; } public IPullRequestSession Session { get; } public ILocalRepositoryModel LocalRepository { get; } - public IRemoteRepositoryModel RemoteRepository { get; } + public string RemoteRepositoryOwner { get; } public string SourceBranchDisplayName { get; set; } public string TargetBranchDisplayName { get; set; } public int CommentCount { get; set; } diff --git a/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs b/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs index 95378c8597..94ca0ba54f 100644 --- a/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestDetailViewModel.cs @@ -148,13 +148,13 @@ private set public ILocalRepositoryModel LocalRepository { get; } /// - /// Gets the remote repository that contains the pull request. + /// Gets the owner of the remote repository that contains the pull request. /// /// /// The remote repository may be different from the local repository if the local /// repository is a fork and the user is viewing pull requests from the parent repository. /// - public IRemoteRepositoryModel RemoteRepository { get; private set; } + public string RemoteRepositoryOwner { get; private set; } /// /// Gets the session for the pull request. @@ -326,13 +326,13 @@ public IReadOnlyList ChangedFilesTree public override void Initialize(ViewWithData data) { int number; - var repo = RemoteRepository; + var repoOwner = RemoteRepositoryOwner; if (data != null) { var arg = (PullRequestDetailArgument)data.Data; number = arg.Number; - repo = arg.Repository; + repoOwner = arg.RepositoryOwner; } else { @@ -349,7 +349,7 @@ public override void Initialize(ViewWithData data) } ErrorMessage = OperationError = null; - modelService.GetPullRequest(repo, number) + modelService.GetPullRequest(repoOwner, LocalRepository.Name, number) .TakeLast(1) .ObserveOn(RxApp.MainThreadScheduler) .Catch(ex => @@ -359,23 +359,23 @@ public override void Initialize(ViewWithData data) IsLoading = IsBusy = false; return Observable.Empty(); }) - .Subscribe(x => Load(repo, x).Forget()); + .Subscribe(x => Load(repoOwner, x).Forget()); } /// /// Loads the view model from octokit models. /// - /// The remote repository. + /// The owner of the remote repository. /// The pull request model. - public async Task Load(IRemoteRepositoryModel remoteRepository, IPullRequestModel pullRequest) + public async Task Load(string remoteRepositoryOwner, IPullRequestModel pullRequest) { - Guard.ArgumentNotNull(remoteRepository, nameof(remoteRepository)); + Guard.ArgumentNotNull(remoteRepositoryOwner, nameof(remoteRepositoryOwner)); try { var firstLoad = (Model == null); Model = pullRequest; - RemoteRepository = remoteRepository; + RemoteRepositoryOwner = remoteRepositoryOwner; Session = await sessionManager.GetSession(pullRequest); Title = Resources.PullRequestNavigationItemText + " #" + pullRequest.Number; diff --git a/src/GitHub.App/ViewModels/PullRequestListViewModel.cs b/src/GitHub.App/ViewModels/PullRequestListViewModel.cs index bfeba84293..a9df06f266 100644 --- a/src/GitHub.App/ViewModels/PullRequestListViewModel.cs +++ b/src/GitHub.App/ViewModels/PullRequestListViewModel.cs @@ -321,7 +321,7 @@ void DoOpenPullRequest(object pullRequest) { Data = new PullRequestDetailArgument { - Repository = SelectedRepository, + RepositoryOwner = SelectedRepository.Owner, Number = (int)pullRequest, } }; diff --git a/src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs b/src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs index ff6926810f..5397b256a3 100644 --- a/src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs +++ b/src/GitHub.Exports.Reactive/ViewModels/IPullRequestDetailViewModel.cs @@ -83,13 +83,13 @@ public interface IPullRequestDetailViewModel : IViewModel, IHasLoading, IHasBusy ILocalRepositoryModel LocalRepository { get; } /// - /// Gets the remote repository that contains the pull request. + /// Gets the owner of the remote repository that contains the pull request. /// /// /// The remote repository may be different from the local repository if the local /// repository is a fork and the user is viewing pull requests from the parent repository. /// - IRemoteRepositoryModel RemoteRepository { get; } + string RemoteRepositoryOwner { get; } /// /// Gets a string describing how to display the pull request's source branch. diff --git a/src/GitHub.Exports/Settings/PkgCmdID.cs b/src/GitHub.Exports/Settings/PkgCmdID.cs index b41e29fdfe..1c8ba5d56e 100644 --- a/src/GitHub.Exports/Settings/PkgCmdID.cs +++ b/src/GitHub.Exports/Settings/PkgCmdID.cs @@ -9,6 +9,7 @@ public static class PkgCmdIDList public const int idGitHubToolbar = 0x1120; public const int showGitHubPaneCommand = 0x200; public const int openPullRequestsCommand = 0x201; + public const int showCurrentPullRequestCommand = 0x202; public const int backCommand = 0x300; public const int forwardCommand = 0x301; public const int refreshCommand = 0x302; diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index 34171454c4..1e6179b5a4 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -295,6 +295,7 @@ + diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.vsct b/src/GitHub.VisualStudio/GitHub.VisualStudio.vsct index 24fd2ab45c..8dc212ef03 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.vsct +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.vsct @@ -88,6 +88,16 @@ + + - + - + @@ -260,7 +270,7 @@ - + @@ -271,13 +281,14 @@ + - + @@ -313,7 +324,7 @@ - + diff --git a/src/GitHub.VisualStudio/Menus/MenuProvider.cs b/src/GitHub.VisualStudio/Menus/MenuProvider.cs index e1d2db07b9..0edf16514e 100644 --- a/src/GitHub.VisualStudio/Menus/MenuProvider.cs +++ b/src/GitHub.VisualStudio/Menus/MenuProvider.cs @@ -44,7 +44,8 @@ public MenuProvider(IGitHubServiceProvider serviceProvider) { new AddConnection(serviceProvider), new OpenPullRequests(serviceProvider), - new ShowGitHubPane(serviceProvider) + new ShowGitHubPane(serviceProvider), + new ShowCurrentPullRequest(serviceProvider) }; DynamicMenus = new List diff --git a/src/GitHub.VisualStudio/Menus/ShowCurrentPullRequest.cs b/src/GitHub.VisualStudio/Menus/ShowCurrentPullRequest.cs new file mode 100644 index 0000000000..058fc75c47 --- /dev/null +++ b/src/GitHub.VisualStudio/Menus/ShowCurrentPullRequest.cs @@ -0,0 +1,40 @@ +using System; +using GitHub.Exports; +using GitHub.UI; +using GitHub.Services; +using GitHub.Extensions; +using GitHub.Models; + +namespace GitHub.VisualStudio.Menus +{ + [ExportMenu(MenuType = MenuType.OpenPullRequests)] + public class ShowCurrentPullRequest : MenuBase, IMenuHandler + { + public ShowCurrentPullRequest(IGitHubServiceProvider serviceProvider) + : base(serviceProvider) + { + Guard.ArgumentNotNull(serviceProvider, nameof(serviceProvider)); + } + + public Guid Guid => Guids.guidGitHubCmdSet; + public int CmdId => PkgCmdIDList.showCurrentPullRequestCommand; + + public void Activate(object data = null) + { + var pullRequestSessionManager = ServiceProvider.ExportProvider.GetExportedValueOrDefault(); + var session = pullRequestSessionManager?.CurrentSession; + if (session == null) + { + return; // No active PR session. + } + + var pullRequest = session.PullRequest; + var arg = new PullRequestDetailArgument { RepositoryOwner = session.RepositoryOwner, Number = pullRequest.Number }; + var viewWithData = new ViewWithData(UIControllerFlow.PullRequestDetail) { Data = arg }; + + var manager = ServiceProvider.TryGetService(); + var host = manager.ShowHomePane(); + host?.ShowView(viewWithData); + } + } +} diff --git a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs index 12b5c5e471..242efef18f 100644 --- a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs +++ b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs @@ -25,8 +25,8 @@ using ReactiveUI; using Task = System.Threading.Tasks.Task; using Microsoft.VisualStudio.TextManager.Interop; -using System.Text; using Microsoft.VisualStudio.Text.Projection; +using System.Globalization; namespace GitHub.VisualStudio.UI.Views { @@ -75,12 +75,18 @@ protected override void OnVisualParentChanged(DependencyObject oldParent) void DoOpenOnGitHub() { - var repo = ViewModel.RemoteRepository; var browser = VisualStudioBrowser; - var url = repo.CloneUrl.ToRepositoryUrl().Append("pull/" + ViewModel.Model.Number); + var cloneUrl = ViewModel.LocalRepository.CloneUrl; + var url = ToPullRequestUrl(cloneUrl.Host, ViewModel.RemoteRepositoryOwner, ViewModel.LocalRepository.Name, ViewModel.Model.Number); browser.OpenUrl(url); } + static Uri ToPullRequestUrl(string host, string owner, string repositoryName, int number) + { + var url = string.Format(CultureInfo.InvariantCulture, "https://{0}/{1}/{2}/pull/{3}", host, owner, repositoryName, number); + return new Uri(url); + } + async Task DoOpenFile(IPullRequestFileNode file, bool workingDirectory) { try From c9d38dfa606c010e67f2ed9bba562e28dc7d8068 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 1 Aug 2017 15:13:49 +0100 Subject: [PATCH 02/56] Add PullRequestStatusView to SccStatusBar This isn't pretty but gets the UI control to where we want it. --- .../GitHub.InlineReviews.csproj | 9 ++ .../InlineCommentMarginProvider.cs | 10 +- .../Services/IPullRequestStatusManager.cs | 8 ++ .../Services/PullRequestStatusManager.cs | 117 ++++++++++++++++++ .../Views/PullRequestStatusView.xaml | 12 ++ .../Views/PullRequestStatusView.xaml.cs | 13 ++ 6 files changed, 167 insertions(+), 2 deletions(-) create mode 100644 src/GitHub.InlineReviews/Services/IPullRequestStatusManager.cs create mode 100644 src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs create mode 100644 src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml create mode 100644 src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml.cs diff --git a/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj b/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj index bcd33d6e52..a2cd7dd459 100644 --- a/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj +++ b/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj @@ -77,6 +77,8 @@ + + @@ -146,6 +148,9 @@ CommentView.xaml + + PullRequestStatusView.xaml + @@ -427,6 +432,10 @@ MSBuild:Compile Designer + + Designer + MSBuild:Compile + diff --git a/src/GitHub.InlineReviews/InlineCommentMarginProvider.cs b/src/GitHub.InlineReviews/InlineCommentMarginProvider.cs index 3dc3d7eabe..4f2cd15de8 100644 --- a/src/GitHub.InlineReviews/InlineCommentMarginProvider.cs +++ b/src/GitHub.InlineReviews/InlineCommentMarginProvider.cs @@ -33,6 +33,7 @@ internal sealed class InlineCommentMarginProvider : IWpfTextViewMarginProvider readonly IInlineCommentPeekService peekService; readonly IPullRequestSessionManager sessionManager; readonly IPackageSettings packageSettings; + readonly IPullRequestStatusManager pullRequestStatusManager; [ImportingConstructor] public InlineCommentMarginProvider( @@ -40,17 +41,22 @@ public InlineCommentMarginProvider( IViewTagAggregatorFactoryService tagAggregatorFactory, IInlineCommentPeekService peekService, IPullRequestSessionManager sessionManager, - IPackageSettings packageSettings) + IPackageSettings packageSettings, + IPullRequestStatusManager pullRequestStatusManager) { this.editorFormatMapService = editorFormatMapService; this.tagAggregatorFactory = tagAggregatorFactory; this.peekService = peekService; this.sessionManager = sessionManager; this.packageSettings = packageSettings; + this.pullRequestStatusManager = pullRequestStatusManager; } public IWpfTextViewMargin CreateMargin(IWpfTextViewHost wpfTextViewHost, IWpfTextViewMargin parent) { + // HACK: When should we show the PR status? + pullRequestStatusManager.ShowStatus(); + if (IsMarginDisabled(wpfTextViewHost)) { return null; @@ -71,7 +77,7 @@ IWpfTextViewMargin CreateMargin(IGlyphFactory glyphFactory var margin = new GlyphMargin(wpfTextViewHost, glyphFactory, gridFactory, tagAggregator, editorFormatMap, IsMarginVisible, MarginPropertiesName, MarginName, true, 17.0); - if(IsDiffView(wpfTextViewHost)) + if (IsDiffView(wpfTextViewHost)) { TrackCommentGlyph(wpfTextViewHost, margin.VisualElement); } diff --git a/src/GitHub.InlineReviews/Services/IPullRequestStatusManager.cs b/src/GitHub.InlineReviews/Services/IPullRequestStatusManager.cs new file mode 100644 index 0000000000..a10e27c650 --- /dev/null +++ b/src/GitHub.InlineReviews/Services/IPullRequestStatusManager.cs @@ -0,0 +1,8 @@ +namespace GitHub.InlineReviews.Services +{ + public interface IPullRequestStatusManager + { + void ShowStatus(); + void HideStatus(); + } +} diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs new file mode 100644 index 0000000000..e0186b6523 --- /dev/null +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs @@ -0,0 +1,117 @@ +using System; +using System.Windows; +using System.Windows.Media; +using System.Windows.Controls; +using System.Windows.Controls.Primitives; +using System.ComponentModel.Composition; +using GitHub.InlineReviews.Views; + +namespace GitHub.InlineReviews.Services +{ + + [Export(typeof(IPullRequestStatusManager))] + public class PullRequestStatusManager : IPullRequestStatusManager + { + const string SccStatusBarName = "SccStatusBar"; + const string GitHubStatusName = "GitHubStatusView"; + + readonly Window mainWindow; + + public PullRequestStatusManager() + { + mainWindow = Application.Current.MainWindow; + } + + public void ShowStatus() + { + var statusBar = FindSccStatusBar(); + if (statusBar != null) + { + var githubStatusBar = FindPullRequestStatusView(statusBar); + if (githubStatusBar == null) + { + var view = new PullRequestStatusView { Name = GitHubStatusName }; + statusBar.Items.Insert(0, view); + } + } + } + + public void HideStatus() + { + var statusBar = FindSccStatusBar(); + if (statusBar != null) + { + var githubStatusBar = FindPullRequestStatusView(statusBar); + if (githubStatusBar != null) + { + statusBar.Items.Remove(githubStatusBar); + } + } + } + + static UserControl FindPullRequestStatusView(StatusBar statusBar) + { + return FindChild(statusBar, GitHubStatusName); + } + + StatusBar FindSccStatusBar() + { + return FindChild(mainWindow, SccStatusBarName); + } + + // https://stackoverflow.com/questions/636383/how-can-i-find-wpf-controls-by-name-or-type + static T FindChild(DependencyObject parent, string childName) where T : DependencyObject + { + // Confirm parent and childName are valid. + if (parent == null) return null; + + T foundChild = null; + + int childrenCount = VisualTreeHelper.GetChildrenCount(parent); + for (int i = 0; i < childrenCount; i++) + { + var child = VisualTreeHelper.GetChild(parent, i); + // If the child is not of the request child type child + T childType = child as T; + if (childType == null) + { + // recursively drill down the tree + foundChild = FindChild(child, childName); + + // If the child is found, break so we do not overwrite the found child. + if (foundChild != null) break; + } + else if (!string.IsNullOrEmpty(childName)) + { + var frameworkElement = child as FrameworkElement; + // If the child's name is set for search + if (frameworkElement != null && frameworkElement.Name == childName) + { + // if the child's name is of the request name + foundChild = (T)child; + break; + } + } + else + { + // child element found. + foundChild = (T)child; + break; + } + } + + return foundChild; + } + + class PullRequestStatusManagerInstaller + { + [STAThread] + void Install() + { + var provider = new PullRequestStatusManager(); + provider.HideStatus(); + provider.ShowStatus(); + } + } + } +} diff --git a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml new file mode 100644 index 0000000000..d7ff203518 --- /dev/null +++ b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml @@ -0,0 +1,12 @@ + + + PR #777 + + diff --git a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml.cs b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml.cs new file mode 100644 index 0000000000..535830924a --- /dev/null +++ b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml.cs @@ -0,0 +1,13 @@ +using System; +using System.Windows.Controls; + +namespace GitHub.InlineReviews.Views +{ + public partial class PullRequestStatusView : UserControl + { + public PullRequestStatusView() + { + InitializeComponent(); + } + } +} From e6f55fdf760143198bdbe034d033f568bd902514 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 1 Aug 2017 15:40:12 +0100 Subject: [PATCH 03/56] Add a simple ViewModel for PullRequestStatusView --- src/GitHub.InlineReviews/GitHub.InlineReviews.csproj | 1 + .../Services/PullRequestStatusManager.cs | 4 +++- .../ViewModels/PullRequestStatusViewModel.cs | 10 ++++++++++ .../Views/PullRequestStatusView.xaml | 7 +++++-- 4 files changed, 19 insertions(+), 3 deletions(-) create mode 100644 src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs diff --git a/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj b/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj index a2cd7dd459..281a18da56 100644 --- a/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj +++ b/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj @@ -111,6 +111,7 @@ + DiffCommentThreadView.xaml diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs index e0186b6523..eada96771f 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs @@ -5,6 +5,7 @@ using System.Windows.Controls.Primitives; using System.ComponentModel.Composition; using GitHub.InlineReviews.Views; +using GitHub.InlineReviews.ViewModels; namespace GitHub.InlineReviews.Services { @@ -30,7 +31,8 @@ public void ShowStatus() var githubStatusBar = FindPullRequestStatusView(statusBar); if (githubStatusBar == null) { - var view = new PullRequestStatusView { Name = GitHubStatusName }; + var viewModel = new PullRequestStatusViewModel { Number = 666, Title = "A beast of a PR" }; + var view = new PullRequestStatusView { Name = GitHubStatusName, DataContext = viewModel }; statusBar.Items.Insert(0, view); } } diff --git a/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs b/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs new file mode 100644 index 0000000000..dc94dd269b --- /dev/null +++ b/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs @@ -0,0 +1,10 @@ +using System; + +namespace GitHub.InlineReviews.ViewModels +{ + class PullRequestStatusViewModel + { + public int Number { get; set; } + public string Title { get; set; } + } +} diff --git a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml index d7ff203518..2d77390468 100644 --- a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml +++ b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml @@ -5,8 +5,11 @@ xmlns:d="http://schemas.microsoft.com/expression/blend/2008" xmlns:local="clr-namespace:GitHub.InlineReviews.Views" mc:Ignorable="d" - d:DesignHeight="300" d:DesignWidth="300"> + d:DesignHeight="20" d:DesignWidth="60"> + + + - PR #777 + PR# From 8b6cfa2ddf3ec6721c3e2e76f31045dee94d1413 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 1 Aug 2017 16:19:03 +0100 Subject: [PATCH 04/56] Wire up property change events --- .../Services/PullRequestStatusManager.cs | 43 ++++++++++++++++--- .../ViewModels/PullRequestStatusViewModel.cs | 36 ++++++++++++++-- 2 files changed, 71 insertions(+), 8 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs index eada96771f..2582e30a4f 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs @@ -6,10 +6,11 @@ using System.ComponentModel.Composition; using GitHub.InlineReviews.Views; using GitHub.InlineReviews.ViewModels; +using GitHub.Services; +using System.ComponentModel; namespace GitHub.InlineReviews.Services { - [Export(typeof(IPullRequestStatusManager))] public class PullRequestStatusManager : IPullRequestStatusManager { @@ -17,9 +18,41 @@ public class PullRequestStatusManager : IPullRequestStatusManager const string GitHubStatusName = "GitHubStatusView"; readonly Window mainWindow; + readonly IPullRequestSessionManager pullRequestSessionManager; + readonly PullRequestStatusViewModel pullRequestStatusViewModel; + + [ImportingConstructor] + public PullRequestStatusManager(IPullRequestSessionManager pullRequestSessionManager) : this(new PullRequestStatusViewModel()) + { + this.pullRequestSessionManager = pullRequestSessionManager; + + RefreshCurrentSession(); + pullRequestSessionManager.PropertyChanged += PullRequestSessionManager_PropertyChanged; + } + + void PullRequestSessionManager_PropertyChanged(object sender, PropertyChangedEventArgs e) + { + if (e.PropertyName == nameof(PullRequestSessionManager.CurrentSession)) + { + RefreshCurrentSession(); + } + } + + void RefreshCurrentSession() + { + var currentSession = pullRequestSessionManager.CurrentSession; + var pullRequest = currentSession?.PullRequest; + + if (pullRequest != null) + { + pullRequestStatusViewModel.Number = pullRequest.Number; + pullRequestStatusViewModel.Title = pullRequest.Title; + } + } - public PullRequestStatusManager() + public PullRequestStatusManager(PullRequestStatusViewModel pullRequestStatusViewModel) { + this.pullRequestStatusViewModel = pullRequestStatusViewModel; mainWindow = Application.Current.MainWindow; } @@ -31,8 +64,7 @@ public void ShowStatus() var githubStatusBar = FindPullRequestStatusView(statusBar); if (githubStatusBar == null) { - var viewModel = new PullRequestStatusViewModel { Number = 666, Title = "A beast of a PR" }; - var view = new PullRequestStatusView { Name = GitHubStatusName, DataContext = viewModel }; + var view = new PullRequestStatusView { Name = GitHubStatusName, DataContext = pullRequestStatusViewModel }; statusBar.Items.Insert(0, view); } } @@ -110,7 +142,8 @@ class PullRequestStatusManagerInstaller [STAThread] void Install() { - var provider = new PullRequestStatusManager(); + var viewModel = new PullRequestStatusViewModel { Number = 666, Title = "A beast of a PR" }; + var provider = new PullRequestStatusManager(viewModel); provider.HideStatus(); provider.ShowStatus(); } diff --git a/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs b/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs index dc94dd269b..828c757ee2 100644 --- a/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs +++ b/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs @@ -1,10 +1,40 @@ using System; +using System.ComponentModel; namespace GitHub.InlineReviews.ViewModels { - class PullRequestStatusViewModel + public class PullRequestStatusViewModel : INotifyPropertyChanged { - public int Number { get; set; } - public string Title { get; set; } + int number; + string title; + + public int Number + { + get { return number; } + set + { + if (number != value) + { + number = value; + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Number))); + } + } + } + + + public string Title + { + get { return title; } + set + { + if (title != value) + { + title = value; + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Title))); + } + } + } + + public event PropertyChangedEventHandler PropertyChanged; } } From 91dcb3dc448abfe72411a71829c24f2ba98fdd9c Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 1 Aug 2017 17:04:08 +0100 Subject: [PATCH 05/56] Make status invisible when not on PR branch --- .../Services/PullRequestStatusManager.cs | 11 +++-------- .../ViewModels/PullRequestStatusViewModel.cs | 5 ++--- .../Views/PullRequestStatusView.xaml | 9 +++++---- 3 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs index 2582e30a4f..8668500c2e 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs @@ -40,14 +40,9 @@ void PullRequestSessionManager_PropertyChanged(object sender, PropertyChangedEve void RefreshCurrentSession() { - var currentSession = pullRequestSessionManager.CurrentSession; - var pullRequest = currentSession?.PullRequest; - - if (pullRequest != null) - { - pullRequestStatusViewModel.Number = pullRequest.Number; - pullRequestStatusViewModel.Title = pullRequest.Title; - } + var pullRequest = pullRequestSessionManager.CurrentSession?.PullRequest; + pullRequestStatusViewModel.Number = pullRequest?.Number; + pullRequestStatusViewModel.Title = pullRequest?.Title; } public PullRequestStatusManager(PullRequestStatusViewModel pullRequestStatusViewModel) diff --git a/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs b/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs index 828c757ee2..c1bb4baf22 100644 --- a/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs +++ b/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs @@ -5,10 +5,10 @@ namespace GitHub.InlineReviews.ViewModels { public class PullRequestStatusViewModel : INotifyPropertyChanged { - int number; + int? number; string title; - public int Number + public int? Number { get { return number; } set @@ -21,7 +21,6 @@ public int Number } } - public string Title { get { return title; } diff --git a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml index 2d77390468..5ebb865e89 100644 --- a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml +++ b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml @@ -6,10 +6,11 @@ xmlns:local="clr-namespace:GitHub.InlineReviews.Views" mc:Ignorable="d" d:DesignHeight="20" d:DesignWidth="60"> - - - - + + PR# + + + From a93a46774ca3d10ce94c49a28a77c21a1f664c19 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 9 Aug 2017 17:27:45 +0100 Subject: [PATCH 06/56] Open details view when PR status is clicked on --- .../Services/PullRequestStatusManager.cs | 80 ++++++++++++++----- .../ViewModels/PullRequestStatusViewModel.cs | 8 ++ .../Views/PullRequestStatusView.xaml | 4 +- 3 files changed, 73 insertions(+), 19 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs index 8668500c2e..db3c21be3d 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs @@ -1,13 +1,16 @@ using System; using System.Windows; using System.Windows.Media; +using System.Windows.Input; using System.Windows.Controls; using System.Windows.Controls.Primitives; +using System.ComponentModel; using System.ComponentModel.Composition; using GitHub.InlineReviews.Views; using GitHub.InlineReviews.ViewModels; using GitHub.Services; -using System.ComponentModel; +using GitHub.VisualStudio; +using Microsoft.VisualStudio.Shell; namespace GitHub.InlineReviews.Services { @@ -22,7 +25,8 @@ public class PullRequestStatusManager : IPullRequestStatusManager readonly PullRequestStatusViewModel pullRequestStatusViewModel; [ImportingConstructor] - public PullRequestStatusManager(IPullRequestSessionManager pullRequestSessionManager) : this(new PullRequestStatusViewModel()) + public PullRequestStatusManager(SVsServiceProvider serviceProvider, IPullRequestSessionManager pullRequestSessionManager) + : this(CreatePullRequestStatusViewModel(serviceProvider)) { this.pullRequestSessionManager = pullRequestSessionManager; @@ -30,21 +34,6 @@ public PullRequestStatusManager(IPullRequestSessionManager pullRequestSessionMan pullRequestSessionManager.PropertyChanged += PullRequestSessionManager_PropertyChanged; } - void PullRequestSessionManager_PropertyChanged(object sender, PropertyChangedEventArgs e) - { - if (e.PropertyName == nameof(PullRequestSessionManager.CurrentSession)) - { - RefreshCurrentSession(); - } - } - - void RefreshCurrentSession() - { - var pullRequest = pullRequestSessionManager.CurrentSession?.PullRequest; - pullRequestStatusViewModel.Number = pullRequest?.Number; - pullRequestStatusViewModel.Title = pullRequest?.Title; - } - public PullRequestStatusManager(PullRequestStatusViewModel pullRequestStatusViewModel) { this.pullRequestStatusViewModel = pullRequestStatusViewModel; @@ -78,6 +67,27 @@ public void HideStatus() } } + static PullRequestStatusViewModel CreatePullRequestStatusViewModel(IServiceProvider serviceProvider) + { + var command = new RaiseVsCommand(serviceProvider, Guids.guidGitHubCmdSetString, PkgCmdIDList.showCurrentPullRequestCommand); + return new PullRequestStatusViewModel(command); + } + + void PullRequestSessionManager_PropertyChanged(object sender, PropertyChangedEventArgs e) + { + if (e.PropertyName == nameof(PullRequestSessionManager.CurrentSession)) + { + RefreshCurrentSession(); + } + } + + void RefreshCurrentSession() + { + var pullRequest = pullRequestSessionManager.CurrentSession?.PullRequest; + pullRequestStatusViewModel.Number = pullRequest?.Number; + pullRequestStatusViewModel.Title = pullRequest?.Title; + } + static UserControl FindPullRequestStatusView(StatusBar statusBar) { return FindChild(statusBar, GitHubStatusName); @@ -132,12 +142,46 @@ static T FindChild(DependencyObject parent, string childName) where T : Depen return foundChild; } + class RaiseVsCommand : ICommand + { + readonly IServiceProvider serviceProvider; + readonly string guid; + readonly int id; + + internal RaiseVsCommand(IServiceProvider serviceProvider, string guid, int id) + { + this.serviceProvider = serviceProvider; + this.guid = guid; + this.id = id; + } + + public bool CanExecute(object parameter) => true; + + public void Execute(object parameter) + { + try + { + var dte = serviceProvider.GetService(typeof(EnvDTE.DTE)) as EnvDTE.DTE; + object customIn = null; + object customOut = null; + dte.Commands.Raise(guid, id, ref customIn, ref customOut); + } + catch (Exception e) + { + VsOutputLogger.WriteLine("Couldn't raise {0}: {1}", nameof(PkgCmdIDList.openPullRequestsCommand), e); + System.Diagnostics.Trace.WriteLine(e); + } + } + + public event EventHandler CanExecuteChanged; + } + class PullRequestStatusManagerInstaller { [STAThread] void Install() { - var viewModel = new PullRequestStatusViewModel { Number = 666, Title = "A beast of a PR" }; + var viewModel = new PullRequestStatusViewModel(null) { Number = 666, Title = "A beast of a PR" }; var provider = new PullRequestStatusManager(viewModel); provider.HideStatus(); provider.ShowStatus(); diff --git a/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs b/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs index c1bb4baf22..ee7b1e285e 100644 --- a/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs +++ b/src/GitHub.InlineReviews/ViewModels/PullRequestStatusViewModel.cs @@ -1,4 +1,5 @@ using System; +using System.Windows.Input; using System.ComponentModel; namespace GitHub.InlineReviews.ViewModels @@ -8,6 +9,11 @@ public class PullRequestStatusViewModel : INotifyPropertyChanged int? number; string title; + public PullRequestStatusViewModel(ICommand showCurrentPullRequestCommand) + { + ShowCurrentPullRequestCommand = showCurrentPullRequestCommand; + } + public int? Number { get { return number; } @@ -34,6 +40,8 @@ public string Title } } + public ICommand ShowCurrentPullRequestCommand { get; } + public event PropertyChangedEventHandler PropertyChanged; } } diff --git a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml index 5ebb865e89..98d48646f0 100644 --- a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml +++ b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml @@ -8,7 +8,9 @@ d:DesignHeight="20" d:DesignWidth="60"> - PR# + From 6d8c4418bad137ec08a91ea6b84fcd72cb8defcb Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 9 Aug 2017 18:05:31 +0100 Subject: [PATCH 07/56] Add hack to ensure that PR status appears --- .../Services/PullRequestStatusManager.cs | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs index db3c21be3d..a974c18cdd 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs @@ -46,11 +46,15 @@ public void ShowStatus() if (statusBar != null) { var githubStatusBar = FindPullRequestStatusView(statusBar); - if (githubStatusBar == null) + + // HACK: Insert every time to ensure that status shows up. + if (githubStatusBar != null) { - var view = new PullRequestStatusView { Name = GitHubStatusName, DataContext = pullRequestStatusViewModel }; - statusBar.Items.Insert(0, view); + statusBar.Items.Remove(githubStatusBar); } + + var view = new PullRequestStatusView { Name = GitHubStatusName, DataContext = pullRequestStatusViewModel }; + statusBar.Items.Insert(0, view); } } From 5f636063691b502c68269a4383354a80a9936696 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 29 Aug 2017 15:19:49 +0100 Subject: [PATCH 08/56] Restore missing using System.Globalization --- src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs index 242efef18f..c6feb9bc94 100644 --- a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs +++ b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs @@ -7,6 +7,7 @@ using System.Windows.Documents; using System.Windows.Input; using System.Windows.Media; +using System.Globalization; using GitHub.Exports; using GitHub.Extensions; using GitHub.InlineReviews.Commands; From 6f9efbda298722adfd9b9dd146f4bf30cd99bb60 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 29 Aug 2017 16:01:50 +0100 Subject: [PATCH 09/56] Find status bar part on main window --- .../Services/PullRequestStatusManager.cs | 72 +++++-------------- 1 file changed, 17 insertions(+), 55 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs index a974c18cdd..df072156d9 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs @@ -1,6 +1,5 @@ using System; using System.Windows; -using System.Windows.Media; using System.Windows.Input; using System.Windows.Controls; using System.Windows.Controls.Primitives; @@ -17,8 +16,7 @@ namespace GitHub.InlineReviews.Services [Export(typeof(IPullRequestStatusManager))] public class PullRequestStatusManager : IPullRequestStatusManager { - const string SccStatusBarName = "SccStatusBar"; - const string GitHubStatusName = "GitHubStatusView"; + const string StatusBarPartName = "PART_SccStatusBarHost"; readonly Window mainWindow; readonly IPullRequestSessionManager pullRequestSessionManager; @@ -45,16 +43,15 @@ public void ShowStatus() var statusBar = FindSccStatusBar(); if (statusBar != null) { - var githubStatusBar = FindPullRequestStatusView(statusBar); - - // HACK: Insert every time to ensure that status shows up. + var githubStatusBar = Find(statusBar); if (githubStatusBar != null) { + // Replace to ensure status shows up. statusBar.Items.Remove(githubStatusBar); } - var view = new PullRequestStatusView { Name = GitHubStatusName, DataContext = pullRequestStatusViewModel }; - statusBar.Items.Insert(0, view); + githubStatusBar = new PullRequestStatusView { DataContext = pullRequestStatusViewModel }; + statusBar.Items.Insert(0, githubStatusBar); } } @@ -63,7 +60,7 @@ public void HideStatus() var statusBar = FindSccStatusBar(); if (statusBar != null) { - var githubStatusBar = FindPullRequestStatusView(statusBar); + var githubStatusBar = Find(statusBar); if (githubStatusBar != null) { statusBar.Items.Remove(githubStatusBar); @@ -92,58 +89,23 @@ void RefreshCurrentSession() pullRequestStatusViewModel.Title = pullRequest?.Title; } - static UserControl FindPullRequestStatusView(StatusBar statusBar) - { - return FindChild(statusBar, GitHubStatusName); - } - - StatusBar FindSccStatusBar() - { - return FindChild(mainWindow, SccStatusBarName); - } - - // https://stackoverflow.com/questions/636383/how-can-i-find-wpf-controls-by-name-or-type - static T FindChild(DependencyObject parent, string childName) where T : DependencyObject + static T Find(StatusBar statusBar) { - // Confirm parent and childName are valid. - if (parent == null) return null; - - T foundChild = null; - - int childrenCount = VisualTreeHelper.GetChildrenCount(parent); - for (int i = 0; i < childrenCount; i++) + foreach (var item in statusBar.Items) { - var child = VisualTreeHelper.GetChild(parent, i); - // If the child is not of the request child type child - T childType = child as T; - if (childType == null) + if (item is T) { - // recursively drill down the tree - foundChild = FindChild(child, childName); - - // If the child is found, break so we do not overwrite the found child. - if (foundChild != null) break; - } - else if (!string.IsNullOrEmpty(childName)) - { - var frameworkElement = child as FrameworkElement; - // If the child's name is set for search - if (frameworkElement != null && frameworkElement.Name == childName) - { - // if the child's name is of the request name - foundChild = (T)child; - break; - } - } - else - { - // child element found. - foundChild = (T)child; - break; + return (T)item; } } - return foundChild; + return default(T); + } + + StatusBar FindSccStatusBar() + { + var contentControl = mainWindow?.Template?.FindName(StatusBarPartName, mainWindow) as ContentControl; + return contentControl?.Content as StatusBar; } class RaiseVsCommand : ICommand From c5f7cf5ecad4710b1655cddf680312e4fdf57b5f Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 29 Aug 2017 16:36:34 +0100 Subject: [PATCH 10/56] Show PR status after solution has loaded Initialize with InlineReviewsPackage. This should probably initialize earlier from its own package. --- .../InlineCommentMarginProvider.cs | 8 +------- src/GitHub.InlineReviews/InlineReviewsPackage.cs | 13 ++++++++++++- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/GitHub.InlineReviews/InlineCommentMarginProvider.cs b/src/GitHub.InlineReviews/InlineCommentMarginProvider.cs index 4f2cd15de8..1029fa1acd 100644 --- a/src/GitHub.InlineReviews/InlineCommentMarginProvider.cs +++ b/src/GitHub.InlineReviews/InlineCommentMarginProvider.cs @@ -33,7 +33,6 @@ internal sealed class InlineCommentMarginProvider : IWpfTextViewMarginProvider readonly IInlineCommentPeekService peekService; readonly IPullRequestSessionManager sessionManager; readonly IPackageSettings packageSettings; - readonly IPullRequestStatusManager pullRequestStatusManager; [ImportingConstructor] public InlineCommentMarginProvider( @@ -41,22 +40,17 @@ public InlineCommentMarginProvider( IViewTagAggregatorFactoryService tagAggregatorFactory, IInlineCommentPeekService peekService, IPullRequestSessionManager sessionManager, - IPackageSettings packageSettings, - IPullRequestStatusManager pullRequestStatusManager) + IPackageSettings packageSettings) { this.editorFormatMapService = editorFormatMapService; this.tagAggregatorFactory = tagAggregatorFactory; this.peekService = peekService; this.sessionManager = sessionManager; this.packageSettings = packageSettings; - this.pullRequestStatusManager = pullRequestStatusManager; } public IWpfTextViewMargin CreateMargin(IWpfTextViewHost wpfTextViewHost, IWpfTextViewMargin parent) { - // HACK: When should we show the PR status? - pullRequestStatusManager.ShowStatus(); - if (IsMarginDisabled(wpfTextViewHost)) { return null; diff --git a/src/GitHub.InlineReviews/InlineReviewsPackage.cs b/src/GitHub.InlineReviews/InlineReviewsPackage.cs index 9271fbd299..5864ec08bc 100644 --- a/src/GitHub.InlineReviews/InlineReviewsPackage.cs +++ b/src/GitHub.InlineReviews/InlineReviewsPackage.cs @@ -5,6 +5,8 @@ using GitHub.VisualStudio; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Shell.Interop; +using Microsoft.VisualStudio.ComponentModelHost; +using GitHub.InlineReviews.Services; namespace GitHub.InlineReviews { @@ -12,13 +14,22 @@ namespace GitHub.InlineReviews [Guid(Guids.InlineReviewsPackageId)] [ProvideAutoLoad(UIContextGuids80.SolutionExists)] [ProvideMenuResource("Menus.ctmenu", 1)] - [ProvideToolWindow(typeof(PullRequestCommentsPane), DocumentLikeTool=true)] + [ProvideToolWindow(typeof(PullRequestCommentsPane), DocumentLikeTool = true)] public class InlineReviewsPackage : Package { protected override void Initialize() { base.Initialize(); PackageResources.Register(this); + ShowPullRequestStatus(); + } + + void ShowPullRequestStatus() + { + var componentModel = (IComponentModel)GetService(typeof(SComponentModel)); + var exportProvider = componentModel.DefaultExportProvider; + var pullRequestStatusManager = exportProvider.GetExportedValue(); + pullRequestStatusManager.ShowStatus(); } } } From 63627908c99931aeadfbe502ef02f9ea4d79bd2f Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 12 Sep 2017 17:48:04 +0100 Subject: [PATCH 11/56] Fix duplicate using --- src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs index c6feb9bc94..d204d1386c 100644 --- a/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs +++ b/src/GitHub.VisualStudio/UI/Views/PullRequestDetailView.xaml.cs @@ -27,7 +27,6 @@ using Task = System.Threading.Tasks.Task; using Microsoft.VisualStudio.TextManager.Interop; using Microsoft.VisualStudio.Text.Projection; -using System.Globalization; namespace GitHub.VisualStudio.UI.Views { From 11b675a84c2897c7e509ea140793172e2274357c Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 12 Sep 2017 18:05:08 +0100 Subject: [PATCH 12/56] Move PullRequestStatus initialization to own package --- src/GitHub.Exports/Settings/Guids.cs | 1 + .../GitHub.InlineReviews.csproj | 1 + .../InlineReviewsPackage.cs | 11 --------- .../PullRequestStatusPackage.cs | 24 +++++++++++++++++++ 4 files changed, 26 insertions(+), 11 deletions(-) create mode 100644 src/GitHub.InlineReviews/PullRequestStatusPackage.cs diff --git a/src/GitHub.Exports/Settings/Guids.cs b/src/GitHub.Exports/Settings/Guids.cs index 2e225c7038..81562cdaff 100644 --- a/src/GitHub.Exports/Settings/Guids.cs +++ b/src/GitHub.Exports/Settings/Guids.cs @@ -13,6 +13,7 @@ public static class Guids public const string StartPagePackageId = "3b764d23-faf7-486f-94c7-b3accc44a70e"; public const string CodeContainerProviderId = "6CE146CB-EF57-4F2C-A93F-5BA685317660"; public const string InlineReviewsPackageId = "248325BE-4A2D-4111-B122-E7D59BF73A35"; + public const string PullRequestStatusPackageId = "5121BEC6-1088-4553-8453-0DDC7C8E2238"; public const string TeamExplorerWelcomeMessage = "C529627F-8AA6-4FDB-82EB-4BFB7DB753C3"; public const string LoginManagerId = "7BA2071A-790A-4F95-BE4A-0EEAA5928AAF"; diff --git a/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj b/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj index 281a18da56..ec92ae0531 100644 --- a/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj +++ b/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj @@ -73,6 +73,7 @@ + diff --git a/src/GitHub.InlineReviews/InlineReviewsPackage.cs b/src/GitHub.InlineReviews/InlineReviewsPackage.cs index 5864ec08bc..5d06a63d87 100644 --- a/src/GitHub.InlineReviews/InlineReviewsPackage.cs +++ b/src/GitHub.InlineReviews/InlineReviewsPackage.cs @@ -5,8 +5,6 @@ using GitHub.VisualStudio; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Shell.Interop; -using Microsoft.VisualStudio.ComponentModelHost; -using GitHub.InlineReviews.Services; namespace GitHub.InlineReviews { @@ -21,15 +19,6 @@ protected override void Initialize() { base.Initialize(); PackageResources.Register(this); - ShowPullRequestStatus(); - } - - void ShowPullRequestStatus() - { - var componentModel = (IComponentModel)GetService(typeof(SComponentModel)); - var exportProvider = componentModel.DefaultExportProvider; - var pullRequestStatusManager = exportProvider.GetExportedValue(); - pullRequestStatusManager.ShowStatus(); } } } diff --git a/src/GitHub.InlineReviews/PullRequestStatusPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusPackage.cs new file mode 100644 index 0000000000..b1bef99240 --- /dev/null +++ b/src/GitHub.InlineReviews/PullRequestStatusPackage.cs @@ -0,0 +1,24 @@ +using System; +using System.Runtime.InteropServices; +using Microsoft.VisualStudio.Shell; +using Microsoft.VisualStudio.Shell.Interop; +using Microsoft.VisualStudio.ComponentModelHost; +using GitHub.VisualStudio; +using GitHub.InlineReviews.Services; + +namespace GitHub.InlineReviews +{ + [PackageRegistration(UseManagedResourcesOnly = true)] + [Guid(Guids.PullRequestStatusPackageId)] + [ProvideAutoLoad(UIContextGuids80.SolutionExists)] + public class PullRequestStatusPackage : Package + { + protected override void Initialize() + { + var componentModel = (IComponentModel)GetService(typeof(SComponentModel)); + var exportProvider = componentModel.DefaultExportProvider; + var pullRequestStatusManager = exportProvider.GetExportedValue(); + pullRequestStatusManager.ShowStatus(); + } + } +} From c6ee258d5f9c59b694d4851a4bd8d849fae04e5c Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 12 Sep 2017 18:42:19 +0100 Subject: [PATCH 13/56] Only create PR status view when session changes Remove view when there is no PR session. --- .../PullRequestStatusPackage.cs | 2 +- .../Services/IPullRequestStatusManager.cs | 3 +- .../Services/PullRequestStatusManager.cs | 87 ++++++++----------- 3 files changed, 38 insertions(+), 54 deletions(-) diff --git a/src/GitHub.InlineReviews/PullRequestStatusPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusPackage.cs index b1bef99240..bf9caf9075 100644 --- a/src/GitHub.InlineReviews/PullRequestStatusPackage.cs +++ b/src/GitHub.InlineReviews/PullRequestStatusPackage.cs @@ -18,7 +18,7 @@ protected override void Initialize() var componentModel = (IComponentModel)GetService(typeof(SComponentModel)); var exportProvider = componentModel.DefaultExportProvider; var pullRequestStatusManager = exportProvider.GetExportedValue(); - pullRequestStatusManager.ShowStatus(); + pullRequestStatusManager.Initialize(); } } } diff --git a/src/GitHub.InlineReviews/Services/IPullRequestStatusManager.cs b/src/GitHub.InlineReviews/Services/IPullRequestStatusManager.cs index a10e27c650..d355b68312 100644 --- a/src/GitHub.InlineReviews/Services/IPullRequestStatusManager.cs +++ b/src/GitHub.InlineReviews/Services/IPullRequestStatusManager.cs @@ -2,7 +2,6 @@ { public interface IPullRequestStatusManager { - void ShowStatus(); - void HideStatus(); + void Initialize(); } } diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs index df072156d9..da7c1cf257 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs @@ -10,6 +10,7 @@ using GitHub.Services; using GitHub.VisualStudio; using Microsoft.VisualStudio.Shell; +using GitHub.Models; namespace GitHub.InlineReviews.Services { @@ -18,44 +19,54 @@ public class PullRequestStatusManager : IPullRequestStatusManager { const string StatusBarPartName = "PART_SccStatusBarHost"; + readonly SVsServiceProvider serviceProvider; readonly Window mainWindow; readonly IPullRequestSessionManager pullRequestSessionManager; - readonly PullRequestStatusViewModel pullRequestStatusViewModel; [ImportingConstructor] public PullRequestStatusManager(SVsServiceProvider serviceProvider, IPullRequestSessionManager pullRequestSessionManager) - : this(CreatePullRequestStatusViewModel(serviceProvider)) + : this() { + this.serviceProvider = serviceProvider; this.pullRequestSessionManager = pullRequestSessionManager; + } + public PullRequestStatusManager() + { + mainWindow = Application.Current.MainWindow; + } + + public void Initialize() + { RefreshCurrentSession(); pullRequestSessionManager.PropertyChanged += PullRequestSessionManager_PropertyChanged; } - public PullRequestStatusManager(PullRequestStatusViewModel pullRequestStatusViewModel) + void PullRequestSessionManager_PropertyChanged(object sender, PropertyChangedEventArgs e) { - this.pullRequestStatusViewModel = pullRequestStatusViewModel; - mainWindow = Application.Current.MainWindow; + if (e.PropertyName == nameof(PullRequestSessionManager.CurrentSession)) + { + RefreshCurrentSession(); + } } - public void ShowStatus() + void RefreshCurrentSession() { - var statusBar = FindSccStatusBar(); - if (statusBar != null) - { - var githubStatusBar = Find(statusBar); - if (githubStatusBar != null) - { - // Replace to ensure status shows up. - statusBar.Items.Remove(githubStatusBar); - } + var pullRequest = pullRequestSessionManager.CurrentSession?.PullRequest; + var viewModel = pullRequest != null ? CreatePullRequestStatusViewModel(serviceProvider, pullRequest) : null; + ShowStatus(viewModel); + } - githubStatusBar = new PullRequestStatusView { DataContext = pullRequestStatusViewModel }; - statusBar.Items.Insert(0, githubStatusBar); - } + static PullRequestStatusViewModel CreatePullRequestStatusViewModel(IServiceProvider serviceProvider, IPullRequestModel pullRequest) + { + var command = new RaiseVsCommand(serviceProvider, Guids.guidGitHubCmdSetString, PkgCmdIDList.showCurrentPullRequestCommand); + var pullRequestStatusViewModel = new PullRequestStatusViewModel(command); + pullRequestStatusViewModel.Number = pullRequest.Number; + pullRequestStatusViewModel.Title = pullRequest.Title; + return pullRequestStatusViewModel; } - public void HideStatus() + void ShowStatus(PullRequestStatusViewModel pullRequestStatusViewModel = null) { var statusBar = FindSccStatusBar(); if (statusBar != null) @@ -63,32 +74,18 @@ public void HideStatus() var githubStatusBar = Find(statusBar); if (githubStatusBar != null) { + // Replace to ensure status shows up. statusBar.Items.Remove(githubStatusBar); } - } - } - static PullRequestStatusViewModel CreatePullRequestStatusViewModel(IServiceProvider serviceProvider) - { - var command = new RaiseVsCommand(serviceProvider, Guids.guidGitHubCmdSetString, PkgCmdIDList.showCurrentPullRequestCommand); - return new PullRequestStatusViewModel(command); - } - - void PullRequestSessionManager_PropertyChanged(object sender, PropertyChangedEventArgs e) - { - if (e.PropertyName == nameof(PullRequestSessionManager.CurrentSession)) - { - RefreshCurrentSession(); + if (pullRequestStatusViewModel != null) + { + githubStatusBar = new PullRequestStatusView { DataContext = pullRequestStatusViewModel }; + statusBar.Items.Insert(0, githubStatusBar); + } } } - void RefreshCurrentSession() - { - var pullRequest = pullRequestSessionManager.CurrentSession?.PullRequest; - pullRequestStatusViewModel.Number = pullRequest?.Number; - pullRequestStatusViewModel.Title = pullRequest?.Title; - } - static T Find(StatusBar statusBar) { foreach (var item in statusBar.Items) @@ -141,17 +138,5 @@ public void Execute(object parameter) public event EventHandler CanExecuteChanged; } - - class PullRequestStatusManagerInstaller - { - [STAThread] - void Install() - { - var viewModel = new PullRequestStatusViewModel(null) { Number = 666, Title = "A beast of a PR" }; - var provider = new PullRequestStatusManager(viewModel); - provider.HideStatus(); - provider.ShowStatus(); - } - } } } From 4f0da47b17cee3209f667f95f92a8a0a1cbf1ab8 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 12 Dec 2017 12:48:11 +0000 Subject: [PATCH 14/56] Use Serilog and fix CA error --- .../Services/PullRequestStatusManager.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs index da7c1cf257..d2d3f118a9 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs @@ -11,12 +11,15 @@ using GitHub.VisualStudio; using Microsoft.VisualStudio.Shell; using GitHub.Models; +using GitHub.Logging; +using Serilog; namespace GitHub.InlineReviews.Services { [Export(typeof(IPullRequestStatusManager))] public class PullRequestStatusManager : IPullRequestStatusManager { + static readonly ILogger log = LogManager.ForContext(); const string StatusBarPartName = "PART_SccStatusBarHost"; readonly SVsServiceProvider serviceProvider; @@ -131,12 +134,16 @@ public void Execute(object parameter) } catch (Exception e) { - VsOutputLogger.WriteLine("Couldn't raise {0}: {1}", nameof(PkgCmdIDList.openPullRequestsCommand), e); + log.Error(e, "Couldn't raise {Guid}:{ID}", guid, id); System.Diagnostics.Trace.WriteLine(e); } } - public event EventHandler CanExecuteChanged; + public event EventHandler CanExecuteChanged + { + add { } + remove { } + } } } } From e2d753cc0b6d16b5896ae0e5b341cd79afd2dce2 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 18 Dec 2017 16:23:49 +0000 Subject: [PATCH 15/56] Add usage metrics for ShowCurrentPullRequest --- src/GitHub.Exports/Models/UsageModel.cs | 1 + .../Services/PullRequestStatusManager.cs | 29 +++++++++++-------- 2 files changed, 18 insertions(+), 12 deletions(-) diff --git a/src/GitHub.Exports/Models/UsageModel.cs b/src/GitHub.Exports/Models/UsageModel.cs index 79deaf1b01..4da8cc33d7 100644 --- a/src/GitHub.Exports/Models/UsageModel.cs +++ b/src/GitHub.Exports/Models/UsageModel.cs @@ -37,6 +37,7 @@ public class UsageModel public int NumberOfPRDetailsOpenFileInSolution { get; set; } public int NumberOfPRReviewDiffViewInlineCommentOpen { get; set; } public int NumberOfPRReviewDiffViewInlineCommentPost { get; set; } + public int NumberOfShowCurrentPullRequest { get; set; } public UsageModel Clone(bool includeWeekly, bool includeMonthly) { diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs index d2d3f118a9..e53c704b98 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusManager.cs @@ -9,9 +9,10 @@ using GitHub.InlineReviews.ViewModels; using GitHub.Services; using GitHub.VisualStudio; -using Microsoft.VisualStudio.Shell; using GitHub.Models; using GitHub.Logging; +using GitHub.Extensions; +using Microsoft.VisualStudio.Shell; using Serilog; namespace GitHub.InlineReviews.Services @@ -25,13 +26,15 @@ public class PullRequestStatusManager : IPullRequestStatusManager readonly SVsServiceProvider serviceProvider; readonly Window mainWindow; readonly IPullRequestSessionManager pullRequestSessionManager; + readonly IUsageTracker usageTracker; [ImportingConstructor] - public PullRequestStatusManager(SVsServiceProvider serviceProvider, IPullRequestSessionManager pullRequestSessionManager) + public PullRequestStatusManager(SVsServiceProvider serviceProvider, IPullRequestSessionManager pullRequestSessionManager, IUsageTracker usageTracker) : this() { this.serviceProvider = serviceProvider; this.pullRequestSessionManager = pullRequestSessionManager; + this.usageTracker = usageTracker; } public PullRequestStatusManager() @@ -56,13 +59,13 @@ void PullRequestSessionManager_PropertyChanged(object sender, PropertyChangedEve void RefreshCurrentSession() { var pullRequest = pullRequestSessionManager.CurrentSession?.PullRequest; - var viewModel = pullRequest != null ? CreatePullRequestStatusViewModel(serviceProvider, pullRequest) : null; + var viewModel = pullRequest != null ? CreatePullRequestStatusViewModel(pullRequest) : null; ShowStatus(viewModel); } - static PullRequestStatusViewModel CreatePullRequestStatusViewModel(IServiceProvider serviceProvider, IPullRequestModel pullRequest) + PullRequestStatusViewModel CreatePullRequestStatusViewModel(IPullRequestModel pullRequest) { - var command = new RaiseVsCommand(serviceProvider, Guids.guidGitHubCmdSetString, PkgCmdIDList.showCurrentPullRequestCommand); + var command = new RaisePullRequestCommand(serviceProvider, usageTracker); var pullRequestStatusViewModel = new PullRequestStatusViewModel(command); pullRequestStatusViewModel.Number = pullRequest.Number; pullRequestStatusViewModel.Title = pullRequest.Title; @@ -108,17 +111,18 @@ StatusBar FindSccStatusBar() return contentControl?.Content as StatusBar; } - class RaiseVsCommand : ICommand + class RaisePullRequestCommand : ICommand { + readonly string guid = Guids.guidGitHubCmdSetString; + readonly int id = PkgCmdIDList.showCurrentPullRequestCommand; + readonly IServiceProvider serviceProvider; - readonly string guid; - readonly int id; + readonly IUsageTracker usageTracker; - internal RaiseVsCommand(IServiceProvider serviceProvider, string guid, int id) + internal RaisePullRequestCommand(IServiceProvider serviceProvider, IUsageTracker usageTracker) { this.serviceProvider = serviceProvider; - this.guid = guid; - this.id = id; + this.usageTracker = usageTracker; } public bool CanExecute(object parameter) => true; @@ -135,8 +139,9 @@ public void Execute(object parameter) catch (Exception e) { log.Error(e, "Couldn't raise {Guid}:{ID}", guid, id); - System.Diagnostics.Trace.WriteLine(e); } + + usageTracker.IncrementCounter(x => x.NumberOfShowCurrentPullRequest).Forget(); } public event EventHandler CanExecuteChanged From f4b3115bc3e5da71ce8511862a2d145317033fa6 Mon Sep 17 00:00:00 2001 From: Don Okuda Date: Mon, 18 Dec 2017 20:06:59 -0800 Subject: [PATCH 16/56] Start a button --- .../Views/PullRequestStatusView.xaml | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml index 98d48646f0..079ac0f575 100644 --- a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml +++ b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml @@ -3,13 +3,27 @@ xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml" xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" xmlns:d="http://schemas.microsoft.com/expression/blend/2008" + xmlns:ui="clr-namespace:GitHub.UI;assembly=GitHub.UI" xmlns:local="clr-namespace:GitHub.InlineReviews.Views" mc:Ignorable="d" d:DesignHeight="20" d:DesignWidth="60"> - From 8e26aadb755d78287bed112239a2899757ba4bdf Mon Sep 17 00:00:00 2001 From: Don Okuda Date: Mon, 18 Dec 2017 23:44:31 -0800 Subject: [PATCH 17/56] Use visual studio color keys --- .../Views/PullRequestStatusView.xaml | 22 ++++++++++--------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml index 079ac0f575..da2855950d 100644 --- a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml +++ b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml @@ -4,26 +4,28 @@ xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" xmlns:d="http://schemas.microsoft.com/expression/blend/2008" xmlns:ui="clr-namespace:GitHub.UI;assembly=GitHub.UI" + xmlns:vsshell="clr-namespace:Microsoft.VisualStudio.Shell;assembly=Microsoft.VisualStudio.Shell.10.0" xmlns:local="clr-namespace:GitHub.InlineReviews.Views" mc:Ignorable="d" d:DesignHeight="20" d:DesignWidth="60"> From 7e1bcc021e3a130f098af9e69ec8aaf9182d6b73 Mon Sep 17 00:00:00 2001 From: Don Okuda Date: Thu, 21 Dec 2017 10:29:11 -0800 Subject: [PATCH 18/56] Set up some triggers --- .../Views/PullRequestStatusView.xaml | 27 ++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml index da2855950d..baa68f7b1a 100644 --- a/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml +++ b/src/GitHub.InlineReviews/Views/PullRequestStatusView.xaml @@ -9,12 +9,37 @@ mc:Ignorable="d" d:DesignHeight="20" d:DesignWidth="60"> + + + + - - + + # - + From e9f0c17c47d8ddc5425f300349ae66d17713f974 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 9 Feb 2018 15:05:17 +0000 Subject: [PATCH 26/56] Convert PullRequestStatusPackage to an AsyncPackage --- src/GitHub.InlineReviews/PullRequestStatusPackage.cs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/GitHub.InlineReviews/PullRequestStatusPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusPackage.cs index ce7eb0f3d6..36a40a8069 100644 --- a/src/GitHub.InlineReviews/PullRequestStatusPackage.cs +++ b/src/GitHub.InlineReviews/PullRequestStatusPackage.cs @@ -1,20 +1,22 @@ using System; +using System.Threading; using System.Runtime.InteropServices; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.ComponentModelHost; using GitHub.VisualStudio; using GitHub.InlineReviews.Services; +using Task = System.Threading.Tasks.Task; namespace GitHub.InlineReviews { [PackageRegistration(UseManagedResourcesOnly = true)] [Guid(Guids.PullRequestStatusPackageId)] [ProvideAutoLoad(Guids.GitSccProviderId)] - public class PullRequestStatusPackage : Package + public class PullRequestStatusPackage : AsyncPackage { - protected override void Initialize() + protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) { - var componentModel = (IComponentModel)GetService(typeof(SComponentModel)); + var componentModel = (IComponentModel)await GetServiceAsync(typeof(SComponentModel)); var exportProvider = componentModel.DefaultExportProvider; var pullRequestStatusManager = exportProvider.GetExportedValue(); pullRequestStatusManager.Initialize(); From c6750b950d5c7bc9714b7597605ada1cc368694b Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 9 Feb 2018 15:11:53 +0000 Subject: [PATCH 27/56] Rename PullRequestStatusPackage to ..StatusBarPackage --- src/GitHub.InlineReviews/GitHub.InlineReviews.csproj | 2 +- ...llRequestStatusPackage.cs => PullRequestStatusBarPackage.cs} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename src/GitHub.InlineReviews/{PullRequestStatusPackage.cs => PullRequestStatusBarPackage.cs} (94%) diff --git a/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj b/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj index a90adfac4c..a555e10c8b 100644 --- a/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj +++ b/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj @@ -88,7 +88,7 @@ - + diff --git a/src/GitHub.InlineReviews/PullRequestStatusPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs similarity index 94% rename from src/GitHub.InlineReviews/PullRequestStatusPackage.cs rename to src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs index 36a40a8069..6d9d00117e 100644 --- a/src/GitHub.InlineReviews/PullRequestStatusPackage.cs +++ b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs @@ -12,7 +12,7 @@ namespace GitHub.InlineReviews [PackageRegistration(UseManagedResourcesOnly = true)] [Guid(Guids.PullRequestStatusPackageId)] [ProvideAutoLoad(Guids.GitSccProviderId)] - public class PullRequestStatusPackage : AsyncPackage + public class PullRequestStatusBarPackage : AsyncPackage { protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) { From dc81286f813b48260575913b629964a890c4d23f Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 9 Feb 2018 15:44:36 +0000 Subject: [PATCH 28/56] Lazily initialize when we're on a GitHub repo Initialize when an active repository appears in IVSGitExt. --- .../Services/PullRequestStatusBarManager.cs | 49 ++++++++++++++----- 1 file changed, 38 insertions(+), 11 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index 7f6435374d..c791219370 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -11,8 +11,10 @@ using GitHub.VisualStudio; using GitHub.Models; using GitHub.Logging; +using GitHub.Helpers; using GitHub.Extensions; using Serilog; +using Task = System.Threading.Tasks.Task; namespace GitHub.InlineReviews.Services { @@ -22,29 +24,54 @@ public class PullRequestStatusBarManager : IPullRequestStatusBarManager static readonly ILogger log = LogManager.ForContext(); const string StatusBarPartName = "PART_SccStatusBarHost"; - readonly IGitHubServiceProvider serviceProvider; + readonly IVSGitExt gitExt; readonly Window mainWindow; - readonly IPullRequestSessionManager pullRequestSessionManager; + readonly Lazy pullRequestSessionManager; readonly IUsageTracker usageTracker; + readonly IGitHubServiceProvider serviceProvider; + + bool initialized; [ImportingConstructor] - public PullRequestStatusBarManager(IGitHubServiceProvider serviceProvider, IPullRequestSessionManager pullRequestSessionManager, IUsageTracker usageTracker) - : this() + public PullRequestStatusBarManager(IVSGitExt gitExt, Lazy pullRequestSessionManager, + IUsageTracker usageTracker, IGitHubServiceProvider serviceProvider) { - this.serviceProvider = serviceProvider; + this.gitExt = gitExt; this.pullRequestSessionManager = pullRequestSessionManager; this.usageTracker = usageTracker; + this.serviceProvider = serviceProvider; + mainWindow = Application.Current.MainWindow; } - public PullRequestStatusBarManager() + public void Initialize() { - mainWindow = Application.Current.MainWindow; + TryInitialize(); + gitExt.ActiveRepositoriesChanged += TryInitialize; } - public void Initialize() + void TryInitialize() + { + if (!initialized && gitExt.ActiveRepositories.Count > 0) + { + initialized = true; + InitializeAsync().Forget(); + gitExt.ActiveRepositoriesChanged -= TryInitialize; + } + } + + async Task InitializeAsync() { - RefreshCurrentSession(); - pullRequestSessionManager.PropertyChanged += PullRequestSessionManager_PropertyChanged; + try + { + await ThreadingHelper.SwitchToMainThreadAsync(); // Switch from VSGitExt to Main thread + + RefreshCurrentSession(); + pullRequestSessionManager.Value.PropertyChanged += PullRequestSessionManager_PropertyChanged; + } + catch (Exception e) + { + log.Error(e, "Error initializing"); + } } void PullRequestSessionManager_PropertyChanged(object sender, PropertyChangedEventArgs e) @@ -57,7 +84,7 @@ void PullRequestSessionManager_PropertyChanged(object sender, PropertyChangedEve void RefreshCurrentSession() { - var pullRequest = pullRequestSessionManager.CurrentSession?.PullRequest; + var pullRequest = pullRequestSessionManager.Value.CurrentSession?.PullRequest; var viewModel = pullRequest != null ? CreatePullRequestStatusViewModel(pullRequest) : null; ShowStatus(viewModel); } From 20879e7c2807d3b49a46f535e16089f81f467215 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 9 Feb 2018 16:31:34 +0000 Subject: [PATCH 29/56] Pass MainWindow to PullRequestStatusBarManager.Initialize This will make it more conducive to testing. --- src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs | 3 ++- .../Services/IPullRequestStatusBarManager.cs | 6 ++++-- .../Services/PullRequestStatusBarManager.cs | 6 +++--- 3 files changed, 9 insertions(+), 6 deletions(-) diff --git a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs index 6d9d00117e..80917b036c 100644 --- a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs +++ b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs @@ -1,4 +1,5 @@ using System; +using System.Windows; using System.Threading; using System.Runtime.InteropServices; using Microsoft.VisualStudio.Shell; @@ -19,7 +20,7 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke var componentModel = (IComponentModel)await GetServiceAsync(typeof(SComponentModel)); var exportProvider = componentModel.DefaultExportProvider; var pullRequestStatusManager = exportProvider.GetExportedValue(); - pullRequestStatusManager.Initialize(); + pullRequestStatusManager.Initialize(Application.Current.MainWindow); } } } diff --git a/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs index 9ff02cafcd..6dee2b0946 100644 --- a/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs @@ -1,7 +1,9 @@ -namespace GitHub.InlineReviews.Services +using System.Windows; + +namespace GitHub.InlineReviews.Services { public interface IPullRequestStatusBarManager { - void Initialize(); + void Initialize(Window mainWindow); } } diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index c791219370..39838246de 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -25,11 +25,11 @@ public class PullRequestStatusBarManager : IPullRequestStatusBarManager const string StatusBarPartName = "PART_SccStatusBarHost"; readonly IVSGitExt gitExt; - readonly Window mainWindow; readonly Lazy pullRequestSessionManager; readonly IUsageTracker usageTracker; readonly IGitHubServiceProvider serviceProvider; + Window mainWindow; bool initialized; [ImportingConstructor] @@ -40,11 +40,11 @@ public PullRequestStatusBarManager(IVSGitExt gitExt, Lazy Date: Fri, 9 Feb 2018 16:43:41 +0000 Subject: [PATCH 30/56] Add some xmldoc comments --- src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs | 3 +++ .../Services/IPullRequestStatusBarManager.cs | 4 ++++ .../Services/PullRequestStatusBarManager.cs | 4 ++++ 3 files changed, 11 insertions(+) diff --git a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs index 80917b036c..7ce394d413 100644 --- a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs +++ b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs @@ -15,6 +15,9 @@ namespace GitHub.InlineReviews [ProvideAutoLoad(Guids.GitSccProviderId)] public class PullRequestStatusBarPackage : AsyncPackage { + /// + /// Initialize the PR status UI on Visual Studio's status bar. + /// protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) { var componentModel = (IComponentModel)await GetServiceAsync(typeof(SComponentModel)); diff --git a/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs index 6dee2b0946..9964ec31b5 100644 --- a/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs @@ -4,6 +4,10 @@ namespace GitHub.InlineReviews.Services { public interface IPullRequestStatusBarManager { + /// + /// Place the PR status control on Visual Studio's status bar. + /// + /// The main window of Visual Studio. void Initialize(Window mainWindow); } } diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index 39838246de..3f1738331f 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -42,6 +42,10 @@ public PullRequestStatusBarManager(IVSGitExt gitExt, Lazy + /// Lazily initialize when user enters the context of a GitHub repository. + /// + /// Visual Studio's main window. public void Initialize(Window window) { mainWindow = window; From f356907ccbe5ec5b329f89bea1e9ad87da181520 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 9 Feb 2018 18:18:29 +0000 Subject: [PATCH 31/56] Add fix for hanging InitializeAsync --- .../GitHubPane/GitHubPaneViewModel.cs | 61 ++++++++----------- 1 file changed, 27 insertions(+), 34 deletions(-) diff --git a/src/GitHub.App/ViewModels/GitHubPane/GitHubPaneViewModel.cs b/src/GitHub.App/ViewModels/GitHubPane/GitHubPaneViewModel.cs index 61c7c4a034..29ce3c0559 100644 --- a/src/GitHub.App/ViewModels/GitHubPane/GitHubPaneViewModel.cs +++ b/src/GitHub.App/ViewModels/GitHubPane/GitHubPaneViewModel.cs @@ -46,8 +46,7 @@ public sealed class GitHubPaneViewModel : ViewModelBase, IGitHubPaneViewModel, I readonly ReactiveCommand refresh; readonly ReactiveCommand showPullRequests; readonly ReactiveCommand openInBrowser; - readonly SemaphoreSlim initializing = new SemaphoreSlim(1); - bool initialized; + Task initializeTask; IViewModel content; ILocalRepositoryModel localRepository; string searchQuery; @@ -198,39 +197,9 @@ public void Dispose() } /// - public async Task InitializeAsync(IServiceProvider paneServiceProvider) + public Task InitializeAsync(IServiceProvider paneServiceProvider) { - await initializing.WaitAsync(); - if (initialized) return; - - try - { - await UpdateContent(teamExplorerContext.ActiveRepository); - teamExplorerContext.WhenAnyValue(x => x.ActiveRepository) - .Skip(1) - .ObserveOn(RxApp.MainThreadScheduler) - .Subscribe(x => UpdateContent(x).Forget()); - - connectionManager.Connections.CollectionChanged += (_, __) => UpdateContent(LocalRepository).Forget(); - - BindNavigatorCommand(paneServiceProvider, PkgCmdIDList.pullRequestCommand, showPullRequests); - BindNavigatorCommand(paneServiceProvider, PkgCmdIDList.backCommand, navigator.NavigateBack); - BindNavigatorCommand(paneServiceProvider, PkgCmdIDList.forwardCommand, navigator.NavigateForward); - BindNavigatorCommand(paneServiceProvider, PkgCmdIDList.refreshCommand, refresh); - BindNavigatorCommand(paneServiceProvider, PkgCmdIDList.githubCommand, openInBrowser); - - paneServiceProvider.AddCommandHandler(Guids.guidGitHubToolbarCmdSet, PkgCmdIDList.helpCommand, - (_, __) => - { - browser.OpenUrl(new Uri(GitHubUrls.Documentation)); - usageTracker.IncrementCounter(x => x.NumberOfGitHubPaneHelpClicks).Forget(); - }); - } - finally - { - initialized = true; - initializing.Release(); - } + return initializeTask = initializeTask ?? CreateInitializeTask(paneServiceProvider); } /// @@ -304,6 +273,30 @@ public Task ShowPullRequest(string owner, string repo, int number) x => x.RemoteRepositoryOwner == owner && x.LocalRepository.Name == repo && x.Number == number); } + async Task CreateInitializeTask(IServiceProvider paneServiceProvider) + { + await UpdateContent(teamExplorerContext.ActiveRepository); + teamExplorerContext.WhenAnyValue(x => x.ActiveRepository) + .Skip(1) + .ObserveOn(RxApp.MainThreadScheduler) + .Subscribe(x => UpdateContent(x).Forget()); + + connectionManager.Connections.CollectionChanged += (_, __) => UpdateContent(LocalRepository).Forget(); + + BindNavigatorCommand(paneServiceProvider, PkgCmdIDList.pullRequestCommand, showPullRequests); + BindNavigatorCommand(paneServiceProvider, PkgCmdIDList.backCommand, navigator.NavigateBack); + BindNavigatorCommand(paneServiceProvider, PkgCmdIDList.forwardCommand, navigator.NavigateForward); + BindNavigatorCommand(paneServiceProvider, PkgCmdIDList.refreshCommand, refresh); + BindNavigatorCommand(paneServiceProvider, PkgCmdIDList.githubCommand, openInBrowser); + + paneServiceProvider.AddCommandHandler(Guids.guidGitHubToolbarCmdSet, PkgCmdIDList.helpCommand, + (_, __) => + { + browser.OpenUrl(new Uri(GitHubUrls.Documentation)); + usageTracker.IncrementCounter(x => x.NumberOfGitHubPaneHelpClicks).Forget(); + }); + } + OleMenuCommand BindNavigatorCommand(IServiceProvider paneServiceProvider, int commandId, ReactiveCommand command) { Guard.ArgumentNotNull(paneServiceProvider, nameof(paneServiceProvider)); From 323f33042dc24f824fb57175a05f443c263a7b22 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 23 Feb 2018 10:52:35 +0000 Subject: [PATCH 32/56] Ensure consistent task ordering using ContinueWith Lock prevent tasks being executed at the same time but doesn't stop them being started out of order. --- .../Services/VSGitExt.cs | 33 ++++++++++--------- .../GitHub.TeamFoundation/VSGitExtTests.cs | 4 ++- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 6dd1d5fd12..81f86e3328 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -1,5 +1,6 @@ using System; using System.Linq; +using System.Threading.Tasks; using System.Collections.Generic; using System.ComponentModel.Composition; using GitHub.Models; @@ -8,7 +9,6 @@ using GitHub.TeamFoundation.Services; using Serilog; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; -using Task = System.Threading.Tasks.Task; namespace GitHub.VisualStudio.Base { @@ -24,7 +24,6 @@ public class VSGitExt : IVSGitExt readonly IGitHubServiceProvider serviceProvider; readonly IVSUIContext context; readonly ILocalRepositoryModelFactory repositoryFactory; - readonly object refreshLock = new object(); IGitExt gitService; IReadOnlyList activeRepositories; @@ -41,23 +40,24 @@ public VSGitExt(IGitHubServiceProvider serviceProvider, IVSUIContextFactory fact this.repositoryFactory = repositoryFactory; // The IGitExt service isn't available when a TFS based solution is opened directly. - // It will become available when moving to a Git based solution (cause a UIContext event to fire). + // It will become available when moving to a Git based solution (and cause a UIContext event to fire). context = factory.GetUIContext(new Guid(Guids.GitSccProviderId)); // Start with empty array until we have a change to initialize. ActiveRepositories = Array.Empty(); + InitializeTask = Task.CompletedTask; + if (context.IsActive && TryInitialize()) { // Refresh ActiveRepositories on background thread so we don't delay startup. - InitializeTask = Task.Run(() => RefreshActiveRepositories()); + QueueRefreshActiveRepositories(); } else { // If we're not in the UIContext or TryInitialize fails, have another go when the UIContext changes. context.UIContextChanged += ContextChanged; log.Debug("VSGitExt will be initialized later"); - InitializeTask = Task.CompletedTask; } } @@ -68,7 +68,7 @@ void ContextChanged(object sender, VSUIContextChangedEventArgs e) if (e.Activated && TryInitialize()) { // Refresh ActiveRepositories on background thread so we don't delay UI context change. - InitializeTask = Task.Run(() => RefreshActiveRepositories()); + QueueRefreshActiveRepositories(); context.UIContextChanged -= ContextChanged; log.Debug("Initialized VSGitExt on UIContextChanged"); } @@ -83,7 +83,7 @@ bool TryInitialize() { if (e.PropertyName == nameof(gitService.ActiveRepositories)) { - RefreshActiveRepositories(); + QueueRefreshActiveRepositories(); } }; @@ -95,19 +95,22 @@ bool TryInitialize() return false; } + void QueueRefreshActiveRepositories() + { + // Execute tasks in sequence on thread pool. + InitializeTask = InitializeTask.ContinueWith(_ => RefreshActiveRepositories(), TaskScheduler.Default); + } + void RefreshActiveRepositories() { try { - lock (refreshLock) - { - log.Debug( - "IGitExt.ActiveRepositories (#{Id}) returned {Repositories}", - gitService.GetHashCode(), - gitService?.ActiveRepositories.Select(x => x.RepositoryPath)); + log.Debug( + "IGitExt.ActiveRepositories (#{Id}) returned {Repositories}", + gitService.GetHashCode(), + gitService?.ActiveRepositories.Select(x => x.RepositoryPath)); - ActiveRepositories = gitService?.ActiveRepositories.Select(x => repositoryFactory.Create(x.RepositoryPath)).ToList(); - } + ActiveRepositories = gitService?.ActiveRepositories.Select(x => repositoryFactory.Create(x.RepositoryPath)).ToList(); } catch (Exception e) { diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index 153d6295fc..bc6ebca7a2 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -63,7 +63,7 @@ public void ActiveRepositories_ReadUsingThreadPoolThread() public class TheActiveRepositoriesChangedEvent { [Test] - public void GitExtPropertyChangedEvent_ActiveRepositoriesChangedIsFired() + public async Task GitExtPropertyChangedEvent_ActiveRepositoriesChangedIsFired() { var context = CreateVSUIContext(true); var gitExt = CreateGitExt(); @@ -75,6 +75,7 @@ public void GitExtPropertyChangedEvent_ActiveRepositoriesChangedIsFired() var eventArgs = new PropertyChangedEventArgs(nameof(gitExt.ActiveRepositories)); gitExt.PropertyChanged += Raise.Event(gitExt, eventArgs); + await target.InitializeTask; Assert.That(wasFired, Is.True); } @@ -188,6 +189,7 @@ public async Task ActiveRepositoriesChangedOrderingShouldBeCorrectAcrossThreads( var task2 = Task.Run(() => gitExt.ActiveRepositories = activeRepositories2); await Task.WhenAll(task1, task2); + await target.InitializeTask; Assert.That(target.ActiveRepositories.Single().LocalPath, Is.EqualTo("repo2")); } From 5200e72947a9fb68fe10d848f031afd3e498c5d6 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 23 Feb 2018 11:03:08 +0000 Subject: [PATCH 33/56] Rename InitializeTask to PendingTasks PendingTasks is used to execute tasks in order on the thread pool. --- src/GitHub.TeamFoundation.14/Services/VSGitExt.cs | 13 +++++++------ .../GitHub.TeamFoundation/VSGitExtTests.cs | 14 +++++++------- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 81f86e3328..d98e3edda5 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -43,11 +43,9 @@ public VSGitExt(IGitHubServiceProvider serviceProvider, IVSUIContextFactory fact // It will become available when moving to a Git based solution (and cause a UIContext event to fire). context = factory.GetUIContext(new Guid(Guids.GitSccProviderId)); - // Start with empty array until we have a change to initialize. + // Start with empty array until we have a chance to initialize. ActiveRepositories = Array.Empty(); - InitializeTask = Task.CompletedTask; - if (context.IsActive && TryInitialize()) { // Refresh ActiveRepositories on background thread so we don't delay startup. @@ -97,8 +95,8 @@ bool TryInitialize() void QueueRefreshActiveRepositories() { - // Execute tasks in sequence on thread pool. - InitializeTask = InitializeTask.ContinueWith(_ => RefreshActiveRepositories(), TaskScheduler.Default); + // Execute tasks in sequence using thread pool (TaskScheduler.Default). + PendingTasks = PendingTasks.ContinueWith(_ => RefreshActiveRepositories(), TaskScheduler.Default); } void RefreshActiveRepositories() @@ -139,6 +137,9 @@ private set public event Action ActiveRepositoriesChanged; - public Task InitializeTask { get; private set; } + /// + /// Tasks that are pending execution on the thread pool. + /// + public Task PendingTasks { get; private set; } = Task.CompletedTask; } } \ No newline at end of file diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index bc6ebca7a2..fb7c7f82ae 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -75,7 +75,7 @@ public async Task GitExtPropertyChangedEvent_ActiveRepositoriesChangedIsFired() var eventArgs = new PropertyChangedEventArgs(nameof(gitExt.ActiveRepositories)); gitExt.PropertyChanged += Raise.Event(gitExt, eventArgs); - await target.InitializeTask; + await target.PendingTasks; Assert.That(wasFired, Is.True); } @@ -109,7 +109,7 @@ public void WhenUIContextChanged_ActiveRepositoriesChangedIsFired() var eventArgs = new VSUIContextChangedEventArgs(true); context.UIContextChanged += Raise.Event>(context, eventArgs); - target.InitializeTask.Wait(); + target.PendingTasks.Wait(); Assert.That(wasFired, Is.True); } @@ -126,7 +126,7 @@ public void WhenUIContextChanged_FiredUsingThreadPoolThread() var eventArgs = new VSUIContextChangedEventArgs(true); context.UIContextChanged += Raise.Event>(context, eventArgs); - target.InitializeTask.Wait(); + target.PendingTasks.Wait(); Assert.That(threadPool, Is.True); } @@ -151,7 +151,7 @@ public void SccProviderContextIsActive_InitializeWithActiveRepositories() var context = CreateVSUIContext(true); var gitExt = CreateGitExt(new[] { repoPath }); var target = CreateVSGitExt(context, gitExt, repoFactory: repoFactory); - target.InitializeTask.Wait(); + target.PendingTasks.Wait(); var activeRepositories = target.ActiveRepositories; @@ -168,7 +168,7 @@ public void ExceptionRefreshingRepositories_ReturnsEmptyList() var context = CreateVSUIContext(true); var gitExt = CreateGitExt(new[] { repoPath }); var target = CreateVSGitExt(context, gitExt, repoFactory: repoFactory); - target.InitializeTask.Wait(); + target.PendingTasks.Wait(); var activeRepositories = target.ActiveRepositories; @@ -189,7 +189,7 @@ public async Task ActiveRepositoriesChangedOrderingShouldBeCorrectAcrossThreads( var task2 = Task.Run(() => gitExt.ActiveRepositories = activeRepositories2); await Task.WhenAll(task1, task2); - await target.InitializeTask; + await target.PendingTasks; Assert.That(target.ActiveRepositories.Single().LocalPath, Is.EqualTo("repo2")); } @@ -221,7 +221,7 @@ static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = nul sp.GetService().Returns(factory); sp.GetService().Returns(gitExt); var vsGitExt = new VSGitExt(sp, factory, repoFactory); - vsGitExt.InitializeTask.Wait(); + vsGitExt.PendingTasks.Wait(); return vsGitExt; } From 97eaced4d9643216cabe927fd33e9e93779b994e Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 23 Feb 2018 12:29:07 +0000 Subject: [PATCH 34/56] Remove redundant initialized flag --- .../Services/PullRequestStatusBarManager.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index 3f1738331f..5fd704d689 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -30,7 +30,6 @@ public class PullRequestStatusBarManager : IPullRequestStatusBarManager readonly IGitHubServiceProvider serviceProvider; Window mainWindow; - bool initialized; [ImportingConstructor] public PullRequestStatusBarManager(IVSGitExt gitExt, Lazy pullRequestSessionManager, @@ -49,17 +48,16 @@ public PullRequestStatusBarManager(IVSGitExt gitExt, Lazy 0) + if (gitExt.ActiveRepositories.Count > 0) { - initialized = true; InitializeAsync().Forget(); - gitExt.ActiveRepositoriesChanged -= TryInitialize; + gitExt.ActiveRepositoriesChanged -= OnActiveRepositoriesChanged; } } From 1ea222ae8b55e4fc1444b0b506d37aedd126bc87 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 23 Feb 2018 16:44:47 +0000 Subject: [PATCH 35/56] Lazy load VSGitExt on Main thread Make PullRequestStatusBarPackage load on background thread. The VSGitExt MEF service must be loaded on the Main thread. --- .../PullRequestStatusBarPackage.cs | 7 +++--- .../Services/IPullRequestStatusBarManager.cs | 5 ++-- .../Services/PullRequestStatusBarManager.cs | 25 +++++++++---------- 3 files changed, 17 insertions(+), 20 deletions(-) diff --git a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs index 7ce394d413..8c2a609315 100644 --- a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs +++ b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs @@ -1,5 +1,4 @@ using System; -using System.Windows; using System.Threading; using System.Runtime.InteropServices; using Microsoft.VisualStudio.Shell; @@ -10,9 +9,9 @@ namespace GitHub.InlineReviews { - [PackageRegistration(UseManagedResourcesOnly = true)] [Guid(Guids.PullRequestStatusPackageId)] - [ProvideAutoLoad(Guids.GitSccProviderId)] + [PackageRegistration(UseManagedResourcesOnly = true, AllowsBackgroundLoading = true)] + [ProvideAutoLoad(Guids.GitSccProviderId, PackageAutoLoadFlags.BackgroundLoad)] public class PullRequestStatusBarPackage : AsyncPackage { /// @@ -23,7 +22,7 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke var componentModel = (IComponentModel)await GetServiceAsync(typeof(SComponentModel)); var exportProvider = componentModel.DefaultExportProvider; var pullRequestStatusManager = exportProvider.GetExportedValue(); - pullRequestStatusManager.Initialize(Application.Current.MainWindow); + await pullRequestStatusManager.InitializeAsync(); } } } diff --git a/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs index 9964ec31b5..f32d09efbb 100644 --- a/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs @@ -1,4 +1,4 @@ -using System.Windows; +using System.Threading.Tasks; namespace GitHub.InlineReviews.Services { @@ -7,7 +7,6 @@ public interface IPullRequestStatusBarManager /// /// Place the PR status control on Visual Studio's status bar. /// - /// The main window of Visual Studio. - void Initialize(Window mainWindow); + Task InitializeAsync(); } } diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index 5fd704d689..4393ca50b2 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -24,15 +24,13 @@ public class PullRequestStatusBarManager : IPullRequestStatusBarManager static readonly ILogger log = LogManager.ForContext(); const string StatusBarPartName = "PART_SccStatusBarHost"; - readonly IVSGitExt gitExt; + readonly Lazy gitExt; readonly Lazy pullRequestSessionManager; readonly IUsageTracker usageTracker; readonly IGitHubServiceProvider serviceProvider; - Window mainWindow; - [ImportingConstructor] - public PullRequestStatusBarManager(IVSGitExt gitExt, Lazy pullRequestSessionManager, + public PullRequestStatusBarManager(Lazy gitExt, Lazy pullRequestSessionManager, IUsageTracker usageTracker, IGitHubServiceProvider serviceProvider) { this.gitExt = gitExt; @@ -45,23 +43,24 @@ public PullRequestStatusBarManager(IVSGitExt gitExt, Lazy /// Visual Studio's main window. - public void Initialize(Window window) + public async Task InitializeAsync() { - mainWindow = window; + await ThreadingHelper.SwitchToMainThreadAsync(); + OnActiveRepositoriesChanged(); - gitExt.ActiveRepositoriesChanged += OnActiveRepositoriesChanged; + gitExt.Value.ActiveRepositoriesChanged += OnActiveRepositoriesChanged; } void OnActiveRepositoriesChanged() { - if (gitExt.ActiveRepositories.Count > 0) + if (gitExt.Value.ActiveRepositories.Count > 0) { - InitializeAsync().Forget(); - gitExt.ActiveRepositoriesChanged -= OnActiveRepositoriesChanged; + StartShowingStatus().Forget(); + gitExt.Value.ActiveRepositoriesChanged -= OnActiveRepositoriesChanged; } } - async Task InitializeAsync() + async Task StartShowingStatus() { try { @@ -103,7 +102,7 @@ PullRequestStatusViewModel CreatePullRequestStatusViewModel(IPullRequestModel pu void ShowStatus(PullRequestStatusViewModel pullRequestStatusViewModel = null) { - var statusBar = FindSccStatusBar(); + var statusBar = FindSccStatusBar(Application.Current.MainWindow); if (statusBar != null) { var githubStatusBar = Find(statusBar); @@ -134,7 +133,7 @@ static T Find(StatusBar statusBar) return default(T); } - StatusBar FindSccStatusBar() + StatusBar FindSccStatusBar(Window mainWindow) { var contentControl = mainWindow?.Template?.FindName(StatusBarPartName, mainWindow) as ContentControl; return contentControl?.Content as StatusBar; From 68d098a8906eb26d204ecda6745b54f1da979e40 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 26 Feb 2018 12:12:53 +0000 Subject: [PATCH 36/56] Do initialization asynchronously on Main thread This will allow this service to be used by a package that is initializing on a background thread. --- .../Services/VSGitExt.cs | 68 ++++++++++++------- .../GitHub.TeamFoundation/VSGitExtTests.cs | 7 +- 2 files changed, 48 insertions(+), 27 deletions(-) diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index d98e3edda5..835db8214d 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -6,6 +6,7 @@ using GitHub.Models; using GitHub.Services; using GitHub.Logging; +using GitHub.Helpers; using GitHub.TeamFoundation.Services; using Serilog; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; @@ -46,34 +47,57 @@ public VSGitExt(IGitHubServiceProvider serviceProvider, IVSUIContextFactory fact // Start with empty array until we have a chance to initialize. ActiveRepositories = Array.Empty(); - if (context.IsActive && TryInitialize()) + PendingTasks = InitializeAsync(); + } + + async Task InitializeAsync() + { + try { - // Refresh ActiveRepositories on background thread so we don't delay startup. - QueueRefreshActiveRepositories(); + if (!context.IsActive || !await TryInitialize()) + { + // If we're not in the UIContext or TryInitialize fails, have another go when the UIContext changes. + context.UIContextChanged += ContextChanged; + log.Debug("VSGitExt will be initialized later"); + } } - else + catch (Exception e) { - // If we're not in the UIContext or TryInitialize fails, have another go when the UIContext changes. - context.UIContextChanged += ContextChanged; - log.Debug("VSGitExt will be initialized later"); + log.Error(e, "Initializing"); } } void ContextChanged(object sender, VSUIContextChangedEventArgs e) { - // If we're in the UIContext and TryInitialize succeeds, we can stop listening for events. - // NOTE: this event can fire with UIContext=true in a TFS solution (not just Git). - if (e.Activated && TryInitialize()) + if (e.Activated) { - // Refresh ActiveRepositories on background thread so we don't delay UI context change. - QueueRefreshActiveRepositories(); - context.UIContextChanged -= ContextChanged; - log.Debug("Initialized VSGitExt on UIContextChanged"); + PendingTasks = ContextChangedAsync(); } } - bool TryInitialize() + async Task ContextChangedAsync() { + try + { + // If we're in the UIContext and TryInitialize succeeds, we can stop listening for events. + // NOTE: this event can fire with UIContext=true in a TFS solution (not just Git). + if (await TryInitialize()) + { + context.UIContextChanged -= ContextChanged; + log.Debug("Initialized VSGitExt on UIContextChanged"); + } + } + catch (Exception e) + { + log.Error(e, "UIContextChanged"); + } + } + + async Task TryInitialize() + { + // GetService must be called on the Main thread. + await ThreadingHelper.SwitchToMainThreadAsync(); + gitService = serviceProvider.GetService(); if (gitService != null) { @@ -81,10 +105,14 @@ bool TryInitialize() { if (e.PropertyName == nameof(gitService.ActiveRepositories)) { - QueueRefreshActiveRepositories(); + // Execute tasks in sequence using thread pool (TaskScheduler.Default). + PendingTasks = PendingTasks.ContinueWith(_ => RefreshActiveRepositories(), TaskScheduler.Default); } }; + // Do this after we start listening so we don't miss an event. + await Task.Run(() => RefreshActiveRepositories()); + log.Debug("Found IGitExt service and initialized VSGitExt"); return true; } @@ -93,12 +121,6 @@ bool TryInitialize() return false; } - void QueueRefreshActiveRepositories() - { - // Execute tasks in sequence using thread pool (TaskScheduler.Default). - PendingTasks = PendingTasks.ContinueWith(_ => RefreshActiveRepositories(), TaskScheduler.Default); - } - void RefreshActiveRepositories() { try @@ -140,6 +162,6 @@ private set /// /// Tasks that are pending execution on the thread pool. /// - public Task PendingTasks { get; private set; } = Task.CompletedTask; + public Task PendingTasks { get; private set; } } } \ No newline at end of file diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index fb7c7f82ae..7dfe6543c2 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -11,11 +11,10 @@ using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; using System.Threading.Tasks; using System.Linq; -using GitHub.TeamFoundation.Services; public class VSGitExtTests { - public class TheConstructor + public class TheConstructor : TestBaseClass { [TestCase(true, 1)] [TestCase(false, 0)] @@ -60,7 +59,7 @@ public void ActiveRepositories_ReadUsingThreadPoolThread() } } - public class TheActiveRepositoriesChangedEvent + public class TheActiveRepositoriesChangedEvent : TestBaseClass { [Test] public async Task GitExtPropertyChangedEvent_ActiveRepositoriesChangedIsFired() @@ -132,7 +131,7 @@ public void WhenUIContextChanged_FiredUsingThreadPoolThread() } } - public class TheActiveRepositoriesProperty + public class TheActiveRepositoriesProperty : TestBaseClass { [Test] public void SccProviderContextNotActive_IsEmpty() From dcd806fb80f7939dd173424c0f742d5964eb217b Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 26 Feb 2018 14:45:01 +0000 Subject: [PATCH 37/56] Add comment about using from background thread --- src/GitHub.TeamFoundation.14/Services/VSGitExt.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 835db8214d..1d68f100f0 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -16,6 +16,10 @@ namespace GitHub.VisualStudio.Base /// /// This service acts as an always available version of . /// + /// + /// Initialization for this service will be done asynchronously and the service will be + /// retrieved on the Main thread. This means the service to be constructed and subscribed to on a background thread. + /// [Export(typeof(IVSGitExt))] [PartCreationPolicy(CreationPolicy.Shared)] public class VSGitExt : IVSGitExt From 2a3cef5db0a427940758a4075b3153fb8a25ca25 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 26 Feb 2018 15:12:00 +0000 Subject: [PATCH 38/56] Factor out GetServiceAsync --- src/GitHub.TeamFoundation.14/Services/VSGitExt.cs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 1d68f100f0..4956c270f6 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -99,10 +99,7 @@ async Task ContextChangedAsync() async Task TryInitialize() { - // GetService must be called on the Main thread. - await ThreadingHelper.SwitchToMainThreadAsync(); - - gitService = serviceProvider.GetService(); + gitService = await GetServiceAsync(); if (gitService != null) { gitService.PropertyChanged += (s, e) => @@ -125,6 +122,13 @@ async Task TryInitialize() return false; } + async Task GetServiceAsync() where T : class + { + // GetService must be called from the Main thread. + await ThreadingHelper.SwitchToMainThreadAsync(); + return serviceProvider.GetService(); + } + void RefreshActiveRepositories() { try From 66fbbaa5ea03b28796b863810370dfc6f6bf2caf Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 26 Feb 2018 18:28:20 +0000 Subject: [PATCH 39/56] Load VS services asynchronously Only IVSGitExt needs to be loaded synchronously. --- .../GitHub.InlineReviews.csproj | 1 - .../PullRequestStatusBarPackage.cs | 13 ++++++++--- .../Services/IPullRequestStatusBarManager.cs | 12 ---------- .../Services/PullRequestStatusBarManager.cs | 22 +++++-------------- 4 files changed, 16 insertions(+), 32 deletions(-) delete mode 100644 src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs diff --git a/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj b/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj index a555e10c8b..418b70a6ae 100644 --- a/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj +++ b/src/GitHub.InlineReviews/GitHub.InlineReviews.csproj @@ -93,7 +93,6 @@ - diff --git a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs index 8c2a609315..883f4f38d0 100644 --- a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs +++ b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs @@ -3,6 +3,8 @@ using System.Runtime.InteropServices; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.ComponentModelHost; +using GitHub.Helpers; +using GitHub.Services; using GitHub.VisualStudio; using GitHub.InlineReviews.Services; using Task = System.Threading.Tasks.Task; @@ -19,10 +21,15 @@ public class PullRequestStatusBarPackage : AsyncPackage /// protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) { + var usageTracker = (IUsageTracker)await GetServiceAsync(typeof(IUsageTracker)); + var serviceProvider = (IGitHubServiceProvider)await GetServiceAsync(typeof(IGitHubServiceProvider)); var componentModel = (IComponentModel)await GetServiceAsync(typeof(SComponentModel)); - var exportProvider = componentModel.DefaultExportProvider; - var pullRequestStatusManager = exportProvider.GetExportedValue(); - await pullRequestStatusManager.InitializeAsync(); + var pullRequestSessionManager = componentModel.DefaultExportProvider.GetExport(); + + await ThreadingHelper.SwitchToMainThreadAsync(); + var gitExt = componentModel.DefaultExportProvider.GetExportedValue(); + + new PullRequestStatusBarManager(gitExt, pullRequestSessionManager, usageTracker, serviceProvider); } } } diff --git a/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs deleted file mode 100644 index f32d09efbb..0000000000 --- a/src/GitHub.InlineReviews/Services/IPullRequestStatusBarManager.cs +++ /dev/null @@ -1,12 +0,0 @@ -using System.Threading.Tasks; - -namespace GitHub.InlineReviews.Services -{ - public interface IPullRequestStatusBarManager - { - /// - /// Place the PR status control on Visual Studio's status bar. - /// - Task InitializeAsync(); - } -} diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index 4393ca50b2..d44112b915 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -18,45 +18,35 @@ namespace GitHub.InlineReviews.Services { - [Export(typeof(IPullRequestStatusBarManager))] - public class PullRequestStatusBarManager : IPullRequestStatusBarManager + public class PullRequestStatusBarManager { static readonly ILogger log = LogManager.ForContext(); const string StatusBarPartName = "PART_SccStatusBarHost"; - readonly Lazy gitExt; + readonly IVSGitExt gitExt; readonly Lazy pullRequestSessionManager; readonly IUsageTracker usageTracker; readonly IGitHubServiceProvider serviceProvider; [ImportingConstructor] - public PullRequestStatusBarManager(Lazy gitExt, Lazy pullRequestSessionManager, + public PullRequestStatusBarManager(IVSGitExt gitExt, Lazy pullRequestSessionManager, IUsageTracker usageTracker, IGitHubServiceProvider serviceProvider) { this.gitExt = gitExt; this.pullRequestSessionManager = pullRequestSessionManager; this.usageTracker = usageTracker; this.serviceProvider = serviceProvider; - } - - /// - /// Lazily initialize when user enters the context of a GitHub repository. - /// - /// Visual Studio's main window. - public async Task InitializeAsync() - { - await ThreadingHelper.SwitchToMainThreadAsync(); OnActiveRepositoriesChanged(); - gitExt.Value.ActiveRepositoriesChanged += OnActiveRepositoriesChanged; + gitExt.ActiveRepositoriesChanged += OnActiveRepositoriesChanged; } void OnActiveRepositoriesChanged() { - if (gitExt.Value.ActiveRepositories.Count > 0) + if (gitExt.ActiveRepositories.Count > 0) { StartShowingStatus().Forget(); - gitExt.Value.ActiveRepositoriesChanged -= OnActiveRepositoriesChanged; + gitExt.ActiveRepositoriesChanged -= OnActiveRepositoriesChanged; } } From 16aabcd600ba8b999a084c1a5ac9f2d73d71f246 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 27 Feb 2018 09:40:48 +0000 Subject: [PATCH 40/56] Make grammarful English --- src/GitHub.TeamFoundation.14/Services/VSGitExt.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 4956c270f6..19205a5e5f 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -18,7 +18,7 @@ namespace GitHub.VisualStudio.Base /// /// /// Initialization for this service will be done asynchronously and the service will be - /// retrieved on the Main thread. This means the service to be constructed and subscribed to on a background thread. + /// retrieved on the Main thread. This means the service can be constructed and subscribed to on a background thread. /// [Export(typeof(IVSGitExt))] [PartCreationPolicy(CreationPolicy.Shared)] From 12057906ffa8c1a2632bc686179cf81595b1f18c Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 27 Feb 2018 10:00:51 +0000 Subject: [PATCH 41/56] Remove dependency on IComponentModel Retrieve IVSGitExt and IPullRequestSessionManager via GitHubServiceProvider. --- .../PullRequestStatusBarPackage.cs | 10 +++------- .../Services/PullRequestStatusBarManager.cs | 14 ++++++++------ 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs index 883f4f38d0..c39ade0184 100644 --- a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs +++ b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs @@ -1,12 +1,11 @@ using System; using System.Threading; using System.Runtime.InteropServices; -using Microsoft.VisualStudio.Shell; -using Microsoft.VisualStudio.ComponentModelHost; using GitHub.Helpers; using GitHub.Services; using GitHub.VisualStudio; using GitHub.InlineReviews.Services; +using Microsoft.VisualStudio.Shell; using Task = System.Threading.Tasks.Task; namespace GitHub.InlineReviews @@ -23,13 +22,10 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke { var usageTracker = (IUsageTracker)await GetServiceAsync(typeof(IUsageTracker)); var serviceProvider = (IGitHubServiceProvider)await GetServiceAsync(typeof(IGitHubServiceProvider)); - var componentModel = (IComponentModel)await GetServiceAsync(typeof(SComponentModel)); - var pullRequestSessionManager = componentModel.DefaultExportProvider.GetExport(); await ThreadingHelper.SwitchToMainThreadAsync(); - var gitExt = componentModel.DefaultExportProvider.GetExportedValue(); - - new PullRequestStatusBarManager(gitExt, pullRequestSessionManager, usageTracker, serviceProvider); + var gitExt = serviceProvider.GetService(); + new PullRequestStatusBarManager(gitExt, usageTracker, serviceProvider); } } } diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index d44112b915..8e47096f1d 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -24,16 +24,15 @@ public class PullRequestStatusBarManager const string StatusBarPartName = "PART_SccStatusBarHost"; readonly IVSGitExt gitExt; - readonly Lazy pullRequestSessionManager; readonly IUsageTracker usageTracker; readonly IGitHubServiceProvider serviceProvider; + IPullRequestSessionManager pullRequestSessionManager; + [ImportingConstructor] - public PullRequestStatusBarManager(IVSGitExt gitExt, Lazy pullRequestSessionManager, - IUsageTracker usageTracker, IGitHubServiceProvider serviceProvider) + public PullRequestStatusBarManager(IVSGitExt gitExt, IUsageTracker usageTracker, IGitHubServiceProvider serviceProvider) { this.gitExt = gitExt; - this.pullRequestSessionManager = pullRequestSessionManager; this.usageTracker = usageTracker; this.serviceProvider = serviceProvider; @@ -56,8 +55,11 @@ async Task StartShowingStatus() { await ThreadingHelper.SwitchToMainThreadAsync(); // Switch from VSGitExt to Main thread + // Create just in time on Main thread. + pullRequestSessionManager = serviceProvider.GetService(); + RefreshCurrentSession(); - pullRequestSessionManager.Value.PropertyChanged += PullRequestSessionManager_PropertyChanged; + pullRequestSessionManager.PropertyChanged += PullRequestSessionManager_PropertyChanged; } catch (Exception e) { @@ -75,7 +77,7 @@ void PullRequestSessionManager_PropertyChanged(object sender, PropertyChangedEve void RefreshCurrentSession() { - var pullRequest = pullRequestSessionManager.Value.CurrentSession?.PullRequest; + var pullRequest = pullRequestSessionManager.CurrentSession?.PullRequest; var viewModel = pullRequest != null ? CreatePullRequestStatusViewModel(pullRequest) : null; ShowStatus(viewModel); } From ebbc6a6031d354592ab3fc749046a9101bf041d3 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 27 Feb 2018 11:53:39 +0000 Subject: [PATCH 42/56] Let PR # appear on status bar before solution has loaded ThreadingHelper.SwitchToMainThreadAsync() only returns when a solution has loaded. Use Application.Current.Dispatcher.Invoke instead of ThreadingHelper.SwitchToMainThreadAsync. --- .../PullRequestStatusBarPackage.cs | 11 +++++++---- .../Services/PullRequestStatusBarManager.cs | 8 ++------ 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs index c39ade0184..1573a10b7f 100644 --- a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs +++ b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs @@ -1,7 +1,7 @@ using System; +using System.Windows; using System.Threading; using System.Runtime.InteropServices; -using GitHub.Helpers; using GitHub.Services; using GitHub.VisualStudio; using GitHub.InlineReviews.Services; @@ -23,9 +23,12 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke var usageTracker = (IUsageTracker)await GetServiceAsync(typeof(IUsageTracker)); var serviceProvider = (IGitHubServiceProvider)await GetServiceAsync(typeof(IGitHubServiceProvider)); - await ThreadingHelper.SwitchToMainThreadAsync(); - var gitExt = serviceProvider.GetService(); - new PullRequestStatusBarManager(gitExt, usageTracker, serviceProvider); + // NOTE: ThreadingHelper.SwitchToMainThreadAsync() doesn't return until a solution is loaded. Using Dispatcher.Invoke instead. + Application.Current.Dispatcher.Invoke(() => + { + var gitExt = serviceProvider.GetService(); + new PullRequestStatusBarManager(gitExt, usageTracker, serviceProvider); + }); } } } diff --git a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs index 8e47096f1d..3ed151b856 100644 --- a/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs +++ b/src/GitHub.InlineReviews/Services/PullRequestStatusBarManager.cs @@ -11,10 +11,8 @@ using GitHub.VisualStudio; using GitHub.Models; using GitHub.Logging; -using GitHub.Helpers; using GitHub.Extensions; using Serilog; -using Task = System.Threading.Tasks.Task; namespace GitHub.InlineReviews.Services { @@ -44,17 +42,15 @@ void OnActiveRepositoriesChanged() { if (gitExt.ActiveRepositories.Count > 0) { - StartShowingStatus().Forget(); gitExt.ActiveRepositoriesChanged -= OnActiveRepositoriesChanged; + Application.Current.Dispatcher.Invoke(() => StartShowingStatus()); } } - async Task StartShowingStatus() + void StartShowingStatus() { try { - await ThreadingHelper.SwitchToMainThreadAsync(); // Switch from VSGitExt to Main thread - // Create just in time on Main thread. pullRequestSessionManager = serviceProvider.GetService(); From f23fd248b18874d64f30917d14a7d2e0055797f5 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Tue, 27 Feb 2018 21:08:15 +0000 Subject: [PATCH 43/56] Expose IVSGitExt as async VS service --- .../Services/IGitHubServiceProvider.cs | 3 ++ .../PullRequestStatusBarPackage.cs | 9 +--- .../Services/VSGitExt.cs | 12 +----- .../GitHub.VisualStudio.csproj | 2 + src/GitHub.VisualStudio/GitHubPackage.cs | 42 ++++++++++++++++++- .../Services/GitHubServiceProvider.cs | 4 ++ 6 files changed, 53 insertions(+), 19 deletions(-) diff --git a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs index c1d142ed7f..000301d0ce 100644 --- a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs +++ b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs @@ -1,4 +1,5 @@ using System; +using System.Threading.Tasks; using System.ComponentModel.Composition.Hosting; using System.Runtime.InteropServices; using GitHub.VisualStudio; @@ -28,5 +29,7 @@ Ret GetService() where T : class /// The owner, which either has to match what was passed to AddService, /// or if it's null, the service will be removed without checking for ownership void RemoveService(Type t, object owner); + + Task GetServiceAsync(Type serviceType); } } diff --git a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs index 1573a10b7f..8b04edb866 100644 --- a/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs +++ b/src/GitHub.InlineReviews/PullRequestStatusBarPackage.cs @@ -1,5 +1,4 @@ using System; -using System.Windows; using System.Threading; using System.Runtime.InteropServices; using GitHub.Services; @@ -22,13 +21,9 @@ protected override async Task InitializeAsync(CancellationToken cancellationToke { var usageTracker = (IUsageTracker)await GetServiceAsync(typeof(IUsageTracker)); var serviceProvider = (IGitHubServiceProvider)await GetServiceAsync(typeof(IGitHubServiceProvider)); + var gitExt = (IVSGitExt)await GetServiceAsync(typeof(IVSGitExt)); - // NOTE: ThreadingHelper.SwitchToMainThreadAsync() doesn't return until a solution is loaded. Using Dispatcher.Invoke instead. - Application.Current.Dispatcher.Invoke(() => - { - var gitExt = serviceProvider.GetService(); - new PullRequestStatusBarManager(gitExt, usageTracker, serviceProvider); - }); + new PullRequestStatusBarManager(gitExt, usageTracker, serviceProvider); } } } diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 19205a5e5f..478aa5fcbe 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -6,7 +6,6 @@ using GitHub.Models; using GitHub.Services; using GitHub.Logging; -using GitHub.Helpers; using GitHub.TeamFoundation.Services; using Serilog; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; @@ -20,8 +19,6 @@ namespace GitHub.VisualStudio.Base /// Initialization for this service will be done asynchronously and the service will be /// retrieved on the Main thread. This means the service can be constructed and subscribed to on a background thread. /// - [Export(typeof(IVSGitExt))] - [PartCreationPolicy(CreationPolicy.Shared)] public class VSGitExt : IVSGitExt { static readonly ILogger log = LogManager.ForContext(); @@ -99,7 +96,7 @@ async Task ContextChangedAsync() async Task TryInitialize() { - gitService = await GetServiceAsync(); + gitService = (IGitExt)await serviceProvider.GetServiceAsync(typeof(IGitExt)); if (gitService != null) { gitService.PropertyChanged += (s, e) => @@ -122,13 +119,6 @@ async Task TryInitialize() return false; } - async Task GetServiceAsync() where T : class - { - // GetService must be called from the Main thread. - await ThreadingHelper.SwitchToMainThreadAsync(); - return serviceProvider.GetService(); - } - void RefreshActiveRepositories() { try diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index 18f6d03a44..c7d3879d04 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -679,6 +679,7 @@ True BuiltProjectOutputGroup;GetCopyToOutputDirectoryItems;DebugSymbolsProjectOutputGroup; DebugSymbolsProjectOutputGroup; + TF14 {161dbf01-1dbf-4b00-8551-c5c00f26720e} @@ -686,6 +687,7 @@ True BuiltProjectOutputGroup;GetCopyToOutputDirectoryItems;DebugSymbolsProjectOutputGroup; DebugSymbolsProjectOutputGroup; + TF15 {158b05e8-fdbc-4d71-b871-c96e28d5adf5} diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index bcd3c2271a..1503b34fea 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -1,4 +1,7 @@ -using System; +extern alias TF14; +extern alias TF15; + +using System; using System.ComponentModel.Composition; using System.Diagnostics; using System.Reflection; @@ -22,6 +25,8 @@ using Octokit; using Serilog; using Task = System.Threading.Tasks.Task; +using VSGitExt14 = TF14.GitHub.VisualStudio.Base.VSGitExt; +using VSGitExt15 = TF15.GitHub.VisualStudio.Base.VSGitExt; namespace GitHub.VisualStudio { @@ -107,12 +112,28 @@ public GHClient(IProgram program) } } + [PartCreationPolicy(CreationPolicy.Shared)] + public class VSGitExtPart + { + readonly IGitHubServiceProvider serviceProvider; + + [ImportingConstructor] + public VSGitExtPart(IGitHubServiceProvider serviceProvider) + { + this.serviceProvider = serviceProvider; + } + + [Export(typeof(IVSGitExt))] + public IVSGitExt VSGitExt => serviceProvider.GetService(); + } + [PackageRegistration(UseManagedResourcesOnly = true, AllowsBackgroundLoading = true)] [ProvideService(typeof(ILoginManager), IsAsyncQueryable = true)] [ProvideService(typeof(IMenuProvider), IsAsyncQueryable = true)] [ProvideService(typeof(IGitHubServiceProvider), IsAsyncQueryable = true)] [ProvideService(typeof(IUsageTracker), IsAsyncQueryable = true)] [ProvideService(typeof(IUsageService), IsAsyncQueryable = true)] + [ProvideService(typeof(IVSGitExt), IsAsyncQueryable = true)] [ProvideService(typeof(IGitHubToolWindowManager))] [Guid(ServiceProviderPackageId)] public sealed class ServiceProviderPackage : AsyncPackage, IServiceProviderPackage, IGitHubToolWindowManager @@ -150,6 +171,7 @@ Version VSVersion protected override Task InitializeAsync(CancellationToken cancellationToken, IProgress progress) { AddService(typeof(IGitHubServiceProvider), CreateService, true); + AddService(typeof(IVSGitExt), CreateService, true); AddService(typeof(IUsageTracker), CreateService, true); AddService(typeof(IUsageService), CreateService, true); AddService(typeof(ILoginManager), CreateService, true); @@ -258,6 +280,11 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT var serviceProvider = await GetServiceAsync(typeof(IGitHubServiceProvider)) as IGitHubServiceProvider; return new UsageTracker(serviceProvider, usageService); } + else if (serviceType == typeof(IVSGitExt)) + { + var sp = await GetServiceAsync(typeof(IGitHubServiceProvider)) as IGitHubServiceProvider; + return CreateVSGitExt(sp); + } else if (serviceType == typeof(IGitHubToolWindowManager)) { return this; @@ -269,5 +296,18 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT return sp.TryGetService(serviceType); } } + + IVSGitExt CreateVSGitExt(IGitHubServiceProvider sp) + { + switch (VSVersion.Major) + { + case 14: + return new Lazy(() => new VSGitExt14(sp)).Value; + case 15: + return new Lazy(() => new VSGitExt15(sp)).Value; + default: + return null; + } + } } } diff --git a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs index 898df8857a..dbd7ba1640 100644 --- a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs +++ b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs @@ -71,6 +71,8 @@ public IServiceProvider GitServiceProvider public object TryGetService(Type t) => theRealProvider.TryGetService(t); public T TryGetService() where T : class => theRealProvider.TryGetService(); + + public Task GetServiceAsync(Type serviceType) => theRealProvider.GetServiceAsync(serviceType); } /// @@ -309,5 +311,7 @@ public void Dispose() Dispose(true); GC.SuppressFinalize(this); } + + public Task GetServiceAsync(Type serviceType) => asyncServiceProvider.GetServiceAsync(serviceType); } } From 921f8c57aaf88f58189ac0c9620b414631353b7a Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 10:59:48 +0000 Subject: [PATCH 44/56] Add generic GetServiceAsync to IGitHubServiceProvider Keep it consistent with GetService and GetService. Fix failing VSGitExt unit tests. --- src/GitHub.Exports/Services/IGitHubServiceProvider.cs | 6 +++++- src/GitHub.TeamFoundation.14/Services/VSGitExt.cs | 2 +- src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs | 7 +++++-- test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs | 6 +++--- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs index 000301d0ce..3c0809b4cc 100644 --- a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs +++ b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs @@ -1,5 +1,6 @@ using System; using System.Threading.Tasks; +using System.Diagnostics.CodeAnalysis; using System.ComponentModel.Composition.Hosting; using System.Runtime.InteropServices; using GitHub.VisualStudio; @@ -30,6 +31,9 @@ Ret GetService() where T : class /// or if it's null, the service will be removed without checking for ownership void RemoveService(Type t, object owner); - Task GetServiceAsync(Type serviceType); + Task GetServiceAsync() where T : class; + + [SuppressMessage("Microsoft.Design", "CA1004:GenericMethodsShouldProvideTypeParameter")] + Task GetServiceAsync() where T : class where Ret : class; } } diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 478aa5fcbe..1b852dfa89 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -96,7 +96,7 @@ async Task ContextChangedAsync() async Task TryInitialize() { - gitService = (IGitExt)await serviceProvider.GetServiceAsync(typeof(IGitExt)); + gitService = await serviceProvider.GetServiceAsync(); if (gitService != null) { gitService.PropertyChanged += (s, e) => diff --git a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs index dbd7ba1640..f9e3d2edef 100644 --- a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs +++ b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs @@ -72,7 +72,9 @@ public IServiceProvider GitServiceProvider public T TryGetService() where T : class => theRealProvider.TryGetService(); - public Task GetServiceAsync(Type serviceType) => theRealProvider.GetServiceAsync(serviceType); + public Task GetServiceAsync() where T : class => theRealProvider.GetServiceAsync(); + + public Task GetServiceAsync() where T : class where Ret : class => theRealProvider.GetServiceAsync(); } /// @@ -312,6 +314,7 @@ public void Dispose() GC.SuppressFinalize(this); } - public Task GetServiceAsync(Type serviceType) => asyncServiceProvider.GetServiceAsync(serviceType); + public async Task GetServiceAsync() where T : class => (T)await asyncServiceProvider.GetServiceAsync(typeof(T)); + public async Task GetServiceAsync() where T : class where Ret : class => (Ret)await asyncServiceProvider.GetServiceAsync(typeof(T)); } } diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index 7dfe6543c2..579f6b6a52 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -25,7 +25,7 @@ public void GetServiceIGitExt_WhenSccProviderContextIsActive(bool isActive, int var target = CreateVSGitExt(context, sp: sp); - sp.Received(expectCalls).GetService(); + sp.Received(expectCalls).GetServiceAsync(); } [TestCase(true, 1)] @@ -39,7 +39,7 @@ public void GetServiceIGitExt_WhenUIContextChanged(bool activated, int expectCal var eventArgs = new VSUIContextChangedEventArgs(activated); context.UIContextChanged += Raise.Event>(context, eventArgs); - sp.Received(expectCalls).GetService(); + sp.Received(expectCalls).GetServiceAsync(); } [Test] @@ -218,7 +218,7 @@ static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = nul var contextGuid = new Guid(Guids.GitSccProviderId); factory.GetUIContext(contextGuid).Returns(context); sp.GetService().Returns(factory); - sp.GetService().Returns(gitExt); + sp.GetServiceAsync().Returns(gitExt); var vsGitExt = new VSGitExt(sp, factory, repoFactory); vsGitExt.PendingTasks.Wait(); return vsGitExt; From ace5498f66a2455797853dfb945e520a4c4468f1 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 12:01:21 +0000 Subject: [PATCH 45/56] Use DTE.Version to choose IVSGitExt implementaiton --- src/GitHub.VisualStudio/GitHubPackage.cs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 1503b34fea..e01b1aab15 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -4,7 +4,6 @@ using System; using System.ComponentModel.Composition; using System.Diagnostics; -using System.Reflection; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; @@ -22,6 +21,7 @@ using Microsoft.VisualStudio; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Shell.Interop; +using EnvDTE; using Octokit; using Serilog; using Task = System.Threading.Tasks.Task; @@ -282,8 +282,9 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT } else if (serviceType == typeof(IVSGitExt)) { + var dte = await GetServiceAsync(typeof(DTE)) as DTE; var sp = await GetServiceAsync(typeof(IGitHubServiceProvider)) as IGitHubServiceProvider; - return CreateVSGitExt(sp); + return CreateVSGitExt(dte.Version, sp); } else if (serviceType == typeof(IGitHubToolWindowManager)) { @@ -297,15 +298,16 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT } } - IVSGitExt CreateVSGitExt(IGitHubServiceProvider sp) + IVSGitExt CreateVSGitExt(string dteVersion, IGitHubServiceProvider sp) { - switch (VSVersion.Major) + switch (dteVersion) { - case 14: + case "14.0": return new Lazy(() => new VSGitExt14(sp)).Value; - case 15: + case "15.0": return new Lazy(() => new VSGitExt15(sp)).Value; default: + log.Error("There is no IVSGitExt implementation for DTE version {Version}", dteVersion); return null; } } From 738b04679f196c48ab0a1bcb32c97d16b9297720 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 12:26:52 +0000 Subject: [PATCH 46/56] Add some comments --- src/GitHub.Exports/Services/IGitHubServiceProvider.cs | 10 ++++++++++ src/GitHub.TeamFoundation.14/Services/VSGitExt.cs | 2 +- src/GitHub.VisualStudio/GitHubPackage.cs | 1 + 3 files changed, 12 insertions(+), 1 deletion(-) diff --git a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs index 3c0809b4cc..91145c2321 100644 --- a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs +++ b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs @@ -31,8 +31,18 @@ Ret GetService() where T : class /// or if it's null, the service will be removed without checking for ownership void RemoveService(Type t, object owner); + /// + /// Gets a Visual Studio service asynchronously. + /// + /// The type of the service. Task GetServiceAsync() where T : class; + /// + /// Gets a Visual Studio service asynchronously. + /// + /// The type of the service. + /// The type of the service instance to return. + /// [SuppressMessage("Microsoft.Design", "CA1004:GenericMethodsShouldProvideTypeParameter")] Task GetServiceAsync() where T : class where Ret : class; } diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 1b852dfa89..34931fdcad 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -17,7 +17,7 @@ namespace GitHub.VisualStudio.Base /// /// /// Initialization for this service will be done asynchronously and the service will be - /// retrieved on the Main thread. This means the service can be constructed and subscribed to on a background thread. + /// retrieved using . This means the service can be constructed and subscribed to from a background thread. /// public class VSGitExt : IVSGitExt { diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index e01b1aab15..83b7445695 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -300,6 +300,7 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT IVSGitExt CreateVSGitExt(string dteVersion, IGitHubServiceProvider sp) { + // DTE.Version always ends with ".0" even for later minor versions. switch (dteVersion) { case "14.0": From f3865b1f173c9a5e2603dde4252c9be996d91916 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 14:19:34 +0000 Subject: [PATCH 47/56] Use a delegate for GetServiceAsync Rather than extending IGitHubServiceProvider, pass a delegate for GetServiceAsync. --- .../Services/IGitHubServiceProvider.cs | 17 -------------- .../Services/VSGitExt.cs | 12 +++++----- src/GitHub.VisualStudio/GitHubPackage.cs | 9 ++++---- .../Services/GitHubServiceProvider.cs | 7 ------ .../GitHub.TeamFoundation/VSGitExtTests.cs | 22 +++++++++++-------- 5 files changed, 23 insertions(+), 44 deletions(-) diff --git a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs index 91145c2321..c1d142ed7f 100644 --- a/src/GitHub.Exports/Services/IGitHubServiceProvider.cs +++ b/src/GitHub.Exports/Services/IGitHubServiceProvider.cs @@ -1,6 +1,4 @@ using System; -using System.Threading.Tasks; -using System.Diagnostics.CodeAnalysis; using System.ComponentModel.Composition.Hosting; using System.Runtime.InteropServices; using GitHub.VisualStudio; @@ -30,20 +28,5 @@ Ret GetService() where T : class /// The owner, which either has to match what was passed to AddService, /// or if it's null, the service will be removed without checking for ownership void RemoveService(Type t, object owner); - - /// - /// Gets a Visual Studio service asynchronously. - /// - /// The type of the service. - Task GetServiceAsync() where T : class; - - /// - /// Gets a Visual Studio service asynchronously. - /// - /// The type of the service. - /// The type of the service instance to return. - /// - [SuppressMessage("Microsoft.Design", "CA1004:GenericMethodsShouldProvideTypeParameter")] - Task GetServiceAsync() where T : class where Ret : class; } } diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 34931fdcad..0c0b6f3376 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -23,7 +23,7 @@ public class VSGitExt : IVSGitExt { static readonly ILogger log = LogManager.ForContext(); - readonly IGitHubServiceProvider serviceProvider; + readonly Func> getServiceAsync; readonly IVSUIContext context; readonly ILocalRepositoryModelFactory repositoryFactory; @@ -31,14 +31,14 @@ public class VSGitExt : IVSGitExt IReadOnlyList activeRepositories; [ImportingConstructor] - public VSGitExt(IGitHubServiceProvider serviceProvider) - : this(serviceProvider, new VSUIContextFactory(), new LocalRepositoryModelFactory()) + public VSGitExt(Func> getServiceAsync) + : this(getServiceAsync, new VSUIContextFactory(), new LocalRepositoryModelFactory()) { } - public VSGitExt(IGitHubServiceProvider serviceProvider, IVSUIContextFactory factory, ILocalRepositoryModelFactory repositoryFactory) + public VSGitExt(Func> getServiceAsync, IVSUIContextFactory factory, ILocalRepositoryModelFactory repositoryFactory) { - this.serviceProvider = serviceProvider; + this.getServiceAsync = getServiceAsync; this.repositoryFactory = repositoryFactory; // The IGitExt service isn't available when a TFS based solution is opened directly. @@ -96,7 +96,7 @@ async Task ContextChangedAsync() async Task TryInitialize() { - gitService = await serviceProvider.GetServiceAsync(); + gitService = (IGitExt)await getServiceAsync(typeof(IGitExt)); if (gitService != null) { gitService.PropertyChanged += (s, e) => diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 83b7445695..d4eaa28f45 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -283,8 +283,7 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT else if (serviceType == typeof(IVSGitExt)) { var dte = await GetServiceAsync(typeof(DTE)) as DTE; - var sp = await GetServiceAsync(typeof(IGitHubServiceProvider)) as IGitHubServiceProvider; - return CreateVSGitExt(dte.Version, sp); + return CreateVSGitExt(dte.Version); } else if (serviceType == typeof(IGitHubToolWindowManager)) { @@ -298,15 +297,15 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT } } - IVSGitExt CreateVSGitExt(string dteVersion, IGitHubServiceProvider sp) + IVSGitExt CreateVSGitExt(string dteVersion) { // DTE.Version always ends with ".0" even for later minor versions. switch (dteVersion) { case "14.0": - return new Lazy(() => new VSGitExt14(sp)).Value; + return new Lazy(() => new VSGitExt14(GetServiceAsync)).Value; case "15.0": - return new Lazy(() => new VSGitExt15(sp)).Value; + return new Lazy(() => new VSGitExt15(GetServiceAsync)).Value; default: log.Error("There is no IVSGitExt implementation for DTE version {Version}", dteVersion); return null; diff --git a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs index f9e3d2edef..898df8857a 100644 --- a/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs +++ b/src/GitHub.VisualStudio/Services/GitHubServiceProvider.cs @@ -71,10 +71,6 @@ public IServiceProvider GitServiceProvider public object TryGetService(Type t) => theRealProvider.TryGetService(t); public T TryGetService() where T : class => theRealProvider.TryGetService(); - - public Task GetServiceAsync() where T : class => theRealProvider.GetServiceAsync(); - - public Task GetServiceAsync() where T : class where Ret : class => theRealProvider.GetServiceAsync(); } /// @@ -313,8 +309,5 @@ public void Dispose() Dispose(true); GC.SuppressFinalize(this); } - - public async Task GetServiceAsync() where T : class => (T)await asyncServiceProvider.GetServiceAsync(typeof(T)); - public async Task GetServiceAsync() where T : class where Ret : class => (Ret)await asyncServiceProvider.GetServiceAsync(typeof(T)); } } diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index 579f6b6a52..e1007ece40 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -21,11 +21,11 @@ public class TheConstructor : TestBaseClass public void GetServiceIGitExt_WhenSccProviderContextIsActive(bool isActive, int expectCalls) { var context = CreateVSUIContext(isActive); - var sp = Substitute.For(); + var sp = Substitute.For(); var target = CreateVSGitExt(context, sp: sp); - sp.Received(expectCalls).GetServiceAsync(); + sp.Received(expectCalls).GetServiceAsync(typeof(IGitExt)); } [TestCase(true, 1)] @@ -33,13 +33,13 @@ public void GetServiceIGitExt_WhenSccProviderContextIsActive(bool isActive, int public void GetServiceIGitExt_WhenUIContextChanged(bool activated, int expectCalls) { var context = CreateVSUIContext(false); - var sp = Substitute.For(); + var sp = Substitute.For(); var target = CreateVSGitExt(context, sp: sp); var eventArgs = new VSUIContextChangedEventArgs(activated); context.UIContextChanged += Raise.Event>(context, eventArgs); - sp.Received(expectCalls).GetServiceAsync(); + sp.Received(expectCalls).GetServiceAsync(typeof(IGitExt)); } [Test] @@ -207,23 +207,27 @@ static IReadOnlyList CreateActiveRepositories(params string[ return repositories.AsReadOnly(); } - static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = null, IGitHubServiceProvider sp = null, + static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = null, IAsyncServiceProvider sp = null, ILocalRepositoryModelFactory repoFactory = null) { context = context ?? CreateVSUIContext(true); gitExt = gitExt ?? CreateGitExt(); - sp = sp ?? Substitute.For(); + sp = sp ?? Substitute.For(); repoFactory = repoFactory ?? Substitute.For(); var factory = Substitute.For(); var contextGuid = new Guid(Guids.GitSccProviderId); factory.GetUIContext(contextGuid).Returns(context); - sp.GetService().Returns(factory); - sp.GetServiceAsync().Returns(gitExt); - var vsGitExt = new VSGitExt(sp, factory, repoFactory); + sp.GetServiceAsync(typeof(IGitExt)).Returns(gitExt); + var vsGitExt = new VSGitExt(sp.GetServiceAsync, factory, repoFactory); vsGitExt.PendingTasks.Wait(); return vsGitExt; } + public interface IAsyncServiceProvider + { + Task GetServiceAsync(Type serviceType); + } + static IGitExt CreateGitExt(params string[] repositoryPaths) { var gitExt = Substitute.For(); From cc6a112784322ef486c243a3e15ef41ffd1c599c Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 15:01:57 +0000 Subject: [PATCH 48/56] Move VSGitExtPart into VSGitExtDispatcher --- .../GitHub.VisualStudio.csproj | 1 + src/GitHub.VisualStudio/GitHubPackage.cs | 15 --------------- .../Services/VSGitExtDispatcher.cs | 19 +++++++++++++++++++ 3 files changed, 20 insertions(+), 15 deletions(-) create mode 100644 src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index c7d3879d04..9181219a36 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -328,6 +328,7 @@ + diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index d4eaa28f45..d9397d8ad0 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -112,21 +112,6 @@ public GHClient(IProgram program) } } - [PartCreationPolicy(CreationPolicy.Shared)] - public class VSGitExtPart - { - readonly IGitHubServiceProvider serviceProvider; - - [ImportingConstructor] - public VSGitExtPart(IGitHubServiceProvider serviceProvider) - { - this.serviceProvider = serviceProvider; - } - - [Export(typeof(IVSGitExt))] - public IVSGitExt VSGitExt => serviceProvider.GetService(); - } - [PackageRegistration(UseManagedResourcesOnly = true, AllowsBackgroundLoading = true)] [ProvideService(typeof(ILoginManager), IsAsyncQueryable = true)] [ProvideService(typeof(IMenuProvider), IsAsyncQueryable = true)] diff --git a/src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs b/src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs new file mode 100644 index 0000000000..b6cda6e6d1 --- /dev/null +++ b/src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs @@ -0,0 +1,19 @@ +using System.ComponentModel.Composition; + +namespace GitHub.Services +{ + [PartCreationPolicy(CreationPolicy.Shared)] + public class VSGitExtDispatcher + { + readonly IGitHubServiceProvider serviceProvider; + + [ImportingConstructor] + public VSGitExtDispatcher(IGitHubServiceProvider serviceProvider) + { + this.serviceProvider = serviceProvider; + } + + [Export(typeof(IVSGitExt))] + public IVSGitExt VSGitExt => serviceProvider.GetService(); + } +} From e51768746c89c8fe7fa7317d26cde474d9b14836 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 15:20:02 +0000 Subject: [PATCH 49/56] Move CreateVSGitExt into VSGitExtFactory --- .../GitHub.VisualStudio.csproj | 1 + src/GitHub.VisualStudio/GitHubPackage.cs | 24 ++------------ .../Services/VSGitExtFactory.cs | 32 +++++++++++++++++++ 3 files changed, 35 insertions(+), 22 deletions(-) create mode 100644 src/GitHub.VisualStudio/Services/VSGitExtFactory.cs diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index 9181219a36..4096db69b9 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -328,6 +328,7 @@ + diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index d9397d8ad0..c3d87d535d 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -1,7 +1,4 @@ -extern alias TF14; -extern alias TF15; - -using System; +using System; using System.ComponentModel.Composition; using System.Diagnostics; using System.Runtime.InteropServices; @@ -25,8 +22,6 @@ using Octokit; using Serilog; using Task = System.Threading.Tasks.Task; -using VSGitExt14 = TF14.GitHub.VisualStudio.Base.VSGitExt; -using VSGitExt15 = TF15.GitHub.VisualStudio.Base.VSGitExt; namespace GitHub.VisualStudio { @@ -268,7 +263,7 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT else if (serviceType == typeof(IVSGitExt)) { var dte = await GetServiceAsync(typeof(DTE)) as DTE; - return CreateVSGitExt(dte.Version); + return VSGitExtFactory.Create(dte.Version, this); } else if (serviceType == typeof(IGitHubToolWindowManager)) { @@ -281,20 +276,5 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT return sp.TryGetService(serviceType); } } - - IVSGitExt CreateVSGitExt(string dteVersion) - { - // DTE.Version always ends with ".0" even for later minor versions. - switch (dteVersion) - { - case "14.0": - return new Lazy(() => new VSGitExt14(GetServiceAsync)).Value; - case "15.0": - return new Lazy(() => new VSGitExt15(GetServiceAsync)).Value; - default: - log.Error("There is no IVSGitExt implementation for DTE version {Version}", dteVersion); - return null; - } - } } } diff --git a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs new file mode 100644 index 0000000000..08632012c5 --- /dev/null +++ b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs @@ -0,0 +1,32 @@ +extern alias TF14; +extern alias TF15; + +using System; +using GitHub.Logging; +using Serilog; +using VSGitExt14 = TF14.GitHub.VisualStudio.Base.VSGitExt; +using VSGitExt15 = TF15.GitHub.VisualStudio.Base.VSGitExt; +using Microsoft.VisualStudio.Shell; + +namespace GitHub.Services +{ + public class VSGitExtFactory + { + static readonly ILogger log = LogManager.ForContext(); + + public static IVSGitExt Create(string dteVersion, IAsyncServiceProvider sp) + { + // DTE.Version always ends with ".0" even for later minor versions. + switch (dteVersion) + { + case "14.0": + return new Lazy(() => new VSGitExt14(sp.GetServiceAsync)).Value; + case "15.0": + return new Lazy(() => new VSGitExt15(sp.GetServiceAsync)).Value; + default: + log.Error("There is no IVSGitExt implementation for DTE version {Version}", dteVersion); + return null; + } + } + } +} From b98cc52f86d696151ba5bb7444dcc42587aab1a9 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 15:27:04 +0000 Subject: [PATCH 50/56] Merge VSGitExtDispatcher into VSGitExtFactory --- .../GitHub.VisualStudio.csproj | 1 - .../Services/VSGitExtDispatcher.cs | 19 ------------------- .../Services/VSGitExtFactory.cs | 13 +++++++++++++ 3 files changed, 13 insertions(+), 20 deletions(-) delete mode 100644 src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs diff --git a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj index 4096db69b9..056d46301a 100644 --- a/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj +++ b/src/GitHub.VisualStudio/GitHub.VisualStudio.csproj @@ -329,7 +329,6 @@ - diff --git a/src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs b/src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs deleted file mode 100644 index b6cda6e6d1..0000000000 --- a/src/GitHub.VisualStudio/Services/VSGitExtDispatcher.cs +++ /dev/null @@ -1,19 +0,0 @@ -using System.ComponentModel.Composition; - -namespace GitHub.Services -{ - [PartCreationPolicy(CreationPolicy.Shared)] - public class VSGitExtDispatcher - { - readonly IGitHubServiceProvider serviceProvider; - - [ImportingConstructor] - public VSGitExtDispatcher(IGitHubServiceProvider serviceProvider) - { - this.serviceProvider = serviceProvider; - } - - [Export(typeof(IVSGitExt))] - public IVSGitExt VSGitExt => serviceProvider.GetService(); - } -} diff --git a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs index 08632012c5..db3c39b4ee 100644 --- a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs +++ b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs @@ -2,6 +2,7 @@ extern alias TF15; using System; +using System.ComponentModel.Composition; using GitHub.Logging; using Serilog; using VSGitExt14 = TF14.GitHub.VisualStudio.Base.VSGitExt; @@ -10,10 +11,22 @@ namespace GitHub.Services { + [PartCreationPolicy(CreationPolicy.Shared)] public class VSGitExtFactory { static readonly ILogger log = LogManager.ForContext(); + readonly IGitHubServiceProvider serviceProvider; + + [ImportingConstructor] + public VSGitExtFactory(IGitHubServiceProvider serviceProvider) + { + this.serviceProvider = serviceProvider; + } + + [Export(typeof(IVSGitExt))] + public IVSGitExt VSGitExt => serviceProvider.GetService(); + public static IVSGitExt Create(string dteVersion, IAsyncServiceProvider sp) { // DTE.Version always ends with ".0" even for later minor versions. From 719e624ae2f62fb32501f553b138b8e483ebdffe Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 15:31:23 +0000 Subject: [PATCH 51/56] Move all construction logic into VSGitExtFactory.Create --- src/GitHub.VisualStudio/GitHubPackage.cs | 4 +--- src/GitHub.VisualStudio/Services/VSGitExtFactory.cs | 12 ++++++++---- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index c3d87d535d..8235511d09 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -18,7 +18,6 @@ using Microsoft.VisualStudio; using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.Shell.Interop; -using EnvDTE; using Octokit; using Serilog; using Task = System.Threading.Tasks.Task; @@ -262,8 +261,7 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT } else if (serviceType == typeof(IVSGitExt)) { - var dte = await GetServiceAsync(typeof(DTE)) as DTE; - return VSGitExtFactory.Create(dte.Version, this); + return await VSGitExtFactory.Create(this); } else if (serviceType == typeof(IGitHubToolWindowManager)) { diff --git a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs index db3c39b4ee..cc2b61078b 100644 --- a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs +++ b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs @@ -2,12 +2,14 @@ extern alias TF15; using System; +using System.Threading.Tasks; using System.ComponentModel.Composition; using GitHub.Logging; using Serilog; +using EnvDTE; +using Microsoft.VisualStudio.Shell; using VSGitExt14 = TF14.GitHub.VisualStudio.Base.VSGitExt; using VSGitExt15 = TF15.GitHub.VisualStudio.Base.VSGitExt; -using Microsoft.VisualStudio.Shell; namespace GitHub.Services { @@ -27,17 +29,19 @@ public VSGitExtFactory(IGitHubServiceProvider serviceProvider) [Export(typeof(IVSGitExt))] public IVSGitExt VSGitExt => serviceProvider.GetService(); - public static IVSGitExt Create(string dteVersion, IAsyncServiceProvider sp) + public async static Task Create(IAsyncServiceProvider sp) { + var dte = await sp.GetServiceAsync(typeof(DTE)) as DTE; + // DTE.Version always ends with ".0" even for later minor versions. - switch (dteVersion) + switch (dte.Version) { case "14.0": return new Lazy(() => new VSGitExt14(sp.GetServiceAsync)).Value; case "15.0": return new Lazy(() => new VSGitExt15(sp.GetServiceAsync)).Value; default: - log.Error("There is no IVSGitExt implementation for DTE version {Version}", dteVersion); + log.Error("There is no IVSGitExt implementation for DTE version {Version}", dte.Version); return null; } } From af9f36405b3795ace7393d42d8da727e70bf23a7 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 16:27:19 +0000 Subject: [PATCH 52/56] Add comments to factory method Cache the call to GetService(). Use Func rather than Lazy (which was a hack). --- .../Services/VSGitExtFactory.cs | 18 ++++++++++-------- 1 file changed, 10 insertions(+), 8 deletions(-) diff --git a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs index cc2b61078b..85649c4e48 100644 --- a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs +++ b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs @@ -18,17 +18,12 @@ public class VSGitExtFactory { static readonly ILogger log = LogManager.ForContext(); - readonly IGitHubServiceProvider serviceProvider; - [ImportingConstructor] public VSGitExtFactory(IGitHubServiceProvider serviceProvider) { - this.serviceProvider = serviceProvider; + VSGitExt = serviceProvider.GetService(); } - [Export(typeof(IVSGitExt))] - public IVSGitExt VSGitExt => serviceProvider.GetService(); - public async static Task Create(IAsyncServiceProvider sp) { var dte = await sp.GetServiceAsync(typeof(DTE)) as DTE; @@ -37,13 +32,20 @@ public async static Task Create(IAsyncServiceProvider sp) switch (dte.Version) { case "14.0": - return new Lazy(() => new VSGitExt14(sp.GetServiceAsync)).Value; + return Create(() => new VSGitExt14(sp.GetServiceAsync)); case "15.0": - return new Lazy(() => new VSGitExt15(sp.GetServiceAsync)).Value; + return Create(() => new VSGitExt15(sp.GetServiceAsync)); default: log.Error("There is no IVSGitExt implementation for DTE version {Version}", dte.Version); return null; } } + + // NOTE: We're being careful to only reference VSGitExt14 and VSGitExt15 from inside a lambda expression. + // This ensures that only the type that's compatible with the running DTE version is loaded. + static IVSGitExt Create(Func factory) => factory.Invoke(); + + [Export] + public IVSGitExt VSGitExt { get; } } } From fc06da7c0eac08bb73d5ea7f18802ab10acca85d Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Wed, 28 Feb 2018 16:47:08 +0000 Subject: [PATCH 53/56] Use ApplicationInfo.GetHostVersionInfo not DTE.Version --- src/GitHub.VisualStudio/GitHubPackage.cs | 3 ++- .../Services/VSGitExtFactory.cs | 16 ++++++---------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/src/GitHub.VisualStudio/GitHubPackage.cs b/src/GitHub.VisualStudio/GitHubPackage.cs index 8235511d09..04d896c6aa 100644 --- a/src/GitHub.VisualStudio/GitHubPackage.cs +++ b/src/GitHub.VisualStudio/GitHubPackage.cs @@ -261,7 +261,8 @@ async Task CreateService(IAsyncServiceContainer container, CancellationT } else if (serviceType == typeof(IVSGitExt)) { - return await VSGitExtFactory.Create(this); + var vsVersion = ApplicationInfo.GetHostVersionInfo().FileMajorPart; + return VSGitExtFactory.Create(vsVersion, this); } else if (serviceType == typeof(IGitHubToolWindowManager)) { diff --git a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs index 85649c4e48..88bba6a1bb 100644 --- a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs +++ b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs @@ -2,11 +2,10 @@ extern alias TF15; using System; -using System.Threading.Tasks; using System.ComponentModel.Composition; +using GitHub.Info; using GitHub.Logging; using Serilog; -using EnvDTE; using Microsoft.VisualStudio.Shell; using VSGitExt14 = TF14.GitHub.VisualStudio.Base.VSGitExt; using VSGitExt15 = TF15.GitHub.VisualStudio.Base.VSGitExt; @@ -24,19 +23,16 @@ public VSGitExtFactory(IGitHubServiceProvider serviceProvider) VSGitExt = serviceProvider.GetService(); } - public async static Task Create(IAsyncServiceProvider sp) + public static IVSGitExt Create(int vsVersion, IAsyncServiceProvider sp) { - var dte = await sp.GetServiceAsync(typeof(DTE)) as DTE; - - // DTE.Version always ends with ".0" even for later minor versions. - switch (dte.Version) + switch (vsVersion) { - case "14.0": + case 14: return Create(() => new VSGitExt14(sp.GetServiceAsync)); - case "15.0": + case 15: return Create(() => new VSGitExt15(sp.GetServiceAsync)); default: - log.Error("There is no IVSGitExt implementation for DTE version {Version}", dte.Version); + log.Error("There is no IVSGitExt implementation for DTE version {Version}", vsVersion); return null; } } From ee9ab4af7f71ad9c604a4013ba7b87836a5cffb8 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Fri, 2 Mar 2018 16:59:36 +0000 Subject: [PATCH 54/56] Simplify the VSGitExt service - Use `UIContext.WhenActivated` instead of UIContext.UIContextChanged event - Don't use async InitializeAsync now we're initializing on background thread - No more need to use ContinueWith --- .../Services/IVSUIContextFactory.cs | 2 +- .../Services/VSGitExt.cs | 90 +++++-------------- .../Services/VSUIContextFactory.cs | 28 +----- .../GitHub.TeamFoundation/VSGitExtTests.cs | 47 +++++++--- 4 files changed, 63 insertions(+), 104 deletions(-) diff --git a/src/GitHub.Exports/Services/IVSUIContextFactory.cs b/src/GitHub.Exports/Services/IVSUIContextFactory.cs index e3aca6cbb7..57c6542d9b 100644 --- a/src/GitHub.Exports/Services/IVSUIContextFactory.cs +++ b/src/GitHub.Exports/Services/IVSUIContextFactory.cs @@ -20,6 +20,6 @@ public VSUIContextChangedEventArgs(bool activated) public interface IVSUIContext { bool IsActive { get; } - event EventHandler UIContextChanged; + void WhenActivated(Action action); } } \ No newline at end of file diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 0c0b6f3376..5626ad4cd1 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -24,8 +24,8 @@ public class VSGitExt : IVSGitExt static readonly ILogger log = LogManager.ForContext(); readonly Func> getServiceAsync; - readonly IVSUIContext context; readonly ILocalRepositoryModelFactory repositoryFactory; + readonly object refreshLock = new object(); IGitExt gitService; IReadOnlyList activeRepositories; @@ -41,94 +41,50 @@ public VSGitExt(Func> getServiceAsync, IVSUIContextFactory fa this.getServiceAsync = getServiceAsync; this.repositoryFactory = repositoryFactory; - // The IGitExt service isn't available when a TFS based solution is opened directly. - // It will become available when moving to a Git based solution (and cause a UIContext event to fire). - context = factory.GetUIContext(new Guid(Guids.GitSccProviderId)); - // Start with empty array until we have a chance to initialize. ActiveRepositories = Array.Empty(); - PendingTasks = InitializeAsync(); - } - - async Task InitializeAsync() - { - try - { - if (!context.IsActive || !await TryInitialize()) - { - // If we're not in the UIContext or TryInitialize fails, have another go when the UIContext changes. - context.UIContextChanged += ContextChanged; - log.Debug("VSGitExt will be initialized later"); - } - } - catch (Exception e) - { - log.Error(e, "Initializing"); - } - } - - void ContextChanged(object sender, VSUIContextChangedEventArgs e) - { - if (e.Activated) - { - PendingTasks = ContextChangedAsync(); - } + // The IGitExt service isn't available when a TFS based solution is opened directly. + // It will become available when moving to a Git based solution (and cause a UIContext event to fire). + var context = factory.GetUIContext(new Guid(Guids.GitSccProviderId)); + context.WhenActivated(() => Initialize()); } - async Task ContextChangedAsync() + void Initialize() { - try + PendingTasks = getServiceAsync(typeof(IGitExt)).ContinueWith(t => { - // If we're in the UIContext and TryInitialize succeeds, we can stop listening for events. - // NOTE: this event can fire with UIContext=true in a TFS solution (not just Git). - if (await TryInitialize()) + gitService = (IGitExt)t.Result; + if (gitService == null) { - context.UIContextChanged -= ContextChanged; - log.Debug("Initialized VSGitExt on UIContextChanged"); + log.Error("Couldn't find IGitExt service"); + return; } - } - catch (Exception e) - { - log.Error(e, "UIContextChanged"); - } - } - async Task TryInitialize() - { - gitService = (IGitExt)await getServiceAsync(typeof(IGitExt)); - if (gitService != null) - { + RefreshActiveRepositories(); gitService.PropertyChanged += (s, e) => { if (e.PropertyName == nameof(gitService.ActiveRepositories)) { - // Execute tasks in sequence using thread pool (TaskScheduler.Default). - PendingTasks = PendingTasks.ContinueWith(_ => RefreshActiveRepositories(), TaskScheduler.Default); + RefreshActiveRepositories(); } }; - - // Do this after we start listening so we don't miss an event. - await Task.Run(() => RefreshActiveRepositories()); - - log.Debug("Found IGitExt service and initialized VSGitExt"); - return true; - } - - log.Error("Couldn't find IGitExt service"); - return false; + }, TaskScheduler.Default); } void RefreshActiveRepositories() { try { - log.Debug( - "IGitExt.ActiveRepositories (#{Id}) returned {Repositories}", - gitService.GetHashCode(), - gitService?.ActiveRepositories.Select(x => x.RepositoryPath)); + lock (refreshLock) + { + log.Debug( + "IGitExt.ActiveRepositories (#{Id}) returned {Repositories}", + gitService.GetHashCode(), + gitService.ActiveRepositories.Select(x => x.RepositoryPath)); - ActiveRepositories = gitService?.ActiveRepositories.Select(x => repositoryFactory.Create(x.RepositoryPath)).ToList(); + ActiveRepositories = gitService?.ActiveRepositories.Select(x => repositoryFactory.Create(x.RepositoryPath)).ToList(); + } } catch (Exception e) { @@ -160,6 +116,6 @@ private set /// /// Tasks that are pending execution on the thread pool. /// - public Task PendingTasks { get; private set; } + public Task PendingTasks { get; private set; } = Task.CompletedTask; } } \ No newline at end of file diff --git a/src/GitHub.TeamFoundation.14/Services/VSUIContextFactory.cs b/src/GitHub.TeamFoundation.14/Services/VSUIContextFactory.cs index 6cedf85cd8..3797f75c02 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSUIContextFactory.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSUIContextFactory.cs @@ -1,6 +1,4 @@ using System; -using System.Collections.Generic; -using System.ComponentModel.Composition; using Microsoft.VisualStudio.Shell; using GitHub.Services; @@ -17,8 +15,7 @@ public IVSUIContext GetUIContext(Guid contextGuid) class VSUIContext : IVSUIContext { readonly UIContext context; - readonly Dictionary, EventHandler> handlers = - new Dictionary, EventHandler>(); + public VSUIContext(UIContext context) { this.context = context; @@ -26,27 +23,6 @@ public VSUIContext(UIContext context) public bool IsActive { get { return context.IsActive; } } - public event EventHandler UIContextChanged - { - add - { - EventHandler handler = null; - if (!handlers.TryGetValue(value, out handler)) - { - handler = (s, e) => value.Invoke(s, new VSUIContextChangedEventArgs(e.Activated)); - handlers.Add(value, handler); - } - context.UIContextChanged += handler; - } - remove - { - EventHandler handler = null; - if (handlers.TryGetValue(value, out handler)) - { - handlers.Remove(value); - context.UIContextChanged -= handler; - } - } - } + public void WhenActivated(Action action) => context.WhenActivated(action); } } diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index e1007ece40..daa0dbd037 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -36,9 +36,9 @@ public void GetServiceIGitExt_WhenUIContextChanged(bool activated, int expectCal var sp = Substitute.For(); var target = CreateVSGitExt(context, sp: sp); - var eventArgs = new VSUIContextChangedEventArgs(activated); - context.UIContextChanged += Raise.Event>(context, eventArgs); + context.IsActive = activated; + target.PendingTasks.Wait(); sp.Received(expectCalls).GetServiceAsync(typeof(IGitExt)); } @@ -106,8 +106,7 @@ public void WhenUIContextChanged_ActiveRepositoriesChangedIsFired() bool wasFired = false; target.ActiveRepositoriesChanged += () => wasFired = true; - var eventArgs = new VSUIContextChangedEventArgs(true); - context.UIContextChanged += Raise.Event>(context, eventArgs); + context.IsActive = true; target.PendingTasks.Wait(); Assert.That(wasFired, Is.True); @@ -123,8 +122,7 @@ public void WhenUIContextChanged_FiredUsingThreadPoolThread() bool threadPool = false; target.ActiveRepositoriesChanged += () => threadPool = Thread.CurrentThread.IsThreadPoolThread; - var eventArgs = new VSUIContextChangedEventArgs(true); - context.UIContextChanged += Raise.Event>(context, eventArgs); + context.IsActive = true; target.PendingTasks.Wait(); Assert.That(threadPool, Is.True); @@ -236,11 +234,40 @@ static IGitExt CreateGitExt(params string[] repositoryPaths) return gitExt; } - static IVSUIContext CreateVSUIContext(bool isActive) + static MockVSUIContext CreateVSUIContext(bool isActive) { - var context = Substitute.For(); - context.IsActive.Returns(isActive); - return context; + return new MockVSUIContext { IsActive = isActive }; + } + + class MockVSUIContext : IVSUIContext + { + bool isActive; + Action action; + + public bool IsActive + { + get { return isActive; } + set + { + isActive = value; + if (isActive && action != null) + { + action.Invoke(); + action = null; + } + } + } + + public void WhenActivated(Action action) + { + if (isActive) + { + action.Invoke(); + return; + } + + this.action = action; + } } class MockGitExt : IGitExt From d72ff9b3a34d23fdad314153d5e31c60ae33b257 Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 5 Mar 2018 13:22:29 +0000 Subject: [PATCH 55/56] Use IAsyncServiceProvider rather than delegate Use IAsyncServiceProvider from the Microsoft.VisualStudio.Shell.Immutable.14.0 assembly. --- .../GitHub.TeamFoundation.14.csproj | 17 +++++++++++++++++ .../Services/VSGitExt.cs | 15 +++++++++------ src/GitHub.TeamFoundation.14/packages.config | 4 ++++ .../GitHub.TeamFoundation.15.csproj | 9 +++++++++ src/GitHub.TeamFoundation.15/packages.config | 2 ++ .../Services/VSGitExtFactory.cs | 4 ++-- .../GitHub.TeamFoundation/VSGitExtTests.cs | 12 ++++-------- test/UnitTests/UnitTests.csproj | 4 ++++ test/UnitTests/packages.config | 1 + 9 files changed, 52 insertions(+), 16 deletions(-) diff --git a/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj b/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj index 352f4c7b3b..886de819a4 100644 --- a/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj +++ b/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj @@ -79,6 +79,10 @@ ..\..\packages\Microsoft.VisualStudio.ComponentModelHost.14.0.25424\lib\net45\Microsoft.VisualStudio.ComponentModelHost.dll True + + ..\..\packages\Microsoft.VisualStudio.OLE.Interop.7.10.6070\lib\Microsoft.VisualStudio.OLE.Interop.dll + True + ..\..\packages\Microsoft.VisualStudio.Shell.14.0.14.3.25407\lib\Microsoft.VisualStudio.Shell.14.0.dll True @@ -87,6 +91,15 @@ ..\..\packages\Microsoft.VisualStudio.Shell.Immutable.10.0.10.0.30319\lib\net40\Microsoft.VisualStudio.Shell.Immutable.10.0.dll True + + ..\..\packages\Microsoft.VisualStudio.Shell.Immutable.11.0.11.0.50727\lib\net45\Microsoft.VisualStudio.Shell.Immutable.11.0.dll + True + + + ..\..\packages\Microsoft.VisualStudio.Shell.Immutable.14.0.14.3.25407\lib\net45\Microsoft.VisualStudio.Shell.Immutable.14.0.dll + True + SI14 + ..\..\packages\Microsoft.VisualStudio.Shell.Interop.7.10.6071\lib\Microsoft.VisualStudio.Shell.Interop.dll True @@ -95,6 +108,10 @@ ..\..\packages\Microsoft.VisualStudio.Shell.Interop.8.0.8.0.50727\lib\Microsoft.VisualStudio.Shell.Interop.8.0.dll True + + ..\..\packages\Microsoft.VisualStudio.TextManager.Interop.7.10.6070\lib\Microsoft.VisualStudio.TextManager.Interop.dll + True + diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 5626ad4cd1..8746ea7e82 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -1,3 +1,4 @@ +extern alias SI14; using System; using System.Linq; using System.Threading.Tasks; @@ -9,6 +10,8 @@ using GitHub.TeamFoundation.Services; using Serilog; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; +using Task = System.Threading.Tasks.Task; +using IAsyncServiceProvider = SI14::Microsoft.VisualStudio.Shell.IAsyncServiceProvider; namespace GitHub.VisualStudio.Base { @@ -23,7 +26,7 @@ public class VSGitExt : IVSGitExt { static readonly ILogger log = LogManager.ForContext(); - readonly Func> getServiceAsync; + readonly IAsyncServiceProvider asyncServiceProvider; readonly ILocalRepositoryModelFactory repositoryFactory; readonly object refreshLock = new object(); @@ -31,14 +34,14 @@ public class VSGitExt : IVSGitExt IReadOnlyList activeRepositories; [ImportingConstructor] - public VSGitExt(Func> getServiceAsync) - : this(getServiceAsync, new VSUIContextFactory(), new LocalRepositoryModelFactory()) + public VSGitExt(IAsyncServiceProvider asyncServiceProvider) + : this(asyncServiceProvider, new VSUIContextFactory(), new LocalRepositoryModelFactory()) { } - public VSGitExt(Func> getServiceAsync, IVSUIContextFactory factory, ILocalRepositoryModelFactory repositoryFactory) + public VSGitExt(IAsyncServiceProvider asyncServiceProvider, IVSUIContextFactory factory, ILocalRepositoryModelFactory repositoryFactory) { - this.getServiceAsync = getServiceAsync; + this.asyncServiceProvider = asyncServiceProvider; this.repositoryFactory = repositoryFactory; // Start with empty array until we have a chance to initialize. @@ -52,7 +55,7 @@ public VSGitExt(Func> getServiceAsync, IVSUIContextFactory fa void Initialize() { - PendingTasks = getServiceAsync(typeof(IGitExt)).ContinueWith(t => + PendingTasks = asyncServiceProvider.GetServiceAsync(typeof(IGitExt)).ContinueWith(t => { gitService = (IGitExt)t.Result; if (gitService == null) diff --git a/src/GitHub.TeamFoundation.14/packages.config b/src/GitHub.TeamFoundation.14/packages.config index acb5ea18a9..1dd7aa0c9b 100644 --- a/src/GitHub.TeamFoundation.14/packages.config +++ b/src/GitHub.TeamFoundation.14/packages.config @@ -3,10 +3,14 @@ + + + + diff --git a/src/GitHub.TeamFoundation.15/GitHub.TeamFoundation.15.csproj b/src/GitHub.TeamFoundation.15/GitHub.TeamFoundation.15.csproj index 1f24f6c7bb..12b14aa4f2 100644 --- a/src/GitHub.TeamFoundation.15/GitHub.TeamFoundation.15.csproj +++ b/src/GitHub.TeamFoundation.15/GitHub.TeamFoundation.15.csproj @@ -95,6 +95,15 @@ ..\..\packages\Microsoft.VisualStudio.Shell.Framework.15.4.27004\lib\net45\Microsoft.VisualStudio.Shell.Framework.dll True + + ..\..\packages\Microsoft.VisualStudio.Shell.Immutable.11.0.11.0.50727\lib\net45\Microsoft.VisualStudio.Shell.Immutable.11.0.dll + True + + + ..\..\packages\Microsoft.VisualStudio.Shell.Immutable.14.0.14.3.25407\lib\net45\Microsoft.VisualStudio.Shell.Immutable.14.0.dll + True + SI14 + ..\..\packages\Microsoft.VisualStudio.Shell.Interop.7.10.6071\lib\Microsoft.VisualStudio.Shell.Interop.dll True diff --git a/src/GitHub.TeamFoundation.15/packages.config b/src/GitHub.TeamFoundation.15/packages.config index fa4276e3e1..ca6ba30df0 100644 --- a/src/GitHub.TeamFoundation.15/packages.config +++ b/src/GitHub.TeamFoundation.15/packages.config @@ -8,6 +8,8 @@ + + diff --git a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs index 88bba6a1bb..6bcfe8f631 100644 --- a/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs +++ b/src/GitHub.VisualStudio/Services/VSGitExtFactory.cs @@ -28,9 +28,9 @@ public static IVSGitExt Create(int vsVersion, IAsyncServiceProvider sp) switch (vsVersion) { case 14: - return Create(() => new VSGitExt14(sp.GetServiceAsync)); + return Create(() => new VSGitExt14(sp)); case 15: - return Create(() => new VSGitExt15(sp.GetServiceAsync)); + return Create(() => new VSGitExt15(sp)); default: log.Error("There is no IVSGitExt implementation for DTE version {Version}", vsVersion); return null; diff --git a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs index daa0dbd037..c7304ad2f3 100644 --- a/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs +++ b/test/UnitTests/GitHub.TeamFoundation/VSGitExtTests.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using System.Threading; using System.ComponentModel; using System.Collections.Generic; @@ -9,8 +10,8 @@ using NUnit.Framework; using NSubstitute; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; -using System.Threading.Tasks; -using System.Linq; +using Microsoft.VisualStudio.Shell; +using Task = System.Threading.Tasks.Task; public class VSGitExtTests { @@ -216,16 +217,11 @@ static VSGitExt CreateVSGitExt(IVSUIContext context = null, IGitExt gitExt = nul var contextGuid = new Guid(Guids.GitSccProviderId); factory.GetUIContext(contextGuid).Returns(context); sp.GetServiceAsync(typeof(IGitExt)).Returns(gitExt); - var vsGitExt = new VSGitExt(sp.GetServiceAsync, factory, repoFactory); + var vsGitExt = new VSGitExt(sp, factory, repoFactory); vsGitExt.PendingTasks.Wait(); return vsGitExt; } - public interface IAsyncServiceProvider - { - Task GetServiceAsync(Type serviceType); - } - static IGitExt CreateGitExt(params string[] repositoryPaths) { var gitExt = Substitute.For(); diff --git a/test/UnitTests/UnitTests.csproj b/test/UnitTests/UnitTests.csproj index e84c3801bb..3c26b01676 100644 --- a/test/UnitTests/UnitTests.csproj +++ b/test/UnitTests/UnitTests.csproj @@ -107,6 +107,10 @@ ..\..\packages\Microsoft.VisualStudio.Shell.Immutable.12.0.12.0.21003\lib\net45\Microsoft.VisualStudio.Shell.Immutable.12.0.dll True + + ..\..\packages\Microsoft.VisualStudio.Shell.Immutable.14.0.14.3.25407\lib\net45\Microsoft.VisualStudio.Shell.Immutable.14.0.dll + True + ..\..\packages\Microsoft.VisualStudio.Shell.Interop.7.10.6071\lib\Microsoft.VisualStudio.Shell.Interop.dll True diff --git a/test/UnitTests/packages.config b/test/UnitTests/packages.config index fec6f8280f..a80e0e8c78 100644 --- a/test/UnitTests/packages.config +++ b/test/UnitTests/packages.config @@ -12,6 +12,7 @@ + From 78f9d718e2508519e00559002db179f7db98f9fa Mon Sep 17 00:00:00 2001 From: Jamie Cansdale Date: Mon, 5 Mar 2018 15:21:14 +0000 Subject: [PATCH 56/56] Give alias to ..Shell.Framework rather than ..Shell.Immutable.14 Use an alias on the up-level assembly rather than the shared one. We're likely to want to use IAsyncServiceProvider from Microsoft.VisualStudio.Shell.Immutable.14 again, so give alias to Microsoft.VisualStudio.Shell.Framework instead. --- .../GitHub.TeamFoundation.14.csproj | 2 +- src/GitHub.TeamFoundation.14/Services/VSGitExt.cs | 3 +-- .../Services/VSGitServices.cs | 11 ++++++++--- .../GitHub.TeamFoundation.15.csproj | 3 ++- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj b/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj index 886de819a4..edd4dcd07a 100644 --- a/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj +++ b/src/GitHub.TeamFoundation.14/GitHub.TeamFoundation.14.csproj @@ -98,7 +98,7 @@ ..\..\packages\Microsoft.VisualStudio.Shell.Immutable.14.0.14.3.25407\lib\net45\Microsoft.VisualStudio.Shell.Immutable.14.0.dll True - SI14 + global ..\..\packages\Microsoft.VisualStudio.Shell.Interop.7.10.6071\lib\Microsoft.VisualStudio.Shell.Interop.dll diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs index 8746ea7e82..0ddc13eba3 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitExt.cs @@ -1,4 +1,3 @@ -extern alias SI14; using System; using System.Linq; using System.Threading.Tasks; @@ -9,9 +8,9 @@ using GitHub.Logging; using GitHub.TeamFoundation.Services; using Serilog; +using Microsoft.VisualStudio.Shell; using Microsoft.VisualStudio.TeamFoundation.Git.Extensibility; using Task = System.Threading.Tasks.Task; -using IAsyncServiceProvider = SI14::Microsoft.VisualStudio.Shell.IAsyncServiceProvider; namespace GitHub.VisualStudio.Base { diff --git a/src/GitHub.TeamFoundation.14/Services/VSGitServices.cs b/src/GitHub.TeamFoundation.14/Services/VSGitServices.cs index 59b341f440..db973edd2d 100644 --- a/src/GitHub.TeamFoundation.14/Services/VSGitServices.cs +++ b/src/GitHub.TeamFoundation.14/Services/VSGitServices.cs @@ -1,4 +1,10 @@ -using System; +#if TEAMEXPLORER15 +// Microsoft.VisualStudio.Shell.Framework has an alias to avoid conflict with IAsyncServiceProvider +extern alias SF15; +using ServiceProgressData = SF15::Microsoft.VisualStudio.Shell.ServiceProgressData; +#endif + +using System; using System.Collections.Generic; using System.ComponentModel.Composition; using System.Globalization; @@ -84,8 +90,7 @@ public async Task Clone( await gitExt.WhenAnyValue(x => x.CanClone).Where(x => x).Take(1); #else var gitExt = serviceProvider.GetService(); - var typedProgress = ((Progress)progress) ?? - new Progress(); + var typedProgress = ((Progress)progress) ?? new Progress(); await Microsoft.VisualStudio.Shell.ThreadHelper.JoinableTaskFactory.RunAsync(async () => { diff --git a/src/GitHub.TeamFoundation.15/GitHub.TeamFoundation.15.csproj b/src/GitHub.TeamFoundation.15/GitHub.TeamFoundation.15.csproj index 12b14aa4f2..059045544e 100644 --- a/src/GitHub.TeamFoundation.15/GitHub.TeamFoundation.15.csproj +++ b/src/GitHub.TeamFoundation.15/GitHub.TeamFoundation.15.csproj @@ -94,6 +94,7 @@ ..\..\packages\Microsoft.VisualStudio.Shell.Framework.15.4.27004\lib\net45\Microsoft.VisualStudio.Shell.Framework.dll True + SF15 ..\..\packages\Microsoft.VisualStudio.Shell.Immutable.11.0.11.0.50727\lib\net45\Microsoft.VisualStudio.Shell.Immutable.11.0.dll @@ -102,7 +103,7 @@ ..\..\packages\Microsoft.VisualStudio.Shell.Immutable.14.0.14.3.25407\lib\net45\Microsoft.VisualStudio.Shell.Immutable.14.0.dll True - SI14 + global ..\..\packages\Microsoft.VisualStudio.Shell.Interop.7.10.6071\lib\Microsoft.VisualStudio.Shell.Interop.dll