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

[Windows support] Update OmniSharpServer submodule #209

Merged
merged 1 commit into from
Aug 30, 2015

Conversation

micbou
Copy link
Collaborator

@micbou micbou commented Aug 29, 2015

OmniSharp Server was modifying the drive letter case of paths causing test failures on ycmd side. This is fixed upstream (see PR OmniSharp/omnisharp-server#207) so we need to update the submodule.

Fix 6 tests (1 error and 5 failures) on Windows (listed in the commit message).

Fix the following tests on Windows:
 - GetDetailedDiagnostic_CsCompleter_Works
 - RunCompleterCommand_GoTo_CsCompleter_Works
 - RunCompleterCommand_GoToImplementation_CsCompleter_Works
 - RunCompleterCommand_GoToImplementationElseDeclaration_CsCompleter
_NoImplementation
 - RunCompleterCommand_GoToImplementationElseDeclaration_CsCompleter
_SingleImplementation
 - RunCompleterCommand_GoToImplementationElseDeclaration_CsCompleter
_MultipleImplementations
@vheon
Copy link
Contributor

vheon commented Aug 29, 2015

In that PR @nosami talked about the new OmniSharp/omnisharp-roslyn does someone know what improvements would it bring?

@nosami
Copy link

nosami commented Aug 29, 2015

@vheon OmniSharp/omnisharp-vim#215

Another big advantage is that it uses Roslyn instead of NRefactory (hence the name :) ). It can handle C#6 and is better at handling malformed C# than NRefactory.

Omnisharp-Roslyn also has support for DNX projects (which are awesome because no csproj files to update :) )

@vheon
Copy link
Contributor

vheon commented Aug 30, 2015

@nosami thanks for the infos and the link!

@nosami
Copy link

nosami commented Aug 30, 2015

It also has ScriptCS support (csx). VB.Net and F# coming soon too.

It also has a few other features not in the old server. Goto definition can jump into metadata-as-source ... and it has an endpoint for semantic syntax highlighting.

On the negative side, it has no endpoints for modifying csproj files (yet)

@nosami
Copy link

nosami commented Aug 30, 2015

ycmd could also benefit from snippet support OmniSharp/omnisharp-vim#209

@mispencer
Copy link
Contributor

I have a implementation of ycmd using omnisharp-roslyn somewhere in my repository. As I recall, I had major problems getting omnisharp-roslyn running, and once I did, I ran into the Windows maximum path length...

@nosami
Copy link

nosami commented Aug 30, 2015

@mispencer fwiw, omnisharp-roslyn is the backend powering VSCode and Atom. Vim, emacs and sublime can be configured to use it.

The max_path issue was fixed some time ago. The main thing that would need changing to make YCM work would be to convert requests to JSON format from form-url-encoded. JSON formatted requests work with both the old server and the new one.

@Valloric
Copy link
Member

Seems like a migration to omnisharp-roslyn lies in our future. :)

@mispencer A pull request would be highly appreciated. :) You mentioned having done the work in a personal fork.

@homu r+

@homu
Copy link
Contributor

homu commented Aug 30, 2015

📌 Commit 585cf77 has been approved by Valloric

@homu
Copy link
Contributor

homu commented Aug 30, 2015

⌛ Testing commit 585cf77 with merge b46b8f0...

homu added a commit that referenced this pull request Aug 30, 2015
[Windows support] Update OmniSharpServer submodule

OmniSharp Server was modifying the drive letter case of paths causing test failures on ycmd side. This is fixed upstream (see PR OmniSharp/omnisharp-server#207) so we need to update the submodule.

Fix 6 tests (1 error and 5 failures) on Windows (listed in the commit message).
@homu
Copy link
Contributor

homu commented Aug 30, 2015

☀️ Test successful - status

@homu homu merged commit 585cf77 into ycm-core:master Aug 30, 2015
@micbou micbou deleted the windows-update-omnisharp-server branch August 30, 2015 18:23
@mispencer
Copy link
Contributor

I'll take another look at omnisharp-roslyn on Monday, time permitting.

@mispencer mispencer mentioned this pull request Sep 1, 2015
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.

6 participants