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

dotnet fsi crashes with long lines #82

Closed
greggyb opened this issue May 22, 2024 · 9 comments
Closed

dotnet fsi crashes with long lines #82

greggyb opened this issue May 22, 2024 · 9 comments
Assignees
Labels
bug Something isn't working upstream

Comments

@greggyb
Copy link
Contributor

greggyb commented May 22, 2024

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 greggyb added the bug Something isn't working label May 22, 2024
@greggyb
Copy link
Contributor Author

greggyb commented May 22, 2024

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
Copy link
Contributor Author

greggyb commented May 22, 2024

Additional complications here: ionide/FsAutoComplete#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
Copy link
Contributor Author

greggyb commented May 22, 2024

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
Copy link
Contributor Author

greggyb commented May 23, 2024

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

PR: ionide/FsAutoComplete#1299

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

  • FSIExtraInteractiveParameters: for parameters that are only used by the interactive interpreter
  • FSIExtraSharedParameters: for parameters used both by the interpreter and the compiler

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
Copy link
Contributor Author

greggyb commented May 23, 2024

@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
Copy link
Member

@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
Copy link
Contributor Author

greggyb commented May 23, 2024

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
Copy link
Contributor Author

greggyb commented May 23, 2024

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.

greggyb added a commit to greggyb/Ionide-vim that referenced this issue May 23, 2024
greggyb added a commit to greggyb/Ionide-vim that referenced this issue May 23, 2024
Removing the default for `fsi_extra_parameters` causes an error when
concatenating command line args for the interpreter. Wrap in a
conditional check for existence. Add a warning message to migrate to the
new parameters

Implements ionide#82
@cannorin
Copy link
Member

@greggyb Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working upstream
Projects
None yet
Development

No branches or pull requests

2 participants