-
-
Notifications
You must be signed in to change notification settings - Fork 284
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
FSI send ignore comments #1859 #2049
base: main
Are you sure you want to change the base?
Conversation
Add functionality to skip comments and warn if a line is too long for the interactive session in `src/Components/Fsi.fs`. - **Skip Comments**: Modify the `sendSelection` function to filter out lines that start with `//` before sending the text to the terminal. - **Warn for Long Lines**: Add a `detectTerminalSize` function to get the terminal size and modify the `send` function to check if the number of lines exceeds the terminal rows. If it does, show a warning message to the user. --- For more details, open the [Copilot Workspace session](https://copilot-workspace.githubnext.com/jkone27/ionide-vscode-fsharp?shareId=XXXX-XXXX-XXXX-XXXX).
Revert changes in RELEASE_NOTES.md to match master
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there any interactions with line numbers and fsi in ionide? I.e. if we strip comments, and then repl will emit a diagnostic, is it navigable back to editor? In this case, it needs to somehow be compensated.
let private commentRegex = | ||
Regex(@"\/{2,3}.+\S+", RegexOptions.ECMAScript) | ||
|
||
let private multilineCommentRegex = | ||
Regex(@"\(\*[\s\S]*?\*\)", RegexOptions.ECMAScript) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should these be compiled?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vzarytovskii these are compiled to JS regexes, and I don't think there's a 'compiled' equivalent on that runtime.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see
@vzarytovskii you make a good point about the compensation - right now the closest thing we have to an 'in-editor' link is the fact that some terminals will look for things that look like a file path/line/column and will auto-link to it. VSCode does do this, so the line numbers might be a bit off. This is something that would be fixed for real if Ionide managed FSI sessions interactively via the FCS APIs instead of by 'driving' |
The "cheapest" solution might be is to not remove comments, but strip them to just |
@@ -494,13 +496,27 @@ module Fsi = | |||
return terminal | |||
} | |||
|
|||
let private commentRegex = | |||
Regex(@"\/{2,3}.+\S+", RegexOptions.ECMAScript) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This won't match the following:
//a
<any number of spaces here>//a<any number of spaces here>
First is quite short, so I suppose it's not an issue, second one might cause some issues?
This will match the following:
let foo = "// asdasdasdasdasdasd"
Which probably is not desired.
Second regex probably has some similar issues.
fixes issue with comments too long , partial fix for general issue 1859, to avoid
FS0193: operation not supported
but comments can be safely ignored and skip FSI send, even if very long