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

Fantomas integration #454

Merged
merged 14 commits into from
Oct 2, 2019
Merged

Fantomas integration #454

merged 14 commits into from
Oct 2, 2019

Conversation

nojaf
Copy link
Contributor

@nojaf nojaf commented Sep 14, 2019

Continuation of the conversion in ionide/ionide-vscode-fsharp#274.
This is poor implementation meant to be improved over time (with the needed guidance of the maintainers). Please comment on everything little detail you see 😉.

First thing I noticed is that the latest beta of Fantomas is already on FCS 31. Currently somewhat waiting on #450.

Hopefully, this experiment will highlight if we need any changes to the Fantomas 3.0 API (See fsprojects/fantomas#454).

//cc: @Krzysztof-Cieslak @jindraivanek

@Krzysztof-Cieslak
Copy link
Member

OK. Thanks a lot for driving this forward.

So the first step - to get access to the file source, AST and checker you need to implement functionality in Commands.fs file (it's in FsAutoComplete.Core project). The state object in Commands class will give you access to the file content (state.Files) ,to the AST of the current file (state.CurrentAST) (this probably should be enough as we always can format only current file) or to more-or-less up-to-date FSharpParseFileResults from any file tracked by FSAC (state.ParseResults).

In Commands class there exists checker field, but it's our custom wrapper, you'll probably need to expose "standard" FSharpChecker in CompilerServiceInterface.fs in FSharpCompilerServiceChecker class.

To get FSharpParsingOptions you can get FSharpProjectOptions from state.FileCheckOptions and then transform it with Utils.projectOptionsToParseOptions

@Krzysztof-Cieslak
Copy link
Member

In meantime I'll poke @baronfel about finishing #450 ;-)

@nojaf
Copy link
Contributor Author

nojaf commented Sep 15, 2019

Thanks for the pointers, Kris.
What should TextDocumentOnTypeFormatting do exactly?
And should TextDocumentRangeFormatting return the entire document or just the reformatted fragment?

@Krzysztof-Cieslak
Copy link
Member

TextDocumentOnTypeFormatting - as far as I understand it's formatting that is run after every key stroke, I think we don't want to implement it? Not sure how fast is Fantomas ;-)

TextDocumentRangeFormatting - should return only the edited range.

The full LSP specification is available here: https://microsoft.github.io/language-server-protocol/specification#textDocument_formatting

@nojaf
Copy link
Contributor Author

nojaf commented Sep 16, 2019

Ok thanks, so TextDocumentOnTypeFormatting someday maybe 😋.
Fantomas is getting faster but I would not implement that one right now.

TextDocumentRangeFormatting - should return only the edited range.

That is great as it is the same method Rider needs. I'm playing around with the idea to trim the public API of Fantomas down to the method we know that are in use by consumers. So maybe, just maybe we can drop https://github.com/nojaf/fantomas/blob/api-changes/src/Fantomas/CodeFormatter.fs#L37.

@nojaf
Copy link
Contributor Author

nojaf commented Oct 2, 2019

@Krzysztof-Cieslak I think this is ready now, currently, it doesn't contain any options for settings but we can always add that later.

@Krzysztof-Cieslak Krzysztof-Cieslak marked this pull request as ready for review October 2, 2019 10:57
@Krzysztof-Cieslak Krzysztof-Cieslak merged commit 89e9441 into ionide:master Oct 2, 2019
@nojaf nojaf deleted the fantomas branch October 2, 2019 11:35
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