Stop FSI from mutating script arguments after -- separator#19926
Open
T-Gro wants to merge 4 commits into
Open
Conversation
RED phase: pin down the correct behaviour of fsi.CommandLineArgs when user arguments follow the -- separator. The abbreviated flags -d, -r, -I are currently colon-joined with their next token (e.g. '-- -d 5' becomes '-d:5' instead of staying as two tokens), which these tests assert against. Theory rows fail on current main; the baseline [<Fact>] passes and locks in the current treatment of non-abbreviated post-'--' args so the GREEN fix cannot regress them. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
#10819) Split argv at the first `--` and only post-process the compiler-args prefix; the suffix (including the `--` separator itself) reaches downstream handlers byte-for-byte. Keeping `--` in the suffix preserves two paths: * with a script preceding `--`, the script's OptionGeneral IsScript handler captures the suffix as script args (matching pre-fix behaviour for fsi.CommandLineArgs); * with no script (e.g. `dotnet fsi -- -d 5`), the `--` token reaches ParseCompilerOptions and fires its OptionRest recordExplicitArg handler so `-d` and `5` are recorded as explicit args instead of being parsed as compiler options. Updates the regression test theory to expect `--` in the captured tail and adds a new no-script Fact that locks the OptionRest path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fantomas-applied formatting in src/Compiler/Interactive/fsi.fs (whitespace and pattern-match line collapse only; no behavioural change) and a single new bullet under ### Fixed in docs/release-notes/.FSharp.Compiler.Service/11.0.100.md referencing Issue #10819. Validation: focused regression filters (FsiCommandLineArgsTests, CommandLineArgs, FsiCliTests) green under -c Release on both net10.0 and net48. SurfaceAreaTest passes on net10.0 with no baseline update (net48 has zero tests for that filter - platform-conditional). Full FSharp.Compiler.ComponentTests and FSharp.Compiler.Service.Tests suites were run under -c Release as the documented alternative to './build.sh -c Release --testcoreclr'; the only failures observed were two well-known flaky ScriptOptionsTests nuget-resolution tests on net48 (unrelated to argument handling; confirmed flaky in isolation; net10.0 runs all 4364 tests green). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
❗ Release notes requiredYou can open this PR in browser to add release notes: open in github.dev
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #10819
fsi.CommandLineArgswas colon-joining abbreviated flags (-d,-r,-I)with their next token when they appeared after
--. NowPostProcessCompilerArgssplits at the first
--and only post-processes the compiler-args prefix.