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

WIP - DO NOT MERGE - Spike for F# support #392

Open
wants to merge 2 commits into
base: 203
Choose a base branch
from
Open

Conversation

maartenba
Copy link

Have spent some time pair programming with @citizenmatt on basic completion/analyzer support for Azure Functions written in F#.

Cool pictures first

Behold!

image
image

Remarks second

This is by no means ready to merge, as there are some things that are hard/impossible right now. I'll annotate these in the PR and tag whoever I think this matters to.

@maartenba maartenba self-assigned this Nov 19, 2020
</ItemGroup>

<ItemGroup>
<Reference Include="FSharp.Compiler.Service, Version=2020.3.0.0, Culture=neutral, PublicKeyToken=3099b8d9d20e74bf">
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mfilippov / @auduchinok Right now, I have referenced assemblies from the F# plugin manually. Support for "full plugins" (back-end and front-end) seem hard to do.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's true. We don't have auto-reference for bundled plugins :( Maybe we should think about it.

{
foreach (var cronSuggestion in CronSuggestions)
{
var token = context.TokenAtCaret as ITokenNode;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@auduchinok Seems completion can only be triggered right before a string, so {caret}"....." triggers completion, "...{caret}...." does not. This completion provider now assumes the quotes are always included. This means that on L98 below, I have to provide completion items with " ?

(I could be misunderstanding the PSI as well)

Copy link
Member

@auduchinok auduchinok Nov 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may have weird completion contexts in places where it wasn't previously possible to complete things. There're chances I'll need to fix it up a bit for completion inside strings to work as expected.

plugins = ['DatabaseTools', 'JavaScriptLanguage', 'terminal', 'restClient'] + extraPlugins
plugins = ['DatabaseTools', 'JavaScriptLanguage', 'terminal', 'restClient', "com.jetbrains.rider.fsharp:$rider_fsharp_plugin_version"] + extraPlugins

// HACK: -- Load F# plugin from custom repository (our local disk)
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mfilippov @auduchinok Same thing here, manually referencing the F# plugin on frontend from a custom disk-based URL.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To fix it we need to add two-phase build for bundled plugins. Or auto-upload to the gallery.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-upload seems like an option, a least on the consuming side.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to upload EAP builds to the gallery - I want to do the same with the Unity ReSharper plugin (not Rider). Nightly seems overkill - could we upload those to the snapshots repository?

@@ -0,0 +1,5 @@
<idea-plugin>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

File needed to make F# an optional thing.

@maartenba maartenba marked this pull request as ready for review November 19, 2020 15:44
@citizenmatt
Copy link
Member

The other hack we had to do was add <idea-version …> into the F# plugin's plugin.xml. I've added a PR for F# to generate this during build. I don't see a reason for it to be disabled. See JetBrains/resharper-fsharp#206

Copy link
Member

@auduchinok auduchinok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, using F# plugin as a dependency for another plugin is really cool! I should probably try not to break APIs myself now. 😄

{
foreach (var cronSuggestion in CronSuggestions)
{
var token = context.TokenAtCaret as ITokenNode;
Copy link
Member

@auduchinok auduchinok Nov 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may have weird completion contexts in places where it wasn't previously possible to complete things. There're chances I'll need to fix it up a bit for completion inside strings to work as expected.

{
if (element.Arguments.Count != 1) return;

var resolveResult = element.ReferenceName.Reference.Resolve();
Copy link
Member

@auduchinok auduchinok Nov 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JFYI In F# plugin we sometimes tend to do checks that don't require resolve first, e.g. by checking names of references, to prevent calling FCS in other files where possible. Resolving type references should be fine, though, it's creating declared elements for functions/values what could be a problem in some cases.

@ForNeVeR ForNeVeR mentioned this pull request Jan 2, 2021
@auduchinok
Copy link
Member

auduchinok commented Mar 24, 2021

@maartenba 2021.1 F# plugin has completion context changes that should fix getting the literal for you. Please let me know if it doesn't work for your case.

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.

4 participants