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
Migrate to the editor 15.6.241 #23943
Changes from 9 commits
47961e9
4b5ecf8
6abe6f0
e132432
e7217a8
79618d1
f2fb29d
3429d55
f0c7a71
7ce4766
0fb5608
12eafc4
49f86cc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -65,7 +65,6 @@ | |
but includes the lib folder so it's copied as a dependency of Microsoft.VisualStudio.Platform.VSEditor. Unfortunately we still have test code that | ||
does depend on internal APIs, so we need to a force a reference here. --> | ||
<Reference Include="$(NuGetPackageRoot)\microsoft.visualstudio.text.internal\$(MicrosoftVisualStudioTextInternalVersion)\lib\net46\Microsoft.VisualStudio.Text.Internal.dll" /> | ||
<PackageReference Include="Microsoft.Tpl.Dataflow" Version="$(MicrosoftTplDataflowVersion)" /> | ||
<PackageReference Include="Microsoft.VisualStudio.Composition" Version="$(MicrosoftVisualStudioCompositionVersion)" /> | ||
<PackageReference Include="Microsoft.VisualStudio.Platform.VSEditor" Version="$(MicrosoftVisualStudioPlatformVSEditorVersion)" /> | ||
<PackageReference Include="BasicUndo" Version="$(BasicUndoVersion)" /> | ||
|
@@ -80,6 +79,7 @@ | |
<PackageReference Include="Microsoft.VisualStudio.Text.UI" Version="$(MicrosoftVisualStudioTextUIVersion)" /> | ||
<PackageReference Include="Microsoft.VisualStudio.Text.UI.Wpf" Version="$(MicrosoftVisualStudioTextUIWpfVersion)" /> | ||
<PackageReference Include="Microsoft.VisualStudio.Validation" Version="$(MicrosoftVisualStudioValidationVersion)" /> | ||
<PackageReference Include="System.Threading.Tasks.Dataflow" Version="$(SystemThreadingTasksDataflowVersion)" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: if this is implied by Microsoft.VisualStudio.Composition and everything works by deleting this line, I'm OK with not stating it. Our projects today over-specify a lot of package versions which makes things get annoying sometimes. |
||
</ItemGroup> | ||
<ItemGroup> | ||
<InternalsVisibleToTest Include="Roslyn.VisualStudio.Services.UnitTests" /> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,7 +40,8 @@ public static Type[] GetLanguageNeutralTypes() | |
typeof(DefaultSymbolMappingService), | ||
typeof(TestWaitIndicator), | ||
typeof(TestExtensionErrorHandler), | ||
typeof(TestExportJoinableTaskContext) // Needed by editor components, but not actually exported anywhere else | ||
typeof(TestExportJoinableTaskContext), // Needed by editor components, but not actually exported anywhere else | ||
typeof(TestObscuringTipManager) // Needed by editor components, but only exported in editor VS layer. Tracked by https://devdiv.visualstudio.com/DevDiv/_workitems?id=544569. | ||
}; | ||
|
||
return types//.Concat(TestHelpers.GetAllTypesWithStaticFieldsImplementingType(typeof(InternalSolutionCrawlerOptions).Assembly, typeof(Microsoft.CodeAnalysis.Options.IOption))) | ||
|
@@ -124,7 +125,7 @@ public static PartDiscovery CreatePartDiscovery(Resolver resolver) | |
|
||
public static ExportProvider CreateExportProvider(ComposableCatalog catalog) | ||
{ | ||
var configuration = CompositionConfiguration.Create(catalog.WithDesktopSupport().WithCompositionService()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. WithDesktopSupport() is deprecated, see https://github.com/Microsoft/vs-mef/blob/v15.5/src/Microsoft.VisualStudio.Composition/Configuration/NetFxAdapters.cs#L59 |
||
var configuration = CompositionConfiguration.Create(catalog.WithCompositionService()); | ||
var runtimeComposition = RuntimeComposition.CreateRuntimeComposition(configuration); | ||
return runtimeComposition.CreateExportProviderFactory().CreateExportProvider(); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,19 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.ComponentModel.Composition; | ||
using System.Linq; | ||
using System.Text; | ||
using System.Threading; | ||
using System.Threading.Tasks; | ||
using Microsoft.CodeAnalysis.Editor.Shared.Utilities; | ||
using Microsoft.VisualStudio.Threading; | ||
|
||
namespace Microsoft.CodeAnalysis.Editor.UnitTests | ||
{ | ||
// In 15.3 the editor took a dependency on JoinableTaskContext. | ||
// JTC appears in the VS MEF composition but does not itself | ||
// contain an ExportAttribute. The Editor's own unit tests | ||
// export it from a field and we need to do the same in order | ||
// to be able to compose in any part of the Editor. | ||
// Starting with 15.3 the editor took a dependency on JoinableTaskContext | ||
// in Text.Logic and Intellisense layers as an editor host provided service. | ||
[PartCreationPolicy(CreationPolicy.NonShared)] // JTC is "main thread" affinitized so should not be shared | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The changes to this file seem somewhat fishy to me:
Maybe we have to just chat in person on this one, but what's going on here? This seems very magic and there's a lot of context missing. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let me look at it again... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it does look fishy, I wish I knew a better way.
This needs further investigation, I just don't have enough context in the text infrastructure. Maybe it would be better not to export JTC here, but explicitly add new instance to each composition? |
||
internal class TestExportJoinableTaskContext : ForegroundThreadAffinitizedObject | ||
{ | ||
[ThreadStatic] | ||
private static JoinableTaskContext s_joinableTaskContext; | ||
|
||
[Export] | ||
private JoinableTaskContext _joinableTaskContext = new JoinableTaskContext( | ||
mainThread: ForegroundThreadAffinitizedObject.CurrentForegroundThreadData.Thread); | ||
private JoinableTaskContext _joinableTaskContext = s_joinableTaskContext ?? (s_joinableTaskContext = new JoinableTaskContext()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
using System.ComponentModel.Composition; | ||
using Microsoft.VisualStudio.Text.Editor; | ||
|
||
namespace Microsoft.CodeAnalysis.Editor.UnitTests | ||
{ | ||
// In 15.6 the editor (QuickInfo in particular) took a dependency on | ||
// IObscuringTipManager, which is only exported in VS editor layer. | ||
// This is tracked by the editor bug https://devdiv.visualstudio.com/DevDiv/_workitems?id=544569. | ||
// Meantime a workaround is to export dummy IObscuringTipManager. | ||
[Export(typeof(IObscuringTipManager))] | ||
internal class TestObscuringTipManager : IObscuringTipManager | ||
{ | ||
public void PushTip(ITextView view, IObscuringTip tip) | ||
{ | ||
} | ||
|
||
public void RemoveTip(ITextView view, IObscuringTip tip) | ||
{ | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These bits are already in d15.6stg (actually it has 15.6.245-preview already), see https://devdiv.visualstudio.com/DevDiv/Editor/_git/VS?path=%2F.corext%2FConfigs%2Fdefault.config&version=GBlab%2Fd15.6stg&line=301&lineStyle=plain&lineEnd=302&lineStartColumn=1&lineEndColumn=1