Skip to content

Port completion popup window from upstream getline.cs - #2

Open
vbfox wants to merge 3 commits into
masterfrom
claude/port-completion-popup
Open

Port completion popup window from upstream getline.cs#2
vbfox wants to merge 3 commits into
masterfrom
claude/port-completion-popup

Conversation

@vbfox

@vbfox vbfox commented Jul 20, 2026

Copy link
Copy Markdown
Owner

Summary

Ports the last cluster of upstream changes to mcs/tools/csharp/getline.cs (Jan–Mar 2016), which predate this F# port and were never carried over:

  • a7a4958 — "Add support for completion popup windows" (the core feature)
  • 642b064 — fix the top_item scroll logic in the popup
  • f91bb07 — call HideCompletions() after auto-inserting a single match
  • aee167f — demo update showing off the new heuristics mode

What changed

  • CompletionState now renders an actual popup window below the cursor instead of dumping every match as flat text.
  • GetLineSettings.HeuristicsMode (Some "csharp") triggers completion automatically (e.g. on .) without waiting for Tab.
  • Up/Down (and Ctrl+P/Ctrl+N) navigate the popup instead of history while it's open; Escape closes it; most other edits close it automatically — Handler now carries a ResetCompletion flag, mirroring upstream's per-key resetCompletion parameter.
  • CmdDone (Enter) inserts the highlighted completion instead of submitting the line when the popup is open.
  • The two follow-up bugfixes (642b064, f91bb07) are folded directly into the initial implementation rather than ported bug-for-bug.
  • TestApp's demo now sets HeuristicsMode and shows a popup automatically when typing ., per aee167f.

Bugs found & fixed along the way

While rewriting the common-prefix matching logic that this feature reuses, found two bugs in the existing F# port (not present upstream):

  • A missing loop increment that could hang indefinitely when two or more completions shared a prefix character.
  • A missing short-circuit that could index a shorter completion string out of bounds (upstream avoids this via goto mismatch, which the original transcription into F# didn't replicate).

Both are covered by regression tests (see below) — I confirmed each actually reproduces the original bug (hang / wrong result) by temporarily reverting the fix and re-running the suite.

Tests

Upstream getline.cs has no test directory of its own, so there was nothing to port there. Added a BlackFox.FsGetLine.Tests project (Expecto, matching the convention in BlackFox/FoxSharp) covering what's actually testable without a real interactive console — Console.ReadKey/cursor APIs make the main edit loop impractical to unit test:

  • CompletionState's popup scrolling math, including a regression test that fails under the pre-642b064 formula
  • The common-prefix matching fix
  • The C# dot-heuristic (numeric literals vs. identifiers)

Wired into the FAKE build as a RunTests task that CI now depends on (previously a DoNothing placeholder in the old build.fsx).

Test plan

  • dotnet build on the full solution
  • Full clean-room ./build.sh CI (restore → build → run tests → pack) succeeds locally, all 23 tests pass
  • Reverted each of the two found bugs individually and confirmed the corresponding test fails (one as an assertion failure, the other as an actual infinite loop caught by timeout)
  • Drove TestApp under a real pty: Tab-triggered popup, arrow-key navigation, Escape-to-close, and the heuristic auto-trigger on . all work end to end with no exceptions
  • GitHub Actions CI green on this PR (Linux + Windows)

Generated by Claude Code

Ports mono/mono commit a7a4958 ("Add support for completion popup
windows") plus its two follow-up fixes (642b064, f91bb07), which never
made it into this F# port:

- CompletionState renders an actual popup window below the cursor
  instead of dumping all matches as flat text.
- GetLineSettings.HeuristicsMode ("csharp") triggers completion
  automatically (e.g. on '.') without waiting for Tab.
- Up/Down (and Ctrl+P/Ctrl+N) navigate the popup instead of history
  while it's open; Escape closes it; most other edits close it
  automatically (Handler now carries a ResetCompletion flag, mirroring
  upstream's per-key `resetCompletion` parameter).
- CmdDone inserts the highlighted completion instead of submitting the
  line when the popup is open.

Folded directly into the initial implementation rather than ported
bug-for-bug:
- 642b064's top_item scroll fix (`selected - top >= height`, not
  `selected + top >= height`).
- f91bb07's HideCompletions() after auto-inserting a single match.

Also found and fixed two bugs while rewriting the common-prefix
matching logic that this feature reuses (present in the existing F#
port, not upstream): a missing loop increment that could hang
indefinitely comparing multiple completions sharing a prefix, and a
missing short-circuit that could index a shorter string out of bounds
(upstream avoids this via `goto`, which the original transcription
didn't replicate).

Adds a BlackFox.FsGetLine.Tests project (Expecto, matching FoxSharp's
convention) covering the pure logic that's actually testable without a
real console: CompletionState's popup scrolling math (including a
regression test for the 642b064 fix), the common-prefix matching fix,
and the C# dot-heuristic. Wired into the FAKE build as a RunTests task
that CI now depends on (previously a `DoNothing` placeholder).

Verified interactively by driving TestApp under a pty: Tab/arrow-key
popup navigation, Escape-to-close, and the heuristic auto-trigger on
'.' all work end to end with no exceptions.
@vbfox
vbfox force-pushed the claude/port-completion-popup branch from 1058a50 to d4da07c Compare July 26, 2026 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants