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

To interpolated string #1143

Merged
merged 9 commits into from
Jul 15, 2023
Merged

Conversation

nojaf
Copy link
Contributor

@nojaf nojaf commented Jul 13, 2023

IonideToInterpolatedString

Todo:

  • We should check the current --langversion to decide if this should be enabled or not.
    @TheAngryByrd how would one do that?

This first version has some limitations:

  • Only support regular strings (so no triple quote or verbatim)
  • No multiline strings
  • The entire application needs to be on one line

I think this is decent enough to throw out there and expand upon later.

WHAT

🤖 Generated by Copilot at cfb20e1

This pull request adds a new code fix that converts sprintf and printf calls to interpolated strings in F# 5. It updates the language server types to provide the code fix to clients and adds tests to verify its behavior. It also adds a new module and file to implement and test the code fix logic.

🤖 Generated by Copilot at cfb20e1

New code fix added
ToInterpolatedString
Autumn of F# 5

🆕🛠️🧪

WHY

HOW

🤖 Generated by Copilot at cfb20e1

  • Add a new code fix that converts sprintf and printf calls to interpolated strings (link, link, link, link, link)
  • Define the logic of the code fix in the ToInterpolatedString module in src/FsAutoComplete/CodeFixes/ToInterpolatedString.fs (link)
  • Add the ToInterpolatedString.fix function to the list of code fixes that the AdaptiveFSharpLspServer and FSharpLspServer types can provide in src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs and src/FsAutoComplete/LspServers/FsAutoComplete.Lsp.fs (link, link)
  • Add the ToInterpolatedStringTests.tests function to the list of tests that the tests function runs in test/FsAutoComplete.Tests.Lsp/CodeFixTests/Tests.fs (link)
  • Define a series of test cases for the new code fix in the ToInterpolatedStringTests module in test/FsAutoComplete.Tests.Lsp/CodeFixTests/ToInterpolatedStringTests.fs (link)

@TheAngryByrd
Copy link
Member

We should check the current --langversion to decide if this should be enabled or not.
@TheAngryByrd how would one do that?

I think this might come through on properties on a project https://github.com/fsharp/FsAutoComplete/blob/main/src/FsAutoComplete/LspServers/AdaptiveFSharpLspServer.fs#L723

@TheAngryByrd
Copy link
Member

@nojaf conflicts because @edgarfgp's PR got in first.

Also do you want to wait to try to only offer based on language version?

@nojaf
Copy link
Contributor Author

nojaf commented Jul 14, 2023

Also do you want to wait to try to only offer based on the language version?

Yeah, this really makes sense. I didn't really get anywhere with your pointer, unfortunately.

@TheAngryByrd TheAngryByrd marked this pull request as draft July 14, 2023 14:28
@TheAngryByrd
Copy link
Member

TheAngryByrd commented Jul 14, 2023

Ok gonna mark this as Request Changes

@TheAngryByrd TheAngryByrd marked this pull request as ready for review July 14, 2023 14:54
@nojaf
Copy link
Contributor Author

nojaf commented Jul 15, 2023

Tremendous thanks for adding support to detect the langversion @TheAngryByrd!
This is ready to go!

@nojaf nojaf requested a review from TheAngryByrd July 15, 2023 14:30
@TheAngryByrd TheAngryByrd merged commit 2de89b0 into ionide:main Jul 15, 2023
9 checks passed
@TheAngryByrd
Copy link
Member

Thanks a ton for this!

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