Files
ARCADIA-Studio/harness/experiments/text-workspace-followup-review-20260913/focus-separable-integration-review.md
T
sorokin.oleg.edu 2df7f580aa Make the Text workspace compact and preserve editing context
Replace the oversized header and reference rail with stable editor slots and a retained supporting drawer. Bind diagnostic, result and menu actions to their original publication and visit; preserve effective selections on navigation and dismiss Actions, Find and the drawer one surface per Escape.

Retain exact Windows source captures, eight final gates (871 desktop passes), independent reviews, native selection/rename/recovery trials and the original failures. All 526 tested inputs and staged evidence were byte-checked before commit. Combined preview integration and packaging remain with issue 27; Linux remains paused.
2026-09-12 22:00:36 -07:00

75 lines
8.7 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# Separable Find focus correction for #27 integration
Static review: the production correction is self-contained on CBC `cbc49a7802fb1bba0dd284fd904c700a0703ae70`. It needs only two local changes in `crates/oa-desktop/src/app/source_symbols.rs`; no new App state, enum, Text workspace, diagnostics publication, drawer, canonical departure-selection helper, or dependency change is required. No source was edited or built in this review. Peer commit `e124029` is not present in the local object database, so compatibility with that actual union remains an integration check for its owner, not a verified claim here.
## Exact production scope
In `restore_source_editor_focus`, retain both existing admission checks and `queue_source_reveal(cursor, false)`, then add:
```rust
// This ticket restores focus to the live editor, not a saved cursor.
// Iced's Cursor omits Word/Line selection kind; moving back to that
// lossy value would collapse an actual word/line selection.
self.source_jump = None;
```
In `source_viewport`, leave every ticket/admission check, ticket consumption, `source_jump.take()`, exact-cursor check, `move_to`, and optional `SelectLine` unchanged. Move the existing `guarded_focus` expression outside the `if let Some(cursor)` branch, replacing the final `Task::none()`:
```rust
if let Some(cursor) = self.source_jump.take() {
if !nav::exact_cursor(self.source_workspace.text(), cursor) {
return Task::none();
}
self.source_editor.move_to(cursor);
if self.source_select_line && cursor.selection.is_none() {
self.source_editor.perform(text_editor::Action::SelectLine);
}
}
// A valid ticket with no jump is focus-only. All intentional navigation
// still takes the cursor branch above, including explicit line selection.
nav::interaction::guarded_focus(
self.source_editor_id(),
self.source_symbols.raw_interaction.clone(),
raw_interaction,
)
```
Do not transplant the surrounding working-file diff. It additionally contains #13 menu exclusion, retained reference display/query lifetime, drawer commands/scroll state, future diagram adapter, and canonical selection capture for navigation departure. Those are separate changes with separate dependencies and evidence.
## Existing CBC protection remains operative
CBC already has `restore_source_editor_focus`, `queue_source_reveal`, `source_viewport`, the guarded Find Close envelope, and their call sites. Its actual editor sensor is keyed by `(active_path, reveal_ticket)` and emits captured `SourceViewport(ticket)` on show/resize. Creating a fresh focus-only reveal still changes this key and schedules the existing path; no new sensor or callback is required.
Before any mutation or focus operation, `source_viewport` checks ticket, interaction ID, raw interaction clock, workspace/revision snapshot, active path, and current navigation availability. The reveal is consumed once. An intentional jump is still checked against current source before movement; invalid destinations return without focus. `SourceViewport` remains exempt from App's generic interaction invalidation so a valid ticket can survive its own delivery. Other interactions invalidate the ticket and editor ID through existing CBC logic.
The returned existing `guarded_focus` is not an unconditional deferred focus. It rechecks the raw clock while traversing, requires exactly one matching mounted editor and no foreign focused widget, then synchronously chains the focus pass with another clock check. The editor ID includes workspace session, active path and interaction. Thus both stale message delivery and later input between Task creation and operation are guarded. Clearing `source_jump` does not discard this authority. `source_select_line` is already set false by `queue_source_reveal(cursor, false)`.
The intentional behavioral extension is narrowly defined: a current admitted reveal with no jump now restores focus. Existing real navigation continues through the cursor branch. Do not replace this with direct `widget::focus`, call `move_to` during focus restoration, clone/reset Content, or canonicalize Word/Line selection at Find Close. Those alternatives lose live selection mode or ticket protection.
## Mounted regression handoff
Current fixture: `crates/oa-desktop/src/app/text_workspace/focus_selection_tests.rs`. Its four tests are:
1. `word_and_line_selection_survive_find_close_and_escape_then_replace_exact_bytes`: actual double/triple-click selection, Close button or captured Escape bridge, guarded viewport Task, selected text, exact replacement, unchanged accepted Document/history; covers four word/line × Close/Escape cases.
2. `mounted_direct_typing_control_and_old_cursor_roundtrip_discriminate_selection_loss`: valid direct typing control and explicit old lossy roundtrip discrimination.
3. `stale_focus_ticket_and_later_raw_input_cannot_steal_focus_or_change_selected_bytes`: both stale viewport delivery and a previously created operation executed after later raw input/reopened Find.
4. `explicit_cursor_navigation_still_moves_and_selects_requested_line`: intentional cursor navigation remains effective.
For a CBC-based separable test commit, place the fixture in a source-navigation/source-symbols test module rather than importing all `app::text_workspace`. Register under the same debug/test cfg convention used by the existing App tests. Resolve its current `use super::*` explicitly to App/Message, `crate::source_navigation as nav`, Point, Task and widget as required by the chosen module; retain existing explicit imports. CBC already exports `views::source_tests::FontLayout` and provides Document/Workspace and the SourceNavigation/SourceFindClose/SourceViewport messages used here. The setup call `app.publish_source_diagnostics(...)` is #13-only and unnecessary for these focus assertions; omit it for CBC rather than introduce that production API. No other TextWorkspace event/state is referenced in the fixture body.
Keep `Column::from_vec` with the permanent zero-height Find slot: `Column::push` elides that child and invalidates the fixture. Keep the actual hit-tested initial caret transfer only before Word/Line selection; never copy a cursor back into App after selecting. App and CPU mounted Content are different renderer-typed instances, so this is a controller/content plus widget-operation evaluator, not a full production GPU/sensor/scroll measurement. Its manually delivered `SourceViewport` tests the controller guard; the owner should retain existing subscription/sensor checks and exercise the peer's actual editor wrapper after integration. The focused port's module-path command must match its final registration; do not reuse a filter that discovers zero tests. Confirm exactly four named tests execute, then run the owner's combined gates.
## Evidence and remaining union work
The portable `harness/experiments/text-find-selection-20260913` archive retains original inputs/logs and corrected source. Valid `focus-red-2` used the fixed fixture with old production: direct typing and explicit navigation passed while two selection tests failed. `focus-green-2` actually recompiled corrected production and passed all four, ending 2026-09-13T03:17:54Z; stdout SHA256 `6f19f24e2fa37db4fc95c9b2d11b65e420299c6c6f90e4657e7e8e328b326fa0`. Red1 failed fixture construction and green1 reused stale compiler cache: neither establishes the correction. This earlier valid red/green predates canonical departure-selection changes and therefore supports separability of the focus behavior. It does not establish an unbuilt CBC-only patch or the peer union.
Owner #27 must reconcile the two function-body changes with its own source, preserve every peer admission predicate and menu/modal ownership check, keep the existing guarded operation and sensor ticket identity, and port/run the four tests in that union. If its editor wrapper or Find Escape dispatch differs, test that actual route; do not replace it with #13's later drawer Escape correction. Back/file canonical selection capture is a different patch; diagram usages/chooser and reference display retention remain separately owned integration dependencies. None is necessary to compile this focus-only production patch on CBC, and none is delivered by it.
Bindings at review:
- CBC raw `source_symbols.rs`: `77cb97ec909bcaf3d952e0c6673171fd81e7ca31568248994c8c6b150e2c7f4e`.
- Working `source_symbols.rs`: `d81914a553114d85452d973a1edb175843d27154b3f9b9fc4cbc5049d423abe4`.
- Mounted fixture: `db7cc3cfbf5f8dce83cf6134e339cc0ba57c0679c47b6268f0e470d09c0e6cc8`.
- Existing `interaction.rs`: `c6efa6105b8ba46655e27a89b9fa59e6fc01a4c6f331f886e035af1e70cbf8a9`; no diff from CBC.
- Red/green archive manifest: `4580d8cbc2f0347f1ebc1448728af6ef8885f6ebbd4c2d2dbe0c41eac97fb283`.