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

VSIX rework#66

Merged
SergeyTeplyakov merged 24 commits intomicrosoft:masterfrom
sharwell:vsix-rework
Jul 7, 2015
Merged

VSIX rework#66
SergeyTeplyakov merged 24 commits intomicrosoft:masterfrom
sharwell:vsix-rework

Conversation

@sharwell
Copy link
Contributor

Summary of changes

  • Refactor the Visual Studio extension so a single extension works in Visual Studio 2010+
  • Fully support Visual Studio 2010-2015 language services
  • Visual Studio 2015 support is a placeholder - the assembly exists but the service factory throws NotSupportedException (this was completed)

Fixes #4

Work left (should now be complete)

  • This is a preview branch which does not include the changes in Added .gitattributes #61. Added .gitattributes #61 needs to be merged first, and then I will update this pull request (I have a branch in my fork that's already prepared to do this efficiently).
  • Some files are missing copyright headers
  • I forgot to add [assembly: ProvideBindingPath] (just remembered while writing this)
  • Update the build script. buildMSI10.xml and/or Setup10.proj will need major work

sharwell added 5 commits June 17, 2015 20:08
This commit removes the separated Visual Studio 2010 and 2012 projects,
leaving a single project file in the same directory as the source code.
This structure eliminates the need to use linked files in all the projects,
vastly simplifying long-term maintenance.
* It was already enabled in Visual Studio 2010
* Support for Visual Studio 2015 requires substantial work for Roslyn
@sharwell
Copy link
Contributor Author

@SergeyTeplyakov The copyright headers question is for you and/or Mike and/or similar.

@hubuk Do you want to see this merged before the setup scripts are updated for it, or afterwards?

Aside from the [ProvideBindingPath] issue which is straightforward for me to correct, I don't have any immediate plans to change this pull request until I get more feedback about the direction. 😄

@sharwell
Copy link
Contributor Author

💭 The [ProvideBindingPath] attribute likely eliminates much (or all?) of the need for using ILMerge on the contents of the VSIX.

@mike-barnett
Copy link
Contributor

I'm fine with using that copyright header.

@sharwell
Copy link
Contributor Author

@hubuk This is now complete (i.e. ready for review) aside from the required changes to the installer. How would you like to proceed?

@hubuk
Copy link
Contributor

hubuk commented Jun 22, 2015

@sharwell I will be reviewing all your changes, but it may take a wile. I will comment any questions here and mark a changsets as reviewed by submitting +1 on each individually. Do you think that this will help to organize review better?
And regarding a merge. I think that your PR may goes first. I will prepare installer project version changes by tomorrow (offline). Will be ready to submit a PR after your code is merged.

@billings7
Copy link
Contributor

@hubuk re: the installer changes, would those have any effect on or benefit from PR #36 at all?

@hubuk
Copy link
Contributor

hubuk commented Jun 22, 2015

@billings7 No, my changes contain a new version update mechanism in buildMSI10.xml only.
Regarding #36:
The change you introduced worked fine. Unfortunately now it is marked as conflicting and I think it requires some more changes. In addition it would be better not to commit CodeTools binaries into git but build them and use compiled image instead. If you can take a look into this it would be great. If no, I can recreate your changes and align them in a new PR when I finish my current work.

@sharwell
Copy link
Contributor Author

@hubuk @billings7 Pull requests created prior to #61 will show merge conflicts in the GitHub UI, but in many cases will not actually have merge conflicts when the merge is run from the command line (and/or the merge conflicts are automatically resolved).

@billings7
Copy link
Contributor

@hubuk I will have a look into that when I can. Changing how CodeTools is built would probably be better in a different PR, from what I can remember some part of it needs VS2012 to build it.

sharwell added 3 commits June 28, 2015 15:28
This assembly will require substantial modifications to support Visual Studio
2015. Rather than interleave both versions in the files using preprocessor
directives, a copy of the code was made so it can be modified without
affecting the original.
@sharwell
Copy link
Contributor Author

I added 3 commits which are the first part of editor support for VS 2015. Locally I only have 2 more methods to implement (GetAnyCallNodeAboveTriggerPoint and GetTargetAtTriggerPoint) before I can test the editor extensions within Visual Studio 2015. I didn't include all of those changes yet because it would break this pull request in the interim - as is, we can still merge this pull request whenever the review of the pre-2015 support is complete.

@SergeyTeplyakov
Copy link
Contributor

Does it means that the integration with VS2015 itself is working fine already (except the async part)?

@sharwell
Copy link
Contributor Author

This pull request only deals with the editor extensions. The other compatibility issues for VS 2015 are actually not due to the IDE itself, but rather the new build tools (MSBuild 14.0 and Roslyn-related differences in the IL).

@SergeyTeplyakov
Copy link
Contributor

Does it mean that #36 is all that we need for VS2015 support?
I already fixed issues with differentce in the IL in #74 (at least, everything that I know about this issue). If you have anything related to roslyn-based behavior, could you shared it with me?

sharwell added 10 commits July 4, 2015 13:51
The Roslyn support requires an ITextBuffer to implement obtain a workspace,
so the signature of ICompiler.GetCompilation was updated to match.
Not everything is completed at this point, but the assembly does compile
against the new Roslyn APIs.
During the build, the extension is now deployed to each supported
environment. By default, launching ContractAdornments for debugging will
launch the same version of Visual Studio currently being used. However,
this may be changed by updating the debug options to point at a different
devenv.exe instance.
Previously several parts of the Roslyn-based editor extensions code used
using alias directives so Roslyn types could be referenced using names found
in the old Visual Studio IntelliSense interop assemblies. This simplified
some of the previous diffs, but now that the functionality is working this
step can be removed.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

📝 Someone will need to figure out what this was doing, but it didn't seem to be essential for the initial pull request. I am in favor of creating a new issue to resolve this warning (by implementing the following excluded code block) after this pull request is merged.

SergeyTeplyakov added a commit that referenced this pull request Jul 7, 2015
@SergeyTeplyakov SergeyTeplyakov merged commit 60cbff1 into microsoft:master Jul 7, 2015
@sharwell sharwell deleted the vsix-rework branch July 7, 2015 16:53
@SergeyTeplyakov SergeyTeplyakov modified the milestones: VS14 Support, CCEditorExtensionVS2015 Jul 9, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants