[Proposal] Make VSGlobals.vsct available via nuget as part of Community.VisualStudio.Toolkit

[CZEMacLeod]

I loved the new stuff for VSGlobals.vsct as part of #21 but thought rather than having the file as part of the template, make it part of the nuget package and therefore updatable when new names get added.

I did some quick experimentation and came up with some stuff that I think works at CZEMacLeod/Community.VisualStudio.Toolkit.VSGlobals.VSCT

Very basically, it adds a new Item type VSCTInclude to go with the existing VSCTCompile. It then ensures that any items of this type have their paths added to the include directories for the VSCTCompile task.

Then, if you have any VSCTCompile items in your project, it adds the VSGlobals.vsct file from the package as an VSCTInclude item.

I also added a quick check to give you a warning if you have VSCTInclude items that are not actually included in your VSCTCompile files.

e.g. if you missed out the

<Include href="VSGlobals.vsct"/>

I did a quick check with a modified version of the community template, and to make it easier to test, I also created a 'short form' or SDK style VSIX project. (It does work there too, and builds and debugs, although some of the editors such as the manifest editor refuse to work as it is the wrong project type.)

If you like the idea of this - feel free to take as much or as little as you like.

I think the project as it stands works without CVST or any extensions, and provides the VSCTInclude functionality, so it could either get contributed to Microsoft.VSSDK.BuildTools or be made a dependency of CVST and just the VSGlobals.vcst file moved here.

[madskristensen]

Super cool. A lot of moving parts here. Could it work like in the flow below?

• VSGlobals.vsct is added to toolkit NuGet package
• .targets file in toolkit NuGet package adds its own directory to VSCTCompile task (so compiler can find VSGlobals.vsct)
• When user adds <include href="VSGlobals.vsct"/> in their .vsct file, their project compiles correctly
• Including the globals should be optional without a warning if you opt out

[yannduran]

@madskristensen I dunno, this sounds way too complicated to me. It works in an elegant way as it is right now.

[reduckted]

I don't see what's complicated about this. You install the NuGet package and then you can add <include href="VSGlobals.vsct"/> to the .vsct file and use the aliased identifiers. Plus, if new IDs are added, then you just have to update the NuGet package instead of somehow getting an updated .vsct file out of the templates.

Isn't the whole idea of this project to simplify things for extension authors? By doing that, it requires moving the complexity from extensions into this project.

[CZEMacLeod]

@madskristensen My original version just added the VSGlobals.vsct path to the VSCTIncludePaths so it worked exactly as you describe.

I guess I maybe overengineered it a little as I thought the technique to ensure that any include file's path was added to the task was useful. It would mean you could put any includes in subfolders to organise your project and then include them without having to worry about paths. The item type could also be used by other nuget packages. As I said - perhaps the VSCTInclude item type should be an enhancement to the VSSDK build tools rather than the community toolkit.

There is an EnableVSGlobalsVSCT property which can be set to false to not automatically add the VSGlobals.vsct file. That way, if you don't want to include it you won't get a warning. It also skips adding it if there are no VSCTCompille items in your project.

I think that this will mostly do it for the .targets file.

  <PropertyGroup>
    <EnableVSGlobalsVSCT Condition="'$(EnableVSGlobalsVSCT)'==''">true</EnableVSGlobalsVSCT>
    <VSGlobalsVSCTDependentUpon Condition="'$(VSGlobalsVSCTDependentUpon)'==''">VSCommandTable.vsct</VSGlobalsVSCTDependentUpon>
  </PropertyGroup>

  <ItemGroup>
    <None Include="$(VSGlobalsVSCTPath)VSGlobals.vsct" Condition="'$(EnableVSGlobalsVSCT)'!='false' AND '@(VSCTCompile)'!=''">
      <Link>%(FileName)%(Extension)</Link>
      <Generator></Generator>
      <Visible>true</Visible>
      <InProject>true</InProject>
      <DependentUpon Condition="EXISTS('$(VSGlobalsVSCTDependentUpon)')">$(VSGlobalsVSCTDependentUpon)</DependentUpon>
    </None>
    <VSCTIncludePath Include="$(VSGlobalsVSCTPath)" Condition="'$(EnableVSGlobalsVSCT)'!='false'"/>
  </ItemGroup>

And you would need the path to VSGlobals.vsct in the .props file

  <PropertyGroup>
    <VSGlobalsVSCTPath Condition="'$VSGlobalsVSCTPath)'==''">$([System.IO.Path]::GetFullPath($([System.IO.Path]::Combine($(MSBuildThisFileDirectory),'..\contentFiles\any\any\'))))</VSGlobalsVSCTPath>
  </PropertyGroup>

This would mean you could override the path to the file to use a local customized/more up to date version by setting it in your project file or Directory.Build.props

[madskristensen]

A couple of comments/questions:

1. Should this be its own NuGet package?
   1. There could be breaking changes in the VSGlobals.vsct so you may want to update it separately from the toolkit
2. In its own NuGet package, only a .props, .targets and the VSGlobals.vsct files would be included to make this work, right?
3. I don't think it would be preferable to include the VSGlobals.vsct file in the consuming project. A complete silent light-up would be my preference (no artifacts in consuming project at all).
4. I don't think there's a need to override the path. If you have a newer version, you can rename it to VSGlobals2.vsct or put it in a different sub-folder in the project. Or am I missing the use case(s)?
5. I understand @yannduran's concern about adding complexity. I do feel that this will actually reduce complexity for most extenders and provide a path for updates that otherwise would be a very manual process.

[CZEMacLeod]

@madskristensen For reference, I split up the two features into separate packages. They work individually and together.

Community.VisualStudio.Toolkit.VSGlobals.VSCT is just the VSGlobals.vsct with EnableVSGlobalsVSCT and nesting logic.

Community.VisualStudio.SDK.BuildTools.VSCTInclude adds the VSCTInclude item type and does the VSCTIncludePaths and build warnings. It also migrates anything that is .vsct in the None group (such as the VSGlobals.vsct file) (behind a EnableDefaultVSCTIncludeItems flag) and glob adds any .vsct items in the local project that are not explicitly included as VSCTCompile or VSCTIncude items.

If you want just the VSGlobals stuff, which is a file that is maintained ???*, this might be a quick way of doing it. It also means that you don't need to remember to add and update each template where the file might go out of sync, and for end users, they can get the latest without downloading the updates templates, creating a new project and copying the file.

It could be included directly in this package, or in a separate package as part of this community effort.

In any case, if any wants it, I can publish the Community.VisualStudio.SDK.BuildTools.VSCTInclude package myself, or it could also be adopted into the Community toolkit; as a separate package - if people feel it is 'too much magic' to include in the base package. Although I feel that is exactly what the Community Toolkit should be doing; all the magic to make the entrance bar as low as possible and things 'just work'.

For now, both packages are just binary blobs in the repo. Once a decision is made, I'll either push them to github, nuget, both, or neither, depending on demand.

*I'm not sure what the 'source of truth' for that file is... I'm guessing it is the multiple copies in the ExtensibilityTemplatePack Project Templates? N.B. My file might be out of date by now 😉

[CZEMacLeod]

@madskristensen

1. Should this be its own NuGet package?

If so - it is almost done as Community.VisualStudio.Toolkit.VSGlobals.VSCT

2. In its own NuGet package, only a .props, .targets and the VSGlobals.vsct files would be included to make this work, right?

Pretty much - see the code at CZEMacLeod/Community.VisualStudio.Toolkit.VSGlobals.VSCT/Community.VisualStudio.Toolkit.VSGlobals.VSCT

3. I don't think it would be preferable to include the VSGlobals.vsct file in the consuming project. A complete silent light-up would be my preference (no artifacts in consuming project at all).

Okay - I was assuming someone might want to be able to open and browse the file from the project. It does auto-nest the same as the templates do currently. I guess if this is the decision it is pretty simple to remove the feature, or put it behind a flag that defaults to false so people can enable and see it if they want.

4. I don't think there's a need to override the path. If you have a newer version, you can rename it to VSGlobals2.vsct or put it in a different sub-folder in the project. Or am I missing the use case(s)?

It is pretty much inherent to the way most nuget targets files are written that they reference their files relative to a base path that is set in the props file. This means that you can override the path in your project and the targets logic still executes, but uses your own files.

I guess the options if you need to use a custom/newer version are

1. Remove the package (if it is separate).
2. Use a different filename for a local file (e.g. VSGlobals2.vsct) i) In this case, the include path is still added for the package's VSGlobals.vsct - for now that is probably not a concern, but it could lead to something happening at build-time where VSCTCompile looks in that path and finds something that conflicts.
3. Set the VSGlobalsVSCTPath to point to your local project folder (or a subfolder). i) This would mean that the package path would not be included and possibly confuse the compiler. ii) If another nuget package, or shared vsct needed the VSGlobals.vsct, it would use the updated/customized version. If another nuget package was installed that provided common vsct templates or something, that referenced this package, or it wasn't separate, you couldn't uninstall it (option 1).

In any case, whether there is a need for it, or not, it does no harm to have the option to override it, in case someone does need it for something, and it doesn't really cost anything to have it.

5. I understand @yannduran's concern about adding complexity. I do feel that this will actually reduce complexity for most extenders and provide a path for updates that otherwise would be a very manual process.

That was my feeling, along with ensuring that there was a single 'source of truth' for that file. It would make it very easy to know which version of the file you had, and be able to find any changes in github, and match that with the package version you are using. It would also make updating the file a simple case of updating the package. At the moment, I would have to know which version of the ExtensibilityTemplatePack you had installed when you created the project, (and possibly even which template was originally used) to be able to find the version you are using, and then to find any changes or updates to it.

[CZEMacLeod]

@madskristensen I don't know if it applies - but I guess we could do some trickery to light up packages for 14.x, 15.x, and 16.x like the toolkit does and only include IDs that exist in those versions?

[madskristensen]

I don't know if it applies - but I guess we could do some trickery to light up packages for 14.x, 15.x, and 16.x like the toolkit does and only include IDs that exist in those versions?

I thought about that, but usually all guid/id pairs are additive between the versions. It's super rare that any go away. If so, it's because that menu group no longer exist, and the button simply wouldn't be displayed - no compiler or runtime errors would occur. Considering that, I think we should just keep a single .vsct file for now.

[madskristensen]

Regarding multiple NuGet packages and complexity, I agree with you on most aspects. I think just a single NuGet package is needed with all the features, if it adheres to:

1. No artifacts showing in the consuming project
2. No other entries in the consuming .csproj other than the <PackageReference />
3. The <include href="VSGlobals.vsct" /> element is the only thing needed to light it up aside from referencing the NuGet package

If these 3 statements can be true, then I think it's totally awesome having extra MSBuild capabilities and features that some people might take advantage of on top of that if they wish.

Let's go for it!!

This also now marks the time where a new GitHub organization is needed to maintain and organize these efforts around. I've created https://github.com/VsixCommunity where I'll move the toolkit project to shortly. Let's host the VSGlobals NuGet package there too

[madskristensen]

Also, should we call the single package something along the lines of Community.VisualStudio.VSCT. That way it can be about anything VSCT if a needed for more features arise in the future?

[CZEMacLeod]

I don't know if it applies - but I guess we could do some trickery to light up packages for 14.x, 15.x, and 16.x like the toolkit does and only include IDs that exist in those versions?

I thought about that, but usually all guid/id pairs are additive between the versions. It's super rare that any go away. If so, it's because that menu group no longer exist, and the button simply wouldn't be displayed - no compiler or runtime errors would occur. Considering that, I think we should just keep a single .vsct file for now.

If the changes are generally cumulative, then perhaps a VSGlobals.vsct which is basically empty except for an include of VSGlobals-V16.vsct (for now) which would include VSGlobals-V15.vsct, and in turn VSGlobals-V14.vsct. That way if you were developing for e.g. V15+ you could use

<Include href="VSGlobals-V15.vsct"/>

instead and get a compile error if you try and use a constant added in a later version?

It could then be a single version release, but still provide the validation if you opt-in.

Maybe it is too much work for now, but perhaps something to think about and put on the back burner for later.

[madskristensen]

That's a great idea. But let's start with a single version because validating all the guid/id pairs in that is going to take me several weeks alone. Let's cross the bridge when the need arises and not preemptively.

[madskristensen]

I'm archiving this repo so we can continue under the new org at https://github.com/VsixCommunity/Community.VisualStudio.Toolkit