Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Conversation

@jcansdale
Copy link
Collaborator

@jcansdale jcansdale commented May 8, 2018

Since custom VS services are disabled in Blend, we need to disable any of our MEF services that are derived from VS services.

What this PR does

  • Fix ExportForProcessAttribute so that when not executing in a specific named process the export ContractName is set to GitHub.Disabled
  • Add ExportForVisualStudioProcessAttribute to conditionally export when executing in Visual Studio (devenv)
  • Add IsVisualStudioProcess method to check when exports are available
  • Disable UIContext_Git when not running in Visual Studio process
  • Disable GitHub/InlineReviews commands when not running in Visual Studio process
  • Disable GitHub options when when not running in Visual Studio process (previously it crashed)

Previously it was setting the ContractType to null when not executing in the named process. This made it use the default ContractType instead of the explicit contract interface. This happened to work when [ExportForProcess(typeof(IFoo), "devenv")] was on class Foo, because the default ContractType was Foo not IFoo. This didn't work when [ExportForProcess(typeof(IFoo), "devenv")] was on a property of type IFoo, because the default ContractType was exactly the same as the explicit contract interface.

This this PR makes sets the ContractName to GitHub.Disabled not executing in the named process. Since we import using using [Import(typeof(IFoo))] rather than [Import("GitHub.Disabled", typeof(IFoo))], the service is effectively disabled. The disabled contract is given the unique name GitHub.Disabled to avoid unlikely possibility of a collision.

The following will still appear in the Blend UI:
image

image

When either of these buttons is clicked on, the following dialog will appear:
image
(not this has been changed from Expression Blend to just Blend)

The regression

This fixes a regression that appeared when I changed to using property based exports, e.g:

public IGitHubServiceProvider GitHubServiceProvider => GetService<IGitHubServiceProvider>();

Previously only the following would have worked as intended.

[ExportForProcess(typeof(IFoo), "devenv")]
public class Foo {

Now the following will also work correctly:

[ExportForProcess("devenv")]
public IFoo Foo { get; }

What this PR doesn't do

There are some remnants that will still appear that are difficult to get rid of. The main thing is they won't crash Blend when clicked on!

image

Reviewers

Previously clicking on any of the remnants above would crash Blend.

@meaghanlewis this might be worth putting into the test plan...

Check that selecting any of the following doesn't crash Blend:
Other Widows > GitHub
Manage connections > Connect to GitHub
Options > GitHub for Visual Studio

Fixes #1620

jcansdale added 2 commits May 8, 2018 17:24
When current process is different to named process, set the MEF
export's contract name to "GitHub.Disabled". Since this contract name
isn't imported, the service is effectively disabled.
There's no need to specify a contract interface when exporting a
property.
jcansdale added 5 commits May 9, 2018 00:38
We should only load auto-load packages into the VS process (not Blend).
This attribute is used for conditionally export services when executing
in the Visual Studio process.
The `IsVisualStudioProcess` method can be used to check the
availability of services exported in this way.
Commands should be disabled to avoid crashing Blend.
This stops the options pane from crashing Blend.
I'm not sure how to hide the options completely in Blend.
@jcansdale jcansdale changed the title [wip] Disable MEF services that are derived from VS services when executing in Blend Disable MEF services that are derived from VS services when executing in Blend May 9, 2018
@jcansdale jcansdale requested review from grokys and meaghanlewis May 9, 2018 15:23
@jcansdale
Copy link
Collaborator Author

jcansdale commented May 10, 2018

For reference, here's what the MEF looks like when the extension is loaded in Blend:
https://gist.github.com/jcansdale/778e464a898c69e1626edfc16ca45b9d

@meaghanlewis
Copy link
Contributor

Tested this using Blend for VS 2015 and 2017. Can confirm that Blend no longer crashes or displays errors when trying to use/view GitHub functionality. Thanks @jcansdale

One thing I find interesting is that even when the GitHub extension is disabled I still see these options in Blend:

  • Other Widows > GitHub
  • Manage connections > Connect to GitHub
  • Options > GitHub for Visual Studio

@jcansdale
Copy link
Collaborator Author

@meaghanlewis,

One thing I find interesting is that even when the GitHub extension is disabled I still see these options in Blend

Yes, this is unfortunately. Unfortunately there isn't an easy way to fix without degrading startup performance when the extension is running in Visual Studio. Here are some notes:

  • Other Widows > GitHub

This button is default visible and loads the GitHubPackage when clicked.

  • Manage connections > Connect to GitHub

I believe this command it also default visible and loads the GitHubPackage when clicked.

  • Options > GitHub for Visual Studio

Don't think there's any support for dynamically enabling/disabling options.

We could maybe show a warning dialog when the user clicks on one of those buttons saying the extension is only supported in VS?

@meaghanlewis
Copy link
Contributor

This LGTM. Thanks so much for adding the dialog that shows after selecting any GitHub functionality. ✨
It will make it clearer to users that we don't support Blend.

screen shot 2018-05-16 at 8 58 49 am

@jcansdale
Copy link
Collaborator Author

@meaghanlewis,

It will make it clearer to users that we don't support Blend.

Yes, I think it will. The user who originally reported the issue also commented on this: #1620 (comment)

@jcansdale jcansdale merged commit 298b3d6 into master May 17, 2018
@jcansdale jcansdale deleted the fixes/1620-disable-in-blend branch May 17, 2018 09:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

After installing extension in VS, Blend hits NullReferenceException when loading solution

4 participants