Large refactor of processing structure #43
Conversation
Backwards-incompatible changes:
* Update helptext command to return { Name = ""; Text = "" }. Fixes #35.
* `project` command response now has 'null' for OutputFile and
TargetFramework if a value cannot be determined.
Other changes:
* FSharp.CompilerBinding removed, and used parts absorbed. Fixes #17.
* ScriptCheckerOptions fetched with no timeout, and also stores them.
Fixes #18, #28.
* If a .fs file is not in a loaded project, produce an incomplete
typecheck environment for it to give basic results.
* Update parsing of project options to include ProjectReferences. Fixes #39.
* Separate parsing of commands, main command loop, and formatting of
response message into separate modules.
rneatherway
force-pushed
the
big-refactor
branch
from
1586d8d to
99d6091
Compare
| if str = "<<EOF>>" then List.rev input | ||
| else readInput (str::input) | ||
|
|
||
| // Parse 'parse "<filename>" [full]' command |
There was a problem hiding this comment.
The [full] here should be [sync], I guess?
|
The changes look good to me. I don't know the full source code anymore, but I had a look through the new version and did not find anything that looked problematic. I still added a bunch of comments here and there - mostly either small things or things that are OK for now, but could be improved in the future. Thanks for doing all the work on this! It's so cool to see this helping Atom too! |
| open Microsoft.FSharp.Compiler | ||
| open Microsoft.FSharp.Compiler.SourceCodeServices | ||
|
|
||
| type ParseAndCheckResults(parse: FSharpParseFileResults, |
There was a problem hiding this comment.
Most of the methods in this type are calling Async.RunSynchronously - so it makes me wonder if they should be themselves asynchronous and the caller should take care of blocking?
(I guess it does not matter now - but it could be useful for cancellation and it might make this file actually reusable for other projects. I think I could have a use for this in some of the web-based F# editors like the one on www.fun3d.net!)
There was a problem hiding this comment.
Yes, you're probably right. These are just some simple wrappers around FCS methods to handle the ident cracking etc.
There was a problem hiding this comment.
And what I meant to say is that perhaps if there is a canonical implementation of ident cracking etc in FCS, something like this file could also be pushed into FCS.
|
That's all from me :-). The failing AppVeyor build seems to be just that something gets called before the compiler gets all information - which is pretty silly issue. Not sure what to do about it. |
|
Thanks a lot for taking a look, it's very useful to have extra eyes on this code. More editors using this are one of the main motivations for working more on this too! The AppVeyor build failure is due to #22, and some of the older tests have massive waits inserted to work around this. After converting the rest to JSON those new ones now intermittently fail :-( It's a choice between allowing editors to specify a timeout by using |
rneatherway
force-pushed
the
big-refactor
branch
from
68fc882 to
e47198a
Compare
Large refactor of processing structure
|
How did you get on with removing the language service, any issues? |
|
No it was all smooth sailing as far as removing the background agent is concerned. I still use a similar wrapper to what you have to handle the ident cracking, as Tomas and I were saying higher up that should really be in FCS. |
|
You can backtrack easy enough if you have already dome the tokenisation I created a prototype of this a few years ago that went up 10 lines, seems to work ok, its the up front processing which take the longest. |
The problem is that in the FsAutoComplete setting we haven't, and we can't easily because we don't get deltas for file source text, just the whole file each time. Tokenizing on each parse seems like it might be too expensive. |
|
Thats how we do it for semantic highlighting, after a parse is complete we tokenise the file and extract any additional keywords that might be defined via CE's |
|
I could do partial tokenisation as the lower section from the changed file is known, but I don't currently. |
|
I take it the tokenisation pass is done asynchronously? And then any extra highlighting info can be applied when it is done? Using the tokenisation information for ident cracking would mean waiting for a parse to be complete and then also for the tokenisation pass before any cracking can be done. It also needs to be completely up to date to get the right tokens, whereas a stale ParseResults is normally good enough. |
|
Yep, additional highlighting is applied asynchronously when it completes. I was just talking about the tokenisation in general in XSF#. You don't need to parse to run the tokeniser, in this instance I do because I require the symbol info to do semantic highlighting. The tokenisation could be yielded at the same time as the parse so that it can be reused later. i.e. semantic highlighting when parsing is finished. |
|
Right, yes I see. It will be too slow to use for the ident cracking still if we tokenise the whole file each time though won't it? |
|
I found it was faster than the parser combinator unless you start looking at large files. |
|
For cracking caret±10 lines should suffice |
So that means you would tokenize the whole file?
Or just go back 10 lines? That is problematic because you might end up in the middle of a string or comment. |
|
Regarding the combinators, whats the purpose of the empty lists like this? |
|
That empty list is because |
Fixes Timeout exception in FSharp.AutoComplete, very rare #18, Sometimes getting timeout #28.
typecheck environment for it to give basic results.
response message into separate modules.