ionide / Ionide-vim

F# Vim plugin based on FsAutoComplete and LSP protocol
MIT License
163 stars 20 forks source link

dotnet fsi crashes with long lines #82

Closed greggyb closed 1 month ago

greggyb commented 1 month ago

Describe the bug

Bug is reported on the fsharp repo. Crosslinking here for visibility and also a workaround.

If you have the dotnet fsi terminal open in multiple windows in vim, it seems to behave as if it is the widest one. In the short term, to work around this bug, I am launching my fsi window, then I am opening a new tab :tabnew and in that new tab, switching to the fsi terminal buffer. That gives the terminal maximum width. Then I :tabn back to my "real" work, where I have 2 vertical splits, one for each of my source file and my fsi terminal.

I don't know if it's worth baking such a workaround into ionide-vim, especially if they fix the interpreter quickly. But I think the workaround is worth sharing here.

If you would prefer not to have this hanging out as an issue, feel free to close it. If desired, I can write a PR for README only that describes this behavior.

greggyb commented 1 month ago

Actually, it appears to be readline-related. The better workaround, now, seems to be to add this line to one's .vimrc or init.vim:

let g:fsharp#fsi_extra_parameters = ['--readline-']

I'd argue that with a workaround this simple, it is a good idea to add this as a default configuration. If agreed, I'm happy to raise a PR for the change.

greggyb commented 1 month ago

Additional complications here: https://github.com/ionide/FsAutoComplete/issues/1210

In short, FSAC sends these parameters (from g:fsharp#fsi_extra_parameters) to the compiler and checkers, and we get linting errors when setting the option as I suggested above. --readline is only an option for dotnet fsi, but not for anything else that FSAC manages, so things that don't want that arg are getting it and complaining.

Some conversation in the linked issue will likely have implications on what Ionide-vim does.

greggyb commented 1 month ago

I will be raising a PR for FSAC soon. That will have knock-on implications for Ionide-vim. I'll create a congruent PR here as well.

In brief, the plan is to have two parameters which replace and obsolete the current fsi_extra_parameters, allowing FSAC to use the right options for (future) interactive codepaths and (current+future) analysis codepaths.

greggyb commented 1 month ago

I have raised the PR for FSAC and have tested a local change for Ionide-vim to align with that.

PR: https://github.com/ionide/FsAutoComplete/pull/1299

With this, the existing FSIExtraParameters config option is split in two. The old parameter goes away and the two new parameters are:

We need to append everything from both of those to the dotnet fsi command. FSAC uses the shared params to pass into the compiler for analysis of script files.

greggyb commented 1 month ago

@cannorin , would you like me to raise a PR for the changes to support this now, or would you prefer to wait for the release of FSAC that has these changes? I'm happy to follow your preference.

cannorin commented 1 month ago

@greggyb As I understand it, there is no good workaround right now since passing --readline to FSAC via commandline would cause lint errors. If so, I think we should wait for the fix to arrive in FSAC.

greggyb commented 1 month ago

All right. I'll sit on the PR until then.

In the meantime, if anyone is having issues with this, an easy workaround for the current version of both FSAC and Ionide-vim is to have the following two lines in your .vimrc or init.vim:

let g:fsharp#fsi_command = "dotnet fsi --readline-" " add interactive-only params directly here
let g:fsharp#fsi_extra_parameters = [ ... ] " only options shared with the compiler

This is because, right now and in the near-to-mid-term future, FSAC does nothing about the interpreter. They want to enhance it to manage a hosted interpreter in the future. For now, and even after the PR for FSAC lands into a release, the interactive options will be ignored.

You can identify shared vs interactive-only commands here: https://learn.microsoft.com/en-us/dotnet/fsharp/language-reference/fsharp-interactive-options

greggyb commented 1 month ago

FSAC dropped a patch release with this change that is live now. I'll raise a PR for this.

There are three parameters that will eventually become two:

  1. FSIExtraParameters: already existed. Still usable exactly as it was. Will go away.
  2. FSIExtraInteractiveParameters: new; currently unused by FSAC, but available to editor plugins and will eventually be used by FSAC; should be used for parameters that are only used by the interactive interpreter
  3. FSIExtraSharedParameters: new; supersedes FSIExtraParameters; should only use parameters shared with the compiler; passed by FSAC into the compiler for code analysis properties

Users should use (FSIExtraParameters) xor (any combination of FSIExtraInteractiveParameters and FSIExtraSharedParameters). FSAC will yell at you (but run just fine) if you use the old and new parameters together.

I intend to

  1. remove the default for FSIParameters; even as an array of an empty string, this is passed as a Some "" through the FSAC config
  2. Add the two new parameters
  3. Set a default of ['--readline-'] for FSIExtraInteractiveParameters, to address the underlying bug in dotnet fsi linked above
  4. Document these changes in the readme.
cannorin commented 1 month ago

@greggyb Thank you!