Fantomas: support cursor api #484
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DedSec256
force-pushed
the
net231-fantomas-caret
branch
from
b058598 to
2ad070a
Compare
auduchinok
reviewed
Feb 24, 2023
ReSharper.FSharp/src/FSharp.Psi.Features/src/CodeCleanup/FSharpReformatCode.fs
Outdated
Show resolved
Hide resolved
...harp/testData/fantomas/FantomasRunOptionsTest/local tool 6_0 with cursor/source/LargeFile.fs
Outdated
Show resolved
Hide resolved
DedSec256
force-pushed
the
net231-fantomas-caret
branch
from
a939f17 to
fe0ef02
Compare
auduchinok
reviewed
Mar 3, 2023
Comment on lines
+192
to
+193
| if (CurrentVersion < Version60) | ||
| return new RdFormatResult(formatResult.Replace("\r\n", args.NewLineText), null); |
There was a problem hiding this comment.
Could you add a comment why this is needed, please?
There was a problem hiding this comment.
You should be able to pass in lf as part of the configuration btw.
(And override whatever the user had)
There was a problem hiding this comment.
This is a relic of the past. We can try to rewrite it in a separate PR.
There was a problem hiding this comment.
I think a comment about the version check would stil be good here. With a link to a PR or a commit.
DedSec256
commented
Mar 3, 2023
ReSharper.FSharp/src/FSharp.Psi.Features/src/CodeCleanup/FSharpReformatCode.fs
Outdated
Show resolved
Hide resolved
auduchinok
reviewed
Mar 3, 2023
| @@ -79,8 +79,8 @@ type FSharpReformatCode(textControlManager: ITextControlManager) = | |||
| sourceFile.GetPsiServices().Files.CommitAllDocuments() | |||
| with _ -> () | |||
| else | |||
| let textControl = textControlManager.VisibleTextControls |> Seq.find (fun c -> c.Document == document) | |||
| let cursorPosition = textControl.Caret.Position.Value.ToDocLineColumn() | |||
| let textControl = textControlManager.VisibleTextControls |> Seq.tryFind (fun c -> c.Document == document) | |||
There was a problem hiding this comment.
What if there are multiple editors with the same document? This check seems to ignore which editor was actually focused before the reformat. Please try to use data contexts or other ways to get the proper text control.
DedSec256
force-pushed
the
net231-fantomas-caret
branch
from
260e80d to
9877c89
Compare
auduchinok
approved these changes
Mar 3, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fsprojects/fantomas#2727