-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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
Bring SignatureHelp support from EditorFeatures.Wpf to EditorFeatures.Cocoa #50389
Conversation
These files are based on dotnet/runtime@cf258a14 (v5.0.0-rtm.20519.4).
The APIs providing the scenario are not public, and not easily ported.
This code normally runs in a 64-bit process, so favor the faster implementation for that case.
…ered in MEF only vs extensibility
…istration Use IEventListener to register workspaces for LSP instead of inside AbstractPackage
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.
👍
[LSP] In brace completion only add a new line between the braces if none exist
No changes at all. Linked files is a good idea, I didn't think of, and makes me realise that an EditorFeatures.Common could potentially just be a shared project, if it's easier than shipping something new. |
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.
Thanks @davidwengier! Approved assuming linked files will be used.
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.
Auto-approval
Add ProjectOperationBenchmarks
Implement SegmentedDictionary<TKey, TValue>
Bring SignatureHelp support from EditorFeatures.Wpf to EditorFeatures.Cocoa
Apologies, I am afraid I am encountering technical difficulties that might have hampered my ability to assist with merging this pull request. I will continue to try to assist if there are further changes to this pull request. |
@davidwengier Did you mean to close? |
Yeah, I have no idea what happened, I didn't close it manually, but at the end of the day the changes are in master so we're good. When I looked at it, the checks were green but GitHub said there were conflicts (without enumerating what they were) and msftbot said it encountered difficulties. So I merged master in locally, which had no conflicts, shrugged, and pushed. After I pushed I saw it close, and all of Sam's commits from other branches came in. I'm guessing msftbot merged the PR, but then somehow GitHub saw my push included only commits already in master, so closed the PR automatically. |
Err.. can we def have a meeting on this . I didn't even know this lib existed. Def want to have a clean story here for work, esp. around ensuring we aren't unnecessary duplicaitng htings or having to fix things in multiple locations. |
can you clarify which chnages went into master? |
Before GitHub did whatever it did, this PR had these three commits in it, which are now in master:
The library has existed for as long time AFAIK, but was in the VS Editor repo until recently. It got moved to Roslyn in #48866 to make changes easier, and to avoid having to duplicate (which stupidly, I forgot about and initially did, before Jason rightly pointed out that linked files were better). Having it Roslyn does solve a dependency mess that I used to have to deal with weekly for VS Mac insertions, but unfortunately it doesn't help us avoid breaking VS for Mac itself, which is what this PR is in response to. |
#50345 moved some files to EditorFeatures.Wpf that are needed by VS for Mac, so this adds them to EditorFeatures.Cocoa so VS Mac can consume them.
An ideal solution here would presumably be an EditorFeatures.Common that can reference Microsoft.VisualStudio.Language.Intellisense, but not WPF, and then these could be shared. I'm not doing that here simply because I have no idea what infrastructure pieces this would require, and its 4pm my time :) Happy to do it if someone wants to let me know whats involved.