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

Generate types from meta model #49

Merged
merged 32 commits into from
Jun 13, 2024

Conversation

TheAngryByrd
Copy link
Member

@TheAngryByrd TheAngryByrd commented Jun 4, 2023

WHAT

🤖 Generated by Copilot at 198ad18

This pull request improves the F# code formatting and testing of the language server protocol library. It updates the .editorconfig file, the test project, and the fantomas tool.

🤖 Generated by Copilot at 198ad18

fantomas renamed
F# code gets formatted well
Autumn of AST

🔧🆙🧪

WHY

Generates types from the metaModel provided by the LSP spec. Hopefully makes it so we don't have to hand roll as much and can stay up to date easier.

  • Generate Structures
  • Generate Enumerations
  • Generate Type Aliases
  • Generate XML docs *1
  • Check serialization/deserialization

1. Currently their metaModel docs have some stuff but it's not as extensive as use manually copying the docs from the LSP spec page itself.

Maybes:

  • Generate Interfaces/Abstract classes for Requests
  • Generate Interfaces/Abstract classes for Notifications

HOW

🤖 Generated by Copilot at 198ad18

  • Rename fantomas-tool to fantomas in dotnet-tools.json to use latest version of F# formatter (link)
  • Update TargetFramework to net7.0 in tests/Ionide.LanguageServerProtocol.Tests.fsproj to use latest .NET SDK (link)
  • Add GenerateTests.fs to tests/Ionide.LanguageServerProtocol.Tests.fsproj to generate test cases for language server protocol messages (link)
  • Add Fabulous.AST package to tests/Ionide.LanguageServerProtocol.Tests.fsproj to work with abstract syntax trees in F# (link)
  • Remove fsharp_max_infix_operator_expression setting from .editorconfig to avoid formatting issues with fantomas (link)
  • Add new settings to .editorconfig to control formatting of brackets, blank lines, arrays, lists, and lambdas in F# (link)

@TheAngryByrd TheAngryByrd force-pushed the generate-types-from-metaModel branch from 74ab60e to 6df1ab7 Compare May 9, 2024 13:35
@baronfel
Copy link
Contributor

baronfel commented May 24, 2024

Requests (will update as I update FSAC):

  • A few types had generated DebuggerDisplay attributes and members that we used in FSAC for logging as well:
    • Position
    • Diagnostics
  • Consider never generating anonymous types - they cannot be pattern-matched against and so make testing difficult. Specific instances:
    • MarkedString
    • all of the anonymous types in the Server/Client Capabilities sections

The anonymous types thing is also painful when constructing large or complex markdown strings - anon records assigned to a local binding will be given a type in your app's namespaces and therefore be incompatible with the anon records defined in this library.

@TheAngryByrd TheAngryByrd force-pushed the generate-types-from-metaModel branch from d978d61 to 744860f Compare May 29, 2024 05:05
@TheAngryByrd TheAngryByrd marked this pull request as ready for review June 10, 2024 14:28
@TheAngryByrd TheAngryByrd marked this pull request as draft June 10, 2024 14:29
@TheAngryByrd TheAngryByrd marked this pull request as ready for review June 11, 2024 22:36
@baronfel
Copy link
Contributor

At this point I'm completely happy with this. @TheAngryByrd has been running FSAC based on https://github.com/ionide/FsAutoComplete/pull/1301/files?diff=split&w=1 (the PR that incorporates this) for a few days now.

cc @razzmatazz as someone that I think was using this model - the changes are somewhat impactful, though mostly due to

  • uint32
  • no longer generating type aliases for erased unions

@baronfel
Copy link
Contributor

@TheAngryByrd Is this a squash, do you think? 32 commits is quite a few :D

@razzmatazz
Copy link
Contributor

cc @razzmatazz as someone that I think was using this model - the changes are somewhat impactful, though mostly due to..

that is not an issue, we will adapt to these changes

@baronfel
Copy link
Contributor

thanks for confirming @razzmatazz!

@baronfel
Copy link
Contributor

Alright, let's merge this and put out a new version!

@baronfel baronfel merged commit 5fec671 into ionide:main Jun 13, 2024
3 checks passed
@edgarfgp
Copy link
Contributor

Really nice to see that Fabulous.AST helped here :)

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