Fix the autosuggestions toggle and remove the dead parallel subsystem#365
Merged
Conversation
Autosuggestions rendered and accepted regardless of the
display.autosuggestions toggle. The render gate is
controller->autosuggestions_enabled in dc_handle_redraw_needed; it was set
true at controller init based only on whether the layer allocated, and the
one function that could change it (display_controller_set_autosuggestions_enabled)
had no callers, so it stayed permanently true.
Drive the feature from config.display_autosuggestions at its three stages:
- generation: update_autosuggestion returns early when off, leaving
ctx->current_suggestion empty
- render: refresh_display syncs controller->autosuggestions_enabled from
the config field before each push, so the dc_handle_redraw_needed gate
and display_controller_get_autosuggestion short-circuit when off
- acceptance: has_autosuggestion returns false when off, so the
RIGHT / END / Ctrl-E / Ctrl-F / forward-word / partial-accept handlers
decline to splice a suggestion
config set display.autosuggestions, display lle autosuggestions on|off, and
a lushrc display.autosuggestions value all resolve to this one field.
display_integration.c carried a second autosuggestions layer
(global_autosuggestions_layer) with its own init/cleanup/update/clear entry
points, separate from the display controller's autosuggestions layer. It was
never rendered: the ghost is drawn in dc_handle_redraw_needed from
controller->autosuggestions_layer, and display_integration_update_autosuggestions
had no callers. The controller-creation fallback inside
display_integration_init_autosuggestions was redundant -- the main
display_integration_init (and the re-enable path) create the controller and
set layered_display_enabled directly.
Spec 10 builds the autosuggestion system around the display controller and
spec 08 gates it on a config enable flag, so the controller's layer is the
single intended path. Remove the parallel subsystem: the
global_autosuggestions_layer / autosuggestions_layer_initialized statics,
display_integration_{init,cleanup,update,clear}_autosuggestions and their
declarations, the now-unused autosuggestions_layer.h include, and the init
and cleanup call sites.
The shell starts and renders, autosuggestions still resolve through the
controller layer, and 156/156 tests pass.
Asserts that display.autosuggestions flips and reads back through every surface -- the lush-mode default, config set display.autosuggestions true|false, and the display lle autosuggestions on|off builtin -- via the display lle autosuggestions status, which reports config.display_autosuggestions: the single field the generation, acceptance, and render gates consult. Each check uses a fresh interactive lush (one setup command + one status read). A single interaction per spawn is deterministic, whereas the LLE's render of rapid command sequences in one session races output capture. The visual ghost and Right-arrow acceptance are not asserted -- suggestion generation and the async ghost draw are timing-dependent under a PTY -- so the test guards the gate wiring without racing the renderer.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
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.
The bug
Autosuggestions rendered and were accepted regardless of
display.autosuggestions. The render gate iscontroller->autosuggestions_enabledin
dc_handle_redraw_needed; it was settrueat controller init based onlyon whether the layer allocated, and the one function that could change it
(
display_controller_set_autosuggestions_enabled) had zero callers — so itwas permanently on. Acceptance and generation were likewise ungated.
The fix (
087ced047)Drive the feature from
config.display_autosuggestionsat all three tracedstages — generation (
update_autosuggestion), render (synccontroller->autosuggestions_enabledfrom config inrefresh_display), andacceptance (
has_autosuggestion).config set display.autosuggestions,display lle autosuggestions on|off, and a lushrc value all resolve to this onefield. Verified by instrumenting the render gate (reads
0when off).One true path (
452de352f)display_integration.ccarried a second, never-rendered autosuggestionslayer (
global_autosuggestions_layer+init/cleanup/update/clear). Therenderer reads
controller->autosuggestions_layer; the paralleldisplay_integration_update_autosuggestionshad no callers, and itscontroller-creation fallback was redundant. Spec 10 builds the suggestion
system around the display controller and spec 08 gates it on a config flag, so
the controller's layer is the single intended path. Removed the parallel
subsystem (-353 lines).
Test (
042fc2851)A PTY regression guard asserts the toggle flips and reads back through the
default,
config set, and thedisplay llebuiltin, via thedisplay lle autosuggestionsstatus (which reports the exact gate field). Eachcheck is a fresh single interaction (deterministic); the visual ghost/accept
are not asserted because suggestion generation and the async draw are
timing-dependent under a PTY. Reliable across 12/12 runs.
Clean under -Werror; 157/157 tests.