-
Notifications
You must be signed in to change notification settings - Fork 123
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
Compiler api harmonise #639
Conversation
Add compiling API’s to SourceCodeServices
Fixes / Implements #516 |
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.
Great work. Could you also please deprecate the methods in SimpeServices.fsi? And adjust any documentation (if there is any for these).
@@ -2697,6 +2807,66 @@ type FSharpChecker(projectCacheSize, keepAssemblyContents, keepAllBackgroundReso | |||
|
|||
member ic.TryGetRecentTypeCheckResultsForFile(filename, options, ?source) = ic.TryGetRecentCheckResultsForFile(filename,options,?source=source) | |||
|
|||
member ic.Compile(argv: string[]) = |
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.
I think these operations should all go via the backgroundCompiler and the Reactor thread - that's more obvious now that they are all part of the proper service API. Could you add that as part of this PR? Or we could do that as a separate item of work.
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.
I think a separate PR as that would be a change of functionality, Im not entirely sure on the interaction of the compile action with an ongoing background compilation. Maybe that in itself is a good reason for a reactor lock.
Do we need to depreciate the compile methods in Maybe |
Yes, I believe it can now be deprecated (though please double check, maybe there's something dangling). |
Theres quite a few uses around on github, I think the bast bet is to mark it as depreciated for now, I can add that to this PR. |
Great, thanks |
Here's the AppVeyor failure:
|
Hmm, dotnetcore issue? |
At the moment I have to disable dotnetcore to get mono to compile. Maybe #637 needs a closer look to get the CI stable. |
@7sharp9 Yes. Either a missing reference, or changed namespace, or need to #if out the functionality, or something. |
@7sharp9 I think |
@dsyme Appveyor is ok now. |
Thanks for this @7sharp9 |
I'll do a further PR later to add the depreciate flag in simplesourcecodeservces and the reactor thread. |
@7sharp9 Thanks. Let's aim to nuke SimpleSourceCodeServices and bring it all into FSharpChecker. |
This PR addresses the fact the
SimpleSourceCodeServices
has compile specific API's that are not present inSourceCodeServices
.The benefits of this PR is that a single
FSharpChecker
can be created and used to do compile operations, rather than having to move betweenSimpleSourceCodeServices
andSourceCodeServices
.