Skip to content

Making Reedline aware of the status of completors#1123

Merged
fdncred merged 10 commits into
nushell:mainfrom
philocalyst:aware
Jul 20, 2026
Merged

Making Reedline aware of the status of completors#1123
fdncred merged 10 commits into
nushell:mainfrom
philocalyst:aware

Conversation

@philocalyst

@philocalyst philocalyst commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

Summary

Before

  • Before, with recently added changes, reedline lied totally unaware the state of completion, as it was originally entirely tied to the its own lifecycle. With recent changes, it now has its own lifecycle, which left reedline totally in the dark, leading to poor behavior in many cases (largely in renderin).

After

  • Reedline is now aware of the state a completor is in, allowing it to make rendering decisions that reflect that state.
  • Flashing is now removed, which was a symptom of reedline updating under new context. It starts with that context now, and is able to respond effectively.

Additional notes

  • I definitely took some liberties here in refactoring some of the impacted functions, in an effort to not let the codebase slip when adding these new conditions. The goal was to add these features will making performance better and I think I accomplished that.
  • There's a corresponding PR in Nushell coming soon.
  • Added a regression test for flashing.
  • Biggest refactor was in create_string -- where I got both paths down to one/two allocations

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Reedline’s completion pipeline so the engine and menus can distinguish between pending, stale, and fresh completion results, enabling menus to avoid rendering “empty” states while background completion work is still in flight (reducing UI flashing/jumping).

Changes:

  • Introduces CompletionResult/CompletionStatus and updates the Completer trait to report completion lifecycle state.
  • Updates the engine loop to poll completer background work and refresh active menus when results become ready.
  • Refactors IDE/columnar menu completion rendering to cache suggestion display metrics and to suppress empty-state rendering while awaiting results.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
src/menu/menu_functions.rs Adds CompletionDisplay to cache suggestions and derived display metrics for menus.
src/menu/list_menu.rs Updates completion call site to the new CompletionResult API.
src/menu/ide_menu.rs Adopts CompletionDisplay and adds pending-aware rendering to avoid “no records” flashing.
src/menu/description_menu.rs Updates completion call site to the new CompletionResult API.
src/menu/columnar_menu.rs Adopts CompletionDisplay, adds pending-aware rendering, and refactors suggestion string formatting.
src/lib.rs Re-exports new completion lifecycle types (CompletionResult, CompletionStatus, Suggestions).
src/engine.rs Polls completion status each loop iteration and refreshes active menus when ready.
src/completion/mod.rs Re-exports new completion lifecycle types from base.
src/completion/history.rs Updates history completer implementation to return CompletionResult.
src/completion/default.rs Updates default completer implementation and docs to return/use CompletionResult.
src/completion/base.rs Adds Suggestions (Arc-backed), CompletionResult, CompletionStatus, and updates Completer trait signatures.
examples/persistent_menus.rs Updates example completer implementation to return CompletionResult.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread src/menu/columnar_menu.rs Outdated
Comment thread src/completion/base.rs
Comment thread src/completion/base.rs Outdated
Comment thread src/menu/description_menu.rs Outdated
Comment thread src/menu/list_menu.rs Outdated
@kronberger-droid

kronberger-droid commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

Nice!
I hope I will get some time this week to take a closer look.

@philocalyst

Copy link
Copy Markdown
Contributor Author

Responded to comments and made more efficent with stronger use of Arced suggestions !

@fdncred

fdncred commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

ok, let's continue to push forward @philocalyst to get these completions in good order. appreciate your help.

@fdncred
fdncred merged commit 093783d into nushell:main Jul 20, 2026
7 checks passed
fdncred pushed a commit to nushell/nushell that referenced this pull request Jul 20, 2026
- This syncs against the changes made in reedlines
[changes](nushell/reedline#1123).
- This consolidates signatures and makes the code more readable and
obvious.
- Also removes things like CompleterExt to pull more of this new tech
into the "core" of nu.
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.

4 participants