search

ionide / FsAutoComplete

F# language server using Language Server Protocol
Other
385 stars 151 forks source link

Separate fsi extra parameters #1299

Closed greggyb closed 1 month ago

greggyb commented 1 month ago

FIX: #1210; split FSIExtraParameters in two

FSIExtraParameters (old) becomes FSIExtraInteractiveParameters and FSIExtraSharedParameters.

Previously, FSIExtraParameters was passed directly to the compiler for code analysis. This is to ensure that the static analysis of a .fsx script file is congruent with the code being evaluated in the interpreter. The FSI interpreter has different parameters than the compiler, documented here.

When an interactive-only parameter was used, this led to compiler errors.

It is intended that FSAC will learn to manage the interactive interpreter in the future, but it does not yet do so. Editor plugins manage the interpreter right now. Some plugins (at least Ionide-vim) use the config option FSIExtraParameters to configure that interpreter, and this makes sense.

To support the current state and the future state, we split the single FSIExtraParameters into FSIExtraInteractiveParameters and FSIExtraSharedParameters so that the interactive interpreter can be launched with the union of these two and the compiler can receive only the shared parameters that it knows how to deal with. This allows editors to use both config options today to launch the interpreter. Today, FSAC only uses the shared parameters.

In the future, when FSAC learns to manage the interactive interpreter, it can union the parameters for the interactive session and continue to pass only the shared parameters to the compiler. When this change occurs, if an editor plugin still manages the dotnet fsi interpreter, that will continue to work. Editor plugins can opt in to FSAC-managed interpreters at their will, and with the same configuration options the users already have set up.

Additional discussion:

baronfel commented 1 month ago

Looks like formatting is off - you can run dotnet build -t Format from the repo root to format things and check in the changes.

Thanks for this comprehensive PR!

baronfel commented 1 month ago

If the CI is green I'll go ahead and merge this and create a new patch-release of FSAC to unblock ionide-vim. Thanks again for the analysis and attention to detail in this Issue and PR @greggyb!