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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Cake support #1681

Merged
merged 2 commits into from Oct 5, 2017

Conversation

Projects
None yet
4 participants
@mholo65
Contributor

mholo65 commented Aug 2, 2017

Adding support for Cake (I think). Honestly, I have no idea what I'm doing 馃槃 @gep13 This needs some TypeScript love from you.

OmniSharp-Roslyn PR here OmniSharp/omnisharp-roslyn#932

NB! For some reason, you'll have to switch language mode from Cake to C# (If you have Cake VS Code Extension installed) to get intellisense.

@@ -460,13 +462,11 @@
"razor"
]
},
"runtime": "node",

This comment has been minimized.

@DustinCampbell

DustinCampbell Aug 2, 2017

Collaborator

You don't want this change

This comment has been minimized.

@mholo65

mholo65 Aug 2, 2017

Contributor

Oh, are VSCode adding these changes automatically? Can't remember changing these.

This comment has been minimized.

@DustinCampbell

DustinCampbell Aug 2, 2017

Collaborator

Yes, this happens when you debug the extension.

"runtimeArgs": [],
"variables": {
"pickProcess": "csharp.listProcess",
"pickRemoteProcess": "csharp.listRemoteProcess"
},
"program": "./out/src/coreclr-debug/proxy.js",

This comment has been minimized.

@DustinCampbell

DustinCampbell Aug 2, 2017

Collaborator

Or this one

},
"linux": {
"program": "./.debugger/vsdbg-ui"
}
},

This comment has been minimized.

@DustinCampbell

DustinCampbell Aug 2, 2017

Collaborator

Or these

@@ -1387,13 +1396,11 @@
"razor"
]
},
"runtime": "node",

This comment has been minimized.

@DustinCampbell

DustinCampbell Aug 2, 2017

Collaborator

Not this

"runtimeArgs": [],
"variables": {
"pickProcess": "csharp.listProcess",
"pickRemoteProcess": "csharp.listRemoteProcess"
},
"program": "./out/src/coreclr-debug/proxy.js",

This comment has been minimized.

@DustinCampbell

DustinCampbell Aug 2, 2017

Collaborator

nor this

"program": "./.debugger/vsdbg-ui"
},
"linux": {
"program": "./.debugger/vsdbg-ui"

This comment has been minimized.

@DustinCampbell

DustinCampbell Aug 2, 2017

Collaborator

Or this

@gep13

This comment has been minimized.

gep13 commented Aug 4, 2017

@mholo65 was there something in particular that you thought was "wrong" with this PR? The changes look good to me. 馃憤

@gep13

gep13 approved these changes Aug 4, 2017

LGTM

@mholo65

This comment has been minimized.

Contributor

mholo65 commented Aug 4, 2017

@gep13 well if we could figure out why we have to change language mode from Cake to C# before this extension wants to send any requests to omnisharp.exe, that would be great 馃槈

omnisharp.exe is started when a.cake file is opened, but no requests are sent. Current fix is to switch language mode

@DustinCampbell

This comment has been minimized.

Collaborator

DustinCampbell commented Aug 4, 2017

well if we could figure out why we have to change language mode from Cake to C# before this extension wants to send any requests to omnisharp.exe, that would be great 馃槈

I posted how that happens in the #cake channel on Slack. The file types that trigger "csharp" as the language is defined in VS Code itself right here. I'll submit a change to add "*.cake" to this list on your behalf today.

@gep13

This comment has been minimized.

gep13 commented Aug 4, 2017

@DustinCampbell said...
The file types that trigger "csharp" as the language is defined in VS Code itself right here. I'll submit a change to add "*.cake" to this list on your behalf today.

Yes, I saw that in the Slack channel, and that is why I raised this issue #1684, I was trying to get clarity on whether the syntax highlighting that we provide for .cake files would be affected by making that change.

Do you know if this will be the case?

@DustinCampbell

This comment has been minimized.

Collaborator

DustinCampbell commented Aug 4, 2017

Yes, the syntax highlighting for .cake files would be affected. What differences does Cake add to C# syntax highlighting? Note that the official C# support for cake highlighting is here: https://github.com/dotnet/csharp-tmLanguage.

@DustinCampbell

This comment has been minimized.

Collaborator

DustinCampbell commented Aug 4, 2017

I think this is generally a problem of trying to spread Cake support across two extensions. If you're trying to have a Cake Support extension provide syntax highlighting and the C# extension, I'm less comfortable with taking this change.

@DustinCampbell

This comment has been minimized.

Collaborator

DustinCampbell commented Aug 4, 2017

Cake is really just C# script with some semantic differences, correct? If that's the case, I would proposal that the Cake Support extension doesn't provide it's own syntax highlighting, but takes advantage of C#'s. Then, the Cake Support extension would be modified to operate on the "csharp" language but only when the extension is ".cake". Thoughts?

@DustinCampbell

This comment has been minimized.

Collaborator

DustinCampbell commented Aug 4, 2017

I'm going to go ahead and move this conversation over to #1684.

@DustinCampbell

This comment has been minimized.

Collaborator

DustinCampbell commented Oct 5, 2017

This looks good. Sorry for the delay!

@DustinCampbell DustinCampbell merged commit 1af9733 into OmniSharp:master Oct 5, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@mholo65 mholo65 deleted the cake-build:feature/cake branch Oct 5, 2017

@mholo65 mholo65 restored the cake-build:feature/cake branch Oct 5, 2017

@gep13

This comment has been minimized.

gep13 commented Oct 5, 2017

WOO HOO! Thanks for merging this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment