Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add property page for encouragements #27

Merged
merged 9 commits into from Jul 29, 2014

Conversation

haacked
Copy link
Owner

@haacked haacked commented Jul 24, 2014

This adds the simplest possible property page for customizing encouragements.

The only problem so far is that you can't hit Enter in the text box to add another encouragement. So if you need to add multiple, you have to cut and paste.

The "OK" button seems to steal the Enter keypress. I tried to handle it in the KeyDown event, but I think the OK button receives it first.

@jaredpar any thoughts on what we can do here?

@jaredpar
Copy link
Collaborator

I spent some time looking at this. I'll start with saying that low level UI, especially older Win32 UI, is not my specialty so take what I say with a grain of skepticism. ;)

The VS options dialog under the hood is a property sheet. All of the message processing for the dialog appears to obey these basic semantics.

In this case the enter key appears to go through normal processing. It sees WM_KEYDOWN + VK_RETURN and eventually hands it off to IsDialogMessageW. That in turn will send WM_SYSCOMMAND + SC_CLOSE to the dialog and results in the close.

There appears to be no way at the Visual Studio level to opt out of this behavior. There may be a way to do this at a lower level (like overriding WndProc) but I'm not strong enough in that area to say whether or not it's possible. I sent an email out to the VS team to see if they have any ideas here.

My guess is they will say that the best approach is to use a list box style control with Add / Remove buttons. Essentially taking the Enter key out of the equation.

@haacked
Copy link
Owner Author

haacked commented Jul 24, 2014

sigh That's what I figured. I guess I could try to override WndProc, but I think it'll be easier to do some kind of list box control.

@jaredpar
Copy link
Collaborator

The VS team got back to me and it is possible to handle Enter in the options page but it does involve a bit of WndProc magic. Basically handling WM_GETDLGCODE and returning DLGC_WANTALLKEYS when the control wants to handle Enter

The WPF version of DialogPage is UIElementDialogPage and it actually already does this automatically. The vague description about the process is here

http://msdn.microsoft.com/en-us/library/vstudio/microsoft.visualstudio.shell.uielementdialogpage.dialogkeypendingevent(v=vs.110).aspx

@haacked
Copy link
Owner Author

haacked commented Jul 25, 2014

@jaredpar thanks man! UIElementDialogPage was exactly what I needed. Care to review this PR when you have time? I think I'll blog it.

return Settings.Default.Encouragements.Split(
new [] {Environment.NewLine}, StringSplitOptions.RemoveEmptyEntries)
.ToList();
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The use of application settings is not version safe in a VSIX. The location of the stored setting file path in part includes the version string and hashes of the executable. When Visual Studio installs an official update these values change and as a consequence change the setting file path. Visual Studio itself doesn't support the use of application settings hence it makes no attempt to migrate this file to the new location and all information is essentially lost.

The supported method of settings is the WritableSettingsStore. It's very similar to application settings and easy enough to access via SVsServiceProvider

    public static WritableSettingsStore GetWritableSettingsStore(this SVsServiceProvider vsServiceProvider)
    {
        var shellSettingsManager = new ShellSettingsManager(vsServiceProvider);
        return shellSettingsManager.GetWritableSettingsStore(SettingsScope.UserSettings);
    }

This may or may not be considered a blocker for you. IIRC the settings just reset on every upgrade so it wouldn't necessarily lose all information, just new information.

@haacked
Copy link
Owner Author

haacked commented Jul 29, 2014

@jaredpar Thanks to your suggestions, I changed my approach to settings. Since OptionsDialogPage is a DialogPage I can add a public property to it and VS seems to handle the saving and restoring of that property value when I change the Encourage options just fine. Hooray!

The problem is I'm having trouble getting that value to the other services.

I made this class also export IEncouragements and implement that interface. But that doesn't really work. It seems I have to actually load the dialog properly first for the settings to be hydrated.

I tried to do that in EncouragePackage.Initialize but it's never called. Any ideas why that's the case?

@jaredpar
Copy link
Collaborator

Mixing options pages and MEF components is problematic because they have different activation models: COM vs. MEF. Even though it's logically one service eventually two instances of OptionsDialogPage will end up floating around the system and they will have different values for encouragements.

Generally the best approach here is to keep the MEF service separate and then import it into the DialogPage when needed. The Site property is a service provider and it can be traversed to get back any MEF exported service on demand via this snippet

var componentModel = (IComponentModel)(Site.GetService(typeof(SComponentModel)));
return componentModel.DefaultExportProvider.GetExportedValue<IEncouragements>();

I was writing up a much longer comment here about how to get the interplay between the two services correct and eventually realized it may be better to just demonstrate it with code.

d2f33de

As for the persistence of the property on the dialog it seemed to only be persisting it for the current Visual Studio session on my box vs across restarts of Visual Studio. I think that's just an effect of how DialogPages are created. The Package base type will generally only create a DialogPage once per session and just reuse that value over and over again (Package::GetDialogPage). I added a quick demo of how to use WritableSettingsStore to use persistence across restarts of Visual Studio (with some questionable error handling practices).

@haacked
Copy link
Owner Author

haacked commented Jul 29, 2014

Weird, that doesn't build for me. :(

1>------ Rebuild All started: Project: EncouragePackage, Configuration: Debug Any CPU ------
1>C:\Program Files (x86)\MSBuild\12.0\bin\Microsoft.Common.CurrentVersion.targets(1696,5): warning MSB3245: Could not resolve this reference. Could not locate the assembly "Microsoft.VisualStudio.ComponentModelHost, Version=11.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL". Check to make sure the assembly exists on disk. If this reference is required by your code, you may get compilation errors.
1>C:\dev\Encourage\EncouragePackage\Options\OptionsDialogPage.cs(10,30,10,48): error CS0234: The type or namespace name 'ComponentModelHost' does not exist in the namespace 'Microsoft.VisualStudio' (are you missing an assembly reference?)
========== Rebuild All: 0 succeeded, 1 failed, 0 skipped ==========

@haacked
Copy link
Owner Author

haacked commented Jul 29, 2014

Ah, I changed the reference to

<Reference Include="Microsoft.VisualStudio.ComponentModelHost, Version=12.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a, processorArchitecture=MSIL" />

and that seemed to fix it.

@jaredpar
Copy link
Collaborator

Blarg! Insert general grumblings about Visual Studio SDK not embracing NuGet as a deployment solution.

I just updated that branch to add the appropriate reference assembly. Need to take that vs. changing the reference tag because changing the reference tag will break the ability to deploy to VS 2012. The version "12.0.0.0" tag actually targets VS 2013 and not VS 2012.

@haacked
Copy link
Owner Author

haacked commented Jul 29, 2014

Ah, right. Shit.

@haacked
Copy link
Owner Author

haacked commented Jul 29, 2014

🍔

I think this PR is ready! Thanks @jaredpar

@jaredpar
Copy link
Collaborator

👍

Looks good to me!

@haacked
Copy link
Owner Author

haacked commented Jul 29, 2014

✨ Thanks!

BTW, I've got a rough draft of a blog post that explains this process. I quote you a bit. Mind taking a look when you have some time? haacked/haacked.com#140

haacked added a commit that referenced this pull request Jul 29, 2014
@haacked haacked merged commit b883804 into master Jul 29, 2014
@haacked haacked deleted the haacked/24-customize-encouragements branch July 29, 2014 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants