Async Tool Window Helpers

[reduckted]

Creating async tool windows is a pain. There are three methods in the AsyncPackage that you need to override, and it's not obvious that you should override all three of those methods. Plus there are some overridable methods for non-async tool windows, which adds to the confusion.

There's also some boilerplate code for creating/showing the tool window (which differs between VS 15 and 16) that could be put in a helper method.

Proposed API

Registering a Tool Window

We will need a new base class for packages to inherit from. This will provide helper methods for registering the tool window.

This base class would override the GetAsyncToolWindowFactory, GetToolWindowTitle and InitializeToolWindowAsync methods (and the equivalents for VS15) to take care of the work required to create a tool window.

abstract class ToolkitPackage : AsyncPackage {
    void AddToolWindow<T>(string title);
    void AddToolWindow<T>(string title, Func<object> initializer);
    void AddToolWindow<T>(string title, Func<Taks<object>> initializer);
}

The AddToolWindow method would be called by the derived class in the InitializeAsync method (similar to how you would use the existing AsyncPackage.AddOptionKey and AsyncPackage.AddService methods).

The generic parameter T is the type of the tool window. This would be used by GetAsyncToolWindowFactory. The title would be used by GetToolWindowTitle, and the initializer function would be used by InitializeToolWindowAsync.

Opening a Tool Window

The boilerplate code for opening a tool window can also be put in the new ToolkitPackage.

abstract class ToolkitPackage : AsyncPackage {
    public Task<T> ShowToolWindowAsync<T>(CancellationToken cancellationToken = default);
}

Examples

Adding a Tool Window

public sealed class TestExtensionPackage : ToolkitPackage
{
    protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress<ServiceProgressData> progress)
    {
        AddToolWindow<RunnerWindow>(RunnerWindow.Title, async () =>
        {
            await JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);
            return await VS.GetDTEAsync();
        });

        AddToolWindow<ThemeWindow>(ThemeWindow.Title, () => new ThemeWindowControlViewModel());
    }
}

Opening a Tool Window

protected override async Task ExecuteAsync(OleMenuCmdEventArgs e)
{
    await Package.ShowToolWindowAsync<RunnerWindow>();
}

Notes

The ToolkitPackage would need to be passed around everywhere instead of an AsyncPackage so that the helper method to open a tool window is accessible. For example, CommandBase.InitializeAsync would need to take a ToolkitPackage rather than an AsyncPackage so that the derived classes can use ShowToolWindowAsync.

Prototype

I've thrown together a prototype here: https://github.com/madskristensen/Community.VisualStudio.Toolkit/compare/master...reduckted:prototype/async-tool-window

If you like this idea, I'll clean it up and create a pull request.

[madskristensen]

This is good stuff. I've thought about having a Toolkit implementation of AsyncPackage for some other reasons too.

For opening a tool window, an extension method on the AsyncPackage class would be super helpful and easy to do.

In regard to this sample:

AddToolWindow<RunnerWindow>(RunnerWindow.Title, async () =>{
     await JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);
     return await VS.GetDTEAsync();
});

I've always felt it weird that the package class would be responsible for handling async initialization of tool windows. It doesn't do that with commands. What if the tool window had a static InitializeAsync method just like commands do? We could have a BaseToolWindow<T> to keep it consistent with commands.

That way, to initialize a tool window from the Package class' InitializeAsync method, it would look like this:

await MyToolWindow.InitializeAsync(this);

For this to work, we'd still need a new toolkit package base class or AsyncToolkitPackage.

Thoughts?

[yannduran]

Yes, consistency is good! Especially so for developers who are new to writing extensions.

A toolkit of AsyncPackage is also a good idea. I've had base package classes in 'Luminous Code' for years for these types of reasons.

[madskristensen]

Come to think of it, the BaseToolWindow<T> class should also expose static Show and/or ShowAsync methods. It's weird to have to go to the package to ask it to show the tool window. You don't do that with invoking commands, for example.

[reduckted]

What if the tool window had a static InitializeAsync method just like commands do? We could have a BaseToolWindow<T> to keep it consistent with commands.

The problem with that is the tool windows are not created and initialized until they are shown. And the AsyncPackage.InitializeToolWindowAsync() method is called before the tool window is actually constructed. From what I can gather, it works like this for async tool windows (synchronous tool windows are different):

1. AsyncPackage.ShowToolWindowAsync is called.
2. If the tool window hasn't been created yet, the AsyncPackage.GetAsyncToolWindowFactory is called. That typically returns this.
3. AsyncPackage.GetToolWindowTitle is called so that a placeholder tool window pane can be created with the title of the tool window.
4. The placeholder tool window is shown, presumably showing some sort of "the tool window is loading" message.
5. AsyncPackage.InitializeToolWindowAsync is called. That method can create an object that will be passed to the constructor of the real tool window.
6. The real tool window is created, passing it the result of AsyncPackage.InitializeToolWindowAsync.
7. Somehow the placeholder tool window is replaced with the real tool window.

So there are a few of issues with having a static InitializeAsync method.

1. Being a static method, there's no way to get the title of the tool window without using reflection. We could require a convention where the title of the tool window is stored in a constant or static field called Title, but that's a bit ugly.
2. There's nowhere for the actual async initialization method to live (the one that the package would call from AsyncPackage.InitializeToolWindowAsync), because it can't be an instance method on the tool window, because the tool window hasn't been created at that point.

But, I've had a go at doing something similar. Hopefully it's not too messy to understand what's going on. You can see it here: https://github.com/madskristensen/Community.VisualStudio.Toolkit/compare/master...reduckted:prototype/initialize-tool-window

To summarize,

• BaseToolWindow<TWindow, TData> is what you inherit to define a tool window (I'll just call it BaseToolWindow from now on). This acts more as a "provider" of the tool window, rather than the actual tool window. It doesn't inherit ToolWindowPane. There's a private class that is used for that.
• The static BaseToolWindow.Initialize() method registers the tool window in the package.
• The package methods do some magic to "intercept" the calls to create a tool window of type BaseToolWindow and replace the type with the private ToolWindowPane implementation.

It works for VS 16, but does not work for VS 15. From what I can see by decompiling the Microsoft.VisualStudio.Shell.15.0 assembly, I'm not sure we'll ever be able to get it to work.

In VS 16 there's a separation between the type of ToolWindowPane being created, and the GUID of the tool window type. This works well for us because the BaseToolWindow has the GUID on it, and that's the type used in the ProvideToolWindow attribute. The private ToolWindowPane implementation doesn't have a GUID.

In VS 15 there is no separation. Creating the tool window using the private ToolWindowPane sort of works for the first tool window created. The tool window is created, but it's not positioned or sized where you would expect. Opening a different tool window just results in the first tool window being shown again. I haven't looked into it too much, but I suspect it's to do with the private ToolWindowPane not having a GUID, so all tool windows appear to be the same type.

[yannduran]

I guess that explains why the initialisation is currently done in the package instead of the same way as commands.

Personally, apart from preferring consistency, I have no problem if it stays in the Packageclass.

[madskristensen]

Yeah, that's a tough one. Thanks @reduckted for looking into it. Async tool windows were introduced in 15.6, so they are new and there is no back compat story for them. Perhaps we can just show the best practice in the template by putting a static method on the tool window class to initialize the data and then call that from the package class.

[madskristensen]

Yeah, that's a tough one. Thanks @reduckted for looking into it. Async tool windows were introduced in 15.6, so they are new and there is no back compat story for them. Perhaps we can just show the best practice in the template by adding a static method that initializes the data and then call that method from the package class.

[reduckted]

Here's another attempt: https://github.com/madskristensen/Community.VisualStudio.Toolkit/compare/master...reduckted:e9fb3857bcba3337043a045d557ff8e947775fb9

I like this one. It does require an additional class to be defined, and the type that you specify in the ProvideToolWindow attribute isn't the same class that you would consider to be the "tool window" class, but I think the code that the item template could generate would help to guide users.

You define a tool window like this:

public class TestToolWindow : BaseToolWindow<TestToolWindow, MyData>
{
    public override string Title => "Test Window";

    public override Type PaneType => typeof(Pane);

    protected override async Task<MyData> CreateDataAsync(CancellationToken cancellationToken)
    {
        await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);
        return await VS.GetServiceAsync<MyData, MyData>();
    }

    protected override object CreateContent(MyData data)
    {
        return new TestWindowControl(data);
    }

    [Guid("30e76b2f-26bc-4256-bd83-e932825f694c")]
    public class Pane : ToolWindowPane { }
}

Note the Pane class contains the Guid attribute. It doesn't have to be a nested class, but I felt it was a good way to go since the class only needs to have the Guid attribute and nothing else.

You declare the tool window on the package like this:

[ProvideToolWindow(typeof(TestToolWindow.Pane))]

You initialize it like this:

public sealed class TestExtensionPackage : ToolkitPackage
{
    protected override async Task InitializeAsync(CancellationToken cancellationToken, IProgress<ServiceProgressData> progress)
    {
        TestToolWindow.Initialize(this);
    }
}

And you show it like this (the extension method that was added recently is no longer needed):

await TestToolWindow.ShowAsync();

[madskristensen]

I really like TestToolWindow.Initialize(this); and await TestToolWindow.ShowAsync();. That's phenomenal and very intuitive to use. The CreateContent and CreateDataAsync methods on the base class confuse me a bit though. How do I set other properties like BitmapImageMoniker on the ToolWindowPane instance like I used to be able to do from its constructor?

[yannduran]

I knew there was something that didn't feel right, but couldn't put my finger on it.

The base class is a great idea. The TestToolWindow.Initialize(this) and await TestToolWindow.ShowAsync() is great for consistency with commands.

But we've always had to have a separate pane class, and a separate control class. I don't think that concept should change. If we're wanting to teach best practices, then single responsibility comes into play here. The template should just create them when adding a ToolWindow.

Keeping those classes as they were eliminates the need for an empty pane class just to add a GUID to, as you just add the GUID like you always did, and you set the properties in the constructor just like you always did.

I don't understand this line:

VS.GetServiceAsync<MyData, MyData>();

Why is the data being passed into the constructor being called as a service?

I agree with Mads, I don't like the CreateDataAsync and CreateContent methods.

[reduckted]

@madskristensen How do I set other properties like BitmapImageMoniker on the ToolWindowPane instance like I used to be able to do from its constructor?

You can still do it from the constructor of the pane. Using the KnownMonikersExplorerWindow as an example:

public class KnownMonikersExplorerWindow : BaseToolWindow<KnownMonikersExplorerWindow, ServicesDTO>
{
    public const string WindowGuidString = "cfff3162-9c8d-4244-b0a7-e3b39a968b24";

    public string Title => "KnownMonikers Explorer";

    public Type PaneType Title => typeof(Pane);

    protected override async Task<MyData> CreateDataAsync(CancellationToken cancellationToken)
    {
        PropertyInfo[] properties = typeof(KnownMonikers).GetProperties(BindingFlags.Static | BindingFlags.Public);

        return new ServicesDTO
        {
            Monikers = properties.Select(p => new KnownMonikersViewModel(p.Name, (ImageMoniker)p.GetValue(null, null))),
            DTE = await VS.GetDTEAsync()
        };
    }

    protected override object CreateContent(ServicesDTO data)
    {
        return new KnownMonikersExplorerControl(data);
    }

    [Guid(WindowGuidString)]
    public class Pane : ToolWindowPane {
        public Pane(ServicesDTO state) : base()
        {
            BitmapImageMoniker = KnownMonikers.Image;
        }
    }
}

@yannduran Keeping those classes as they were eliminates the need for an empty pane class just to add a GUID to, as you just add the GUID like you always did, and you set the properties in the constructor just like you always did.

As I explained in an earlier comment, you can't make the base class and the pane class the same thing, because the pane class isn't created until the window actually needs to be created, and that is after the async services are loaded. If the base class contains the method that loads the async services, then you can't call it until you've created the tool window.

Even if there was some way we could do that, I'm very hesitant to directly create the ToolWindowPane, because Visual Studio seems to do some sort of tracking while it is being created. See Microsoft.VisualStudio.Shell.Package.InstantiateToolWindow(Type, object). I have no idea what it's doing, but it's clearly there for a reason, and by-passing that by just "newing" up a new instance of the tool window directly seems like a bad idea.

@yannduran Why is the data being passed into the constructor being called as a service?

It's just the example that I used in the code. I didn't want to use something like VS.GetDTEAsync() in case newcomers thought that they needed to pass a DTE object to the tool window.

Edit: @yannduran I just noticed that the example I posted didn't have the Pane class inheriting from ToolWindowPane. Perhaps that caused a bit of confusion.

[reduckted]

I agree with Mads, I don't like the CreateDataAsync and CreateContent methods.

Yeah, it's a bit ugly. How about this: https://github.com/madskristensen/Community.VisualStudio.Toolkit/compare/master...reduckted:5cf81277b5ab43e839676c8de7ad841e0a1f059a

(or just the changes I made: https://github.com/reduckted/Community.VisualStudio.Toolkit/commit/5cf81277b5ab43e839676c8de7ad841e0a1f059a)

I've merged the CreateDataAsync() and CreateContent() methods into a single CreateAsync() method that should return the content to show in the tool window pane. For example:

public override async Task<object> CreateAsync(CancellationToken cancellationToken)
{
    await Task.Yield();
    await Task.Delay(2000);
    await ThreadHelper.JoinableTaskFactory.SwitchToMainThreadAsync(cancellationToken);
    return new RunnerWindowControl(await VS.GetDTEAsync());
}

The tool window pane's Content property can accept any type of object, but I'm thinking it might be a good idea to require it to be a FrameworkElement, as that would guide people into returning the correct thing. So the signature would become:

public override async Task<FrameworkElement> CreateAsync(CancellationToken cancellationToken)

[madskristensen]

That looks absolutely fantastic. I think we need to keep the constructor in the Pane class and set a ImageMoniker so people can see that it's similar to a regular ToolWindowPane and the official documentation still works.

I love it!!! This partly brings async tool windows to VS 2017 and 2015, which is just amazing.

What do people think. Does this look good to you? And @reduckted, are you happy with it?

[bluetarpmedia]

This looks really cool! Couple of small thoughts:

• Is it worth adding a ShowAsync overload to handle multi-instance tool windows, or add the int id, bool create parameters with default values of 0 and true?
• Should ToolkitPackage.AddToolWindow lazily create the _toolWindowProviders list so that extensions without tool windows don't need to new up that list?

[yannduran]

• Is it worth adding a ShowAsync overload to handle multi-instance tool windows

Yes, absolutely!

• Should ToolkitPackage.AddToolWindow lazily create the _toolWindowProviders list so that extensions without tool windows don't need to new up that list?

Good point

[yannduran]

@reduckted It's just the example that I used in the code. I didn't want to use something like VS.GetDTEAsync() in case newcomers thought that they needed to pass a DTE object to the tool window.

I think you'll find that example will be just as confusing. I've been writing extensions for many years, including with async tool windows and it confused me.

I'm wasn't talking abut passing the data, I've used that myself, just calling it in a GetServiceAsync call didn't make sense.

[yannduran]

@reduckted It's just the example that I used in the code. I didn't want to use something like VS.GetDTEAsync() in case newcomers thought that they needed to pass a DTE object to the tool window.

I think you'll find that example will be just as confusing. I've been writing extensions for many years, including with async tool windows and it confused me.

I'm wasn't talking abut passing the data, I've used that myself, just calling it in a GetServiceAsync call didn't make sense.

[reduckted]

are you happy with it?

I am! I'll add the last few bits mentioned by @bluetarpmedia and then I'll create a PR for it.

[reduckted]

Just created PR #25 for this.