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

Try starting Omnisharp server without solution file #1663

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

sankhesh
Copy link
Contributor

@sankhesh sankhesh commented Oct 12, 2022

With this change, ycmd will attempt to start the Omnisharp server with the current working directory when no solution file is found. The runtime check for solution file presence is not required and exception raising is defered to Omnisharp.


This change is Reviewable

@puremourning
Copy link
Member

@sankhesh can you given an example of a specific problem that this solves?

@mispencer WDYT?

@sankhesh
Copy link
Contributor Author

@puremourning
Currently, ycmd checks for a solution file in the module configuration and current working directory and starts the Omnisharp server iff a solution file exists.

However, it is common for C# projects (Unity projects, for example) to not have a VS solution file to go with. Secondly, Onmisharp doesn't really require a solution file anymore. It just needs a working directory which is what the change provides.

Hth

@sankhesh sankhesh force-pushed the empty_solution_omnisharp_server branch from 0a0fc8c to 3a708f9 Compare October 12, 2022 13:44
@puremourning
Copy link
Member

Thanks,

Can we please add a test directory and some tests that show it works? Hopefully just a simple project without a solution file and basic tests. We have test primitives for things like StartCompleterServerInDirectory or something.

Otherwise, I'm likely to just break this as I don't c# often

@sankhesh
Copy link
Contributor Author

Yeah - the test failure in CI is related to this feature. I'll fix the tests so that it provides sufficient testing for this feature as well.

Btw, can you label this PR as "hacktoberfest-accepted". Might as well :)

@puremourning
Copy link
Member

Btw, can you label this PR as "hacktoberfest-accepted". Might as well :)

I don't know what that means but if you need me to do something for you to earn internet points, then let me know and I guess I can do it if it isn't too much effort.

@sankhesh
Copy link
Contributor Author

Ah, I see. Hacktoberfest is a fun month-long fest that rewards open source contributors. To claim this PR as a valid contribution, a maintainer (I assume you're one) would need to label this PR with the text hacktoberfest-accepted.

@puremourning
Copy link
Member

@Mergifyio rebase

With this change, ycmd will attempt to start the Omnisharp server with
the current working directory when no solution file is found. The
runtime check for solution file presence is not required and exception
raising is defered to Omnisharp.
- Change the test to ensure that the OmniSharp server starts with an
empty solution file.
- Remove the test that was testing that the case where OmniSharp server
should not start when an unambiguous solution file is not found.
@Valloric Valloric force-pushed the empty_solution_omnisharp_server branch from f4bd725 to fa3bdab Compare January 9, 2023 19:52
@mergify
Copy link
Contributor

mergify bot commented Jan 9, 2023

⚠️ This pull request got rebased on behalf of a random user of the organization.
This behavior will change on the 1st February 2023, Mergify will pick the author of the pull request instead.

To get the future behavior now, you can configure bot_account options (e.g.: bot_account: { author } or update_bot_account: { author }.

Or you can create a dedicated github account for squash and rebase operations, and use it in different bot_account options.

@mergify
Copy link
Contributor

mergify bot commented Jan 9, 2023

rebase

✅ Branch has been successfully rebased

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.

2 participants