[Feature] Show current PR on status bar #1396
Conversation
This isn't pretty but gets the UI control to where we want it.
Initialize with InlineReviewsPackage. This should probably initialize earlier from its own package.
Remove view when there is no PR session.
Show current PR on status bar and allow navigation to PR details MVP
Keep it consistent with GetService<T> and GetService<T, Ret>. Fix failing VSGitExt unit tests.
Rather than extending IGitHubServiceProvider, pass a delegate for GetServiceAsync.
Cache the call to GetService<IVSGitExt>(). Use Func<IVSGitExt> rather than Lazy<IVSGitExt> (which was a hack).
| this.usageTracker = usageTracker; | ||
| this.serviceProvider = serviceProvider; | ||
|
|
||
| OnActiveRepositoriesChanged(); |
There was a problem hiding this comment.
This might be called twice!
|
Just a heads up, I want to focus on some stuff around the code review workflow and won't be able to fix the icon color soon. Since it's not a critical blocker for this feature, we could consider merging if this PR looks good otherwise, and I can address it in a separate issue / PR. |
- 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
Simplify the VSGitExt service
…vice Expose IVSGitExt as async VS service
| public VSGitExt(IGitHubServiceProvider serviceProvider) | ||
| : this(serviceProvider, new VSUIContextFactory(), new LocalRepositoryModelFactory()) | ||
| public VSGitExt(Func<Type, Task<object>> getServiceAsync) | ||
| : this(getServiceAsync, new VSUIContextFactory(), new LocalRepositoryModelFactory()) |
There was a problem hiding this comment.
Any reason to send a callback instead of sending in IAsyncServiceProvider (which provides GetServiceAsync) here?
There was a problem hiding this comment.
The issue was that AsyncPackage implements 3 interfaces called IAsyncServiceProvider, 2 of which exist in the same namespace. Passing a delegate was a way to avoid taking a new dependency on Microsoft.VisualStudio.Shell.Immutable.14.0 and needing to disambiguate using extern alias (which works but is a bit messy).
I've pushed a commit that uses the interface from Microsoft.VisualStudio.Shell.Immutable.14.0. If we're likely to be using IAsyncServiceProvider elsewhere, having the correct assembly to hand is maybe a good thing (even if we need to use extern alias).
An alternative approach would be to use the lower level Interop.IAsyncServiceProvider interface from here:
namespace Microsoft.VisualStudio.Shell.Interop
{
public interface IAsyncServiceProvider
{
IVsTask QueryServiceAsync([ComAliasName("OLE.REFGUID")] ref Guid guidService);
}
}
The fact that this takes a Guid and returns a IVsTask again makes it a bit messy.
My instinct is to go with this last commit. What do you think?
Use IAsyncServiceProvider from the Microsoft.VisualStudio.Shell.Immutable.14.0 assembly.
Which two assemblies provide the same type? |
|
It's a collision between I'll see what happens if I remove the |
|
Ah, we're forced to reference
|
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.
|
I've changed it so that rather than giving an alias to the shared This will make finding the correct |
The following PRs target this one:
Functionality
To do
[ProvideAutoLoad(Guids.GitSccProviderId)]?Use PR number from branch metadata?IPullRequestSessionManager, see https://github.com/github/VisualStudio/pull/1388/files#diff-8f8b4cd719001e4a8dd4b96e211243a8R53Related issues