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

Move SymbolSearch down to EditorFeatures #45505

Merged
merged 2 commits into from
Jun 29, 2020
Merged

Move SymbolSearch down to EditorFeatures #45505

merged 2 commits into from
Jun 29, 2020

Conversation

tmat
Copy link
Member

@tmat tmat commented Jun 28, 2020

... from EditorFeatures.Wpf.

@tmat tmat marked this pull request as ready for review June 29, 2020 01:45
@tmat tmat requested review from a team as code owners June 29, 2020 01:45
@tmat
Copy link
Member Author

tmat commented Jun 29, 2020

@CyrusNajmabadi PTAL

@CyrusNajmabadi
Copy link
Member

I'm totally fine with this. However, i'm curious what's the overall motivation here. Could you expand on that a bit? Thanks!

@tmat
Copy link
Member Author

tmat commented Jun 29, 2020

The motivation is to remove dependency on WPF layer from ServiceHub project:
https://github.com/dotnet/roslyn/pull/45505/files#diff-526e287dd825598a68895e89a4d13489L19

Features that do not depend on WPF shouldn't be in WPF layer.

@tmat tmat merged commit f755e8a into dotnet:master Jun 29, 2020
@ghost ghost added this to the Next milestone Jun 29, 2020
@tmat tmat deleted the Layering2 branch June 29, 2020 03:18
@CyrusNajmabadi
Copy link
Member

WFM! Thanks!

333fred added a commit to 333fred/roslyn that referenced this pull request Jun 29, 2020
* upstream/master: (1226 commits)
  Remove unnecessary Clone() (dotnet#45469)
  Align addition of a synthesized override of object.Equals(object? obj) in records with the latest design. (dotnet#45475)
  Move SymbolSearch down to EditorFeatures (dotnet#45505)
  VisitType in MethodToClassRewriter for function pointers.
  Fix up nondeterminism in serializing naming style preferences
  Update dependencies from https://github.com/dotnet/arcade build 20200626.2 (dotnet#45482)
  Fix typo
  Move to vnext
  Add constant inerpolated strings to the test plan, update status for records.
  Don't emit ldftn when the result is unused.
  PR Feedback: * Additional tests for nested function contexts. * Override VisitFunctionPointerLoad in MethodToClassRewriter. * Adjust debug asserts.
  Add records to compiler test plan (dotnet#45434)
  Expand comment in CreateRecoverableText
  Replace binary serialization of encoding with a custom serializer. (dotnet#45374)
  LangVersion 9 (dotnet#44911)
  Avoid loading document text in AbstractObjectBrowserLibraryManager.DocumentChangedAsync
  Allow TryGetTextVersion to pass through to the initial source
  Ensure recoverable text is in temporary storage
  Fix test
  Updates the option page type GUID to match the one in pkgdef
  ...
@jasonmalinowski
Copy link
Member

So this is ironcially the identical change that @sharwell was making in #45115, which we were not going to take because this was potentially going to cause other issues. RemoteControl isn't a library that is cross-plat -- it depends on things like "the Internet Explorer cache" and the registry, and so it won't run on VS for Mac. In @sharwell's PR we were looking at alternative solutions.

@tmat is this fixing an urgent issue? Otherwise we may need to roll this back.

@tmat
Copy link
Member Author

tmat commented Jun 29, 2020

@jasonmalinowski Let's roll back.

jasonmalinowski added a commit to jasonmalinowski/roslyn that referenced this pull request Jun 29, 2020
This reverts commit f755e8a. That
change meant our EditorFeatures code was using DllImports in the
cross-platform binary, and we weren't sure what that would do in
VS for Mac if that code ran. It also brought a dependency on
Microsoft.VisualStudio.RemoteControl, which also seems to depend on
various Windows specific APIs.

We'll finish up the decision of what to do in
dotnet#45115
ghost pushed a commit that referenced this pull request Jun 29, 2020
…earch-to-editorfeatures

Revert "Move SymbolSearch down to EditorFeatures (#45505)"
333fred added a commit that referenced this pull request Jun 30, 2020
…e_168

* upstream/master: (102 commits)
  Change contrast ratio to get close to 1.5:1 (#45526)
  Revert "Move SymbolSearch down to EditorFeatures (#45505)"
  Delay accessibility checks to avoid cycles (#45441)
  Prevent trying to convert metadata references into circular project references
  Remove unnecessary Clone() (#45469)
  Align addition of a synthesized override of object.Equals(object? obj) in records with the latest design. (#45475)
  Move SymbolSearch down to EditorFeatures (#45505)
  VisitType in MethodToClassRewriter for function pointers.
  Fix up nondeterminism in serializing naming style preferences
  Update dependencies from https://github.com/dotnet/arcade build 20200626.2 (#45482)
  Fix typo
  Move to vnext
  Add constant inerpolated strings to the test plan, update status for records.
  Don't emit ldftn when the result is unused.
  PR Feedback: * Additional tests for nested function contexts. * Override VisitFunctionPointerLoad in MethodToClassRewriter. * Adjust debug asserts.
  Add records to compiler test plan (#45434)
  Expand comment in CreateRecoverableText
  Replace binary serialization of encoding with a custom serializer. (#45374)
  LangVersion 9 (#44911)
  Avoid loading document text in AbstractObjectBrowserLibraryManager.DocumentChangedAsync
  ...
@dibarbet dibarbet modified the milestones: Next, 16.7.P4 Jun 30, 2020
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.

4 participants