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

FSharpWorkspace prototype #17920

Open
wants to merge 17 commits into
base: lsp
Choose a base branch
from
Open

FSharpWorkspace prototype #17920

wants to merge 17 commits into from

Conversation

0101
Copy link
Contributor

@0101 0101 commented Oct 24, 2024

Summary

An F# workspace based on a dependency graph of files, projects, references.

It's kind of an early prototype but whoever is interested can already have a look and maybe give some useful feedback.

It can right now in VS-LSP deal with opening/closing/editing files, adding and removing files in projects, in-memory F# project references.

Public API so far

The user will create an instance of FSharpWorkspace and hold on to it. It's mutable and will be keeping the state of the workspace.

They can add F# projects to it with:

  • AddCommandLineArgs(projectPath, outputPath, compilerArgs)
  • That's it for now but we can potentially add other ways if people need it

They can update the projects if anything changes the same way. Just call AddCommandLineArgs with the new arguments.

  • Dependencies between projects are automatically discovered from the compiler args (-r: matching outputPath). So outputPath has to be provided correctly for in-memory references to work.

They notify about files opening/closing/being edited by OpenFile, CloseFile, ChangeFile.

They can request FSharpProjectSnapshot for a given file and it will be lazily computed based on current state. The snapshot is still immutable and contains everything needed by FSharpChecker to work with the file. Parts of it may be lazily computed (e.g. if we're only interested in parsing it will not compute hash of references).

Some technical details

To save some hash re-computation I decide to further (internally) split ProjectSnapshot into

  • ProjectCore = everything except files and (in-memory) references
  • ProjectBase = ProjectCore + (in-memory) references
  • ProjectSnapshot = ProjectBase + source files

I need help with names! 😂

I've been trying to come up with the simplest model for lazy "reactive" computation for the workspace. Right now it's the DependencyGraph. I don't love how it deals with different types of dependencies (DUs and "unpacking" them) but not sure how to solve it better without some sort of type-level programming 😅

@0101 0101 requested a review from a team as a code owner October 24, 2024 15:43
static member CreateFromString(filename: string, content: string) =
FSharpFileSnapshot(
filename,
Md5Hasher.hashString content |> Md5Hasher.toString,
Copy link
Member

Choose a reason for hiding this comment

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

Should probably eventually change to something more proper, can't ship with md5 hashing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Most definitely. We should probably bring in some version of xxHash.

Copy link
Member

@vzarytovskii vzarytovskii Oct 25, 2024

Choose a reason for hiding this comment

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

Yeah. That was my thinking as well.

Copy link
Member

@vzarytovskii vzarytovskii left a comment

Choose a reason for hiding this comment

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

Personally, I think we should just merge it and iterate further.

@0101
Copy link
Contributor Author

0101 commented Oct 25, 2024

Personally, I think we should just merge it and iterate further.

Yeah I just thought this might be a good point for people to start looking at it and poke holes or give advice on how to do it better.

@nojaf
Copy link
Contributor

nojaf commented Oct 25, 2024

It would be nice if some NuGet packages could get attached to this PR (Upload artifact stuff in GitHub Actions). This would make it easier to play with in fsautocomplete for example.

@vzarytovskii
Copy link
Member

It would be nice if some NuGet packages could get attached to this PR (Upload artifact stuff in GitHub Actions). This would make it easier to play with in fsautocomplete for example.

Yeah we can do that, but probably won't be final, since types/packages/namespaces/etc can change.

@psfinaki
Copy link
Member

I am still grasping the topic but...

To save some hash re-computation I decide to further (internally) split ProjectSnapshot into

  • ProjectCore = everything except files and (in-memory) references
  • ProjectBase = ProjectCore + (in-memory) references
  • ProjectSnapshot = ProjectBase + source files

I need help with names! 😂

How about ProjectProperties, ProjectPropertiesWithReferences and Project, respectively?

Base and Core and honestly very obscure suffixes. Better go long! :D

@0101
Copy link
Contributor Author

0101 commented Oct 29, 2024

How about ProjectProperties, ProjectPropertiesWithReferences and Project, respectively?

Base and Core and honestly very obscure suffixes. Better go long! :D

ProjectProperties could work. Although it's quite similar to ProjectOptions which we already have (and which I'd like to deprecate at some point).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

5 participants