Fix two bugs in tab-completion common-prefix matching - #3
Merged
Conversation
Adds a BlackFox.FsGetLine.Tests project, matching FoxSharp's convention, wired into the solution and the FAKE build as a RunTests task that CI now depends on (previously a DoNothing placeholder). No tests yet; regression tests for specific bugs land in follow-up commits. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
cmdTabOrComplete's inner scan over the other candidates never advanced its index (`i`), so once two or more completions shared a character at some position, the loop condition (`i < ncompletions && not mismatch`) kept re-checking the same index forever instead of eventually setting `mismatch` or exhausting the candidates. Any tab-completion with two or more matches sharing so much as their first character would hang. Extracted the logic into a top-level `commonPrefixLength` function so it can be unit tested, and added the missing `i <- i + 1`. Confirmed the pre-fix code hangs on the regression input by running it in a timeout-guarded task. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
vbfox
force-pushed
the
claude/port-pr2-bugfixes
branch
from
July 26, 2026 21:44
dc0b1ae to
6f7cbd3
Compare
Owner
Author
|
Rewrote part of the logic over LLM output to be more FSharpy and have some FsCheck tests |
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
1 participant
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.
Summary
While reviewing PR #2 (which ports the upstream completion popup feature), that PR's description called out two pre-existing bugs in the existing F# port's tab-completion common-prefix matching (
cmdTabOrCompleteinFsGetLine.fs), unrelated to the popup feature itself. This PR extracts and fixes just those two bugs againstmaster, with regression tests, so they land independently of the larger popup port.<instead of<=, and two independentifs instead ofelif) that could index a shorter completion string out of bounds once the scan reached exactly its length.Note on the second bug
PR #2's own fix for this only added the
elifshort-circuit but kept the<comparison. I verified empirically that this is still incomplete: it crashes when the longer completion is listed first (e.g.["abc"; "ab"]), because PR #2's regression test happened to list the shorter completion first, which never exercises the crash path. The same< p(not<=) exists in upstream mono's C#getline.cstoo, so this is a latent bug there as well, not something introduced by the F# port. This PR fixes it correctly with<=.Commits
BlackFox.FsGetLine.Testsproject, wired into the solution and the FAKE build (RunTeststask,CI/Defaultdepend on it). Same setup as PR Port completion popup window from upstream getline.cs #2, without the popup feature or its tests.commonPrefixLengthfunction and adds the missingi <- i + 1.<=and short-circuits it withelif.Test plan
dotnet build FsGetLine.sln -c Releasesucceedsdotnet testpasses (2/2)IndexOutOfRangeException) by running the pre-fix logic in a timeout-guarded script before applying each fixGenerated by Claude Code