diff --git a/internal/app/app.go b/internal/app/app.go index fdb6f7a..a1ec161 100644 --- a/internal/app/app.go +++ b/internal/app/app.go @@ -137,6 +137,7 @@ type InputListener interface { const ( caretDebounce = 150 * time.Millisecond terminalMutationDebounce = 50 * time.Millisecond + terminalRepaintDebounce = 500 * time.Millisecond objectReviewCaptureTimeout = 2 * time.Second flatReviewCaptureTimeout = 10 * time.Second documentCaptureTimeout = 15 * time.Second diff --git a/internal/app/current_line.go b/internal/app/current_line.go index 335e5af..e850148 100644 --- a/internal/app/current_line.go +++ b/internal/app/current_line.go @@ -19,6 +19,7 @@ func (handler *eventReviewHandler) current_line(command input.Command) { } handler.state.pending.terminalFocus.clear() handler.state.pending.terminalDeletion.clear() + handler.state.pending.terminalRepaint.clear() style := review.FlatSpeak if command == input.SpellCurrentLine { style = review.FlatSpell diff --git a/internal/app/event_accessibility.go b/internal/app/event_accessibility.go index 175b557..0307f48 100644 --- a/internal/app/event_accessibility.go +++ b/internal/app/event_accessibility.go @@ -254,15 +254,29 @@ func (handler *accessibilityEventHandler) handle(event atspi.Event) { if autocompleteConsumed { return } + replacedDeletion := false if state.pending.terminalDeletion.event.Type != "" && event.Type != atspi.CaretEvent { - isReplacement := event.Type == atspi.TextInsertedEvent && + pendingDeletion := state.pending.terminalDeletion.event + // A user edit at the caret is confirmation speech, not repaint + // churn: never coalesce it away or drop it behind a following + // deletion. It flushes like any other pending deletion. + userDeletion := state.pending.terminalDeletion.intent == input.DeleteIntent + sameTerminal := event.Source.Role == "terminal" && event.Source.Path != "" && - event.Source.Path == state.pending.terminalDeletion.event.Source.Path && - event.Detail1 == state.pending.terminalDeletion.event.Detail1 + event.Source.Path == pendingDeletion.Source.Path && + event.Source.BusName == pendingDeletion.Source.BusName + isReplacement := event.Type == atspi.TextInsertedEvent && sameTerminal && !userDeletion && + terminalReplacementOverlap(pendingDeletion, event) if isReplacement { + replacedDeletion = true state.pending.terminalDeletion.clear() logger.Debug("coalesced terminal deletion with following insertion", "role", event.Source.Role) - } else { + } else if !(event.Type == atspi.TextDeletedEvent && sameTerminal && !userDeletion) { + // Another deletion from the same terminal is part of the same + // repaint burst. It is handled below: scroll shifts return + // early, anything else replaces the pending deletion. Either + // way the pending deletion must not flush early, or the burst + // speaks the text it is about to rewrite. flushTerminalDeletion() } } @@ -298,9 +312,47 @@ func (handler *accessibilityEventHandler) handle(event atspi.Event) { logger.Debug("skipped terminal scrollback deletion", "role", event.Source.Role) return } - state.pending.terminalDeletion.schedule(event, input.NoIntent, terminalMutationDebounce) + deletionIntent := input.NoIntent + if is_user_terminal_deletion(event, intent) { + deletionIntent = input.DeleteIntent + } + state.pending.terminalDeletion.schedule(event, deletionIntent, terminalMutationDebounce) return } + // Latest wins for in-place terminal repaints: hold the insertion + // briefly so a newer rewrite of the same region replaces it before + // either speaks. A rewrite of a different region, or appended output + // from the same terminal, first flushes the held repaint in order. + flushTerminalRepaint := func() { + if state.pending.terminalRepaint.event.Type == "" { + return + } + pending, _ := state.pending.terminalRepaint.take() + if is_trivial_terminal_repaint(pending) { + logger.Debug("dropped trivial terminal repaint fragment", "role", pending.Source.Role) + return + } + speak(pending) + } + if is_latest_wins_terminal_repaint(event, intent, replacedDeletion) { + if state.pending.terminalRepaint.event.Type != "" && + !terminalReplacementOverlap(state.pending.terminalRepaint.event, event) { + flushTerminalRepaint() + } + state.pending.terminalRepaint.schedule(event, input.NoIntent, terminalRepaintDebounce) + logger.Debug("queued latest-wins terminal repaint", "role", event.Source.Role) + if autocompleteRequest.Text != "" { + speech.Enqueue(autocompleteRequest) + } + return + } + if event.Type == atspi.TextInsertedEvent && event.Source.Role == "terminal" && + state.pending.terminalRepaint.event.Type != "" && + event.Source.Path != "" && + event.Source.Path == state.pending.terminalRepaint.event.Source.Path && + event.Source.BusName == state.pending.terminalRepaint.event.Source.BusName { + flushTerminalRepaint() + } speak(event) if autocompleteRequest.Text != "" { speech.Enqueue(autocompleteRequest) diff --git a/internal/app/event_caret.go b/internal/app/event_caret.go index a98fbf8..9910e8e 100644 --- a/internal/app/event_caret.go +++ b/internal/app/event_caret.go @@ -21,6 +21,22 @@ func (handler *accessibilityEventHandler) handleCaret(event atspi.Event, activit objectReview := handler.objectReview debounce := handler.debounce if event.Type == atspi.CaretEvent { + // A reader started while a terminal already has focus may never + // receive a focus-gain event for it. Caret movement caused by the + // user's own typing on the focused terminal is enough to anchor + // flat review; without this, review stays focusless until a + // workspace switch replays focus. Restricting this to typing and + // delete input keeps a background terminal's own output churn + // from claiming review focus. + if objectReview != nil && event.Source.Role == "terminal" && event.Source.Focused && + (intent == input.TypingIntent || intent == input.DeleteIntent) { + objectReview.SyncFocus(event.Source.ID()) + if !state.focusedID.Valid() { + state.owner.follow(event) + state.focusedID = event.Source.ID() + state.current = event.Source + } + } if event.Source.ReadOnlyCombo.Valid() && event.Source.ReadOnlyCombo == state.focusedID && state.current.ID() == state.focusedID && state.current.Role == "combo box" { // The combo's name/value event presents this selection, even when diff --git a/internal/app/event_caret_test.go b/internal/app/event_caret_test.go index 0425192..2d1196e 100644 --- a/internal/app/event_caret_test.go +++ b/internal/app/event_caret_test.go @@ -26,6 +26,7 @@ import ( "git.stormux.org/storm/cthulhu/internal/atspi" "git.stormux.org/storm/cthulhu/internal/input" + "git.stormux.org/storm/cthulhu/internal/review" ) func TestProcessEventsSpeaksChangedCaretLines(t *testing.T) { @@ -156,6 +157,38 @@ func TestReadOnlyComboCaretRequiresExactOwnedCombo(t *testing.T) { } } +func TestTerminalCaretSeedsReviewFocusWithoutFocusEvent(t *testing.T) { + // A reader started while the terminal already has focus may never see + // a focus-gain event. Caret movement from the focused terminal is the + // evidence flat review needs to anchor to it. + worker, err := review.NewWorker(&documentLineCapturer{calls: make(chan documentLineCaptureCall, 1)}, time.Second) + if err != nil { + t.Fatal(err) + } + state := &eventLoopState{} + defer state.pending.clear() + handler := accessibilityEventHandler{state: state, logger: slog.New(slog.DiscardHandler), objectReview: worker, debounce: time.Hour} + terminal := atspi.Snapshot{BusName: ":1.9", Path: "/terminal", Role: "terminal", Focused: true} + // Output churn without user typing must not claim review focus. + handler.handleCaret(atspi.NewEvent(atspi.CaretEvent, terminal), input.Activity{}, input.NoIntent) + if got := worker.Focus(); got.Valid() { + t.Fatalf("review focus seeded without typing = %+v", got) + } + handler.handleCaret(atspi.NewEvent(atspi.CaretEvent, terminal), input.Activity{}, input.TypingIntent) + if got := worker.Focus(); got != terminal.ID() { + t.Fatalf("review focus = %+v, want terminal %+v", got, terminal.ID()) + } + if state.focusedID != terminal.ID() || state.current.ID() != terminal.ID() { + t.Fatalf("focused state = %+v/%+v, want terminal", state.focusedID, state.current.ID()) + } + // An unfocused terminal caret must not steal review focus. + other := atspi.Snapshot{BusName: ":1.9", Path: "/other-terminal", Role: "terminal"} + handler.handleCaret(atspi.NewEvent(atspi.CaretEvent, other), input.Activity{}, input.TypingIntent) + if got := worker.Focus(); got != terminal.ID() { + t.Fatalf("review focus after unfocused caret = %+v, want %+v", got, terminal.ID()) + } +} + func TestProcessEventsSpeaksCaretCharacters(t *testing.T) { speech := &fakeSpeechQueue{spoken: make(chan string, 3)} adapter := &fakeAdapter{events: make(chan atspi.Event, 3)} diff --git a/internal/app/event_loop.go b/internal/app/event_loop.go index 84df1d6..240f310 100644 --- a/internal/app/event_loop.go +++ b/internal/app/event_loop.go @@ -333,6 +333,17 @@ func processEventsWithAttributes( } event, _ := state.pending.terminalDeletion.take() speak(event) + case <-state.pending.terminalRepaint.ready: + if focusGate.Locked() { + state.pending.clear() + continue + } + event, _ := state.pending.terminalRepaint.take() + if is_trivial_terminal_repaint(event) { + logger.Debug("dropped trivial terminal repaint fragment", "role", event.Source.Role) + continue + } + speak(event) case command := <-reviewRequests: switch command { case input.PresentCurrentLine, input.SpellCurrentLine, input.PhoneticCurrentLine: diff --git a/internal/app/event_review_results.go b/internal/app/event_review_results.go index 878c057..87c6524 100644 --- a/internal/app/event_review_results.go +++ b/internal/app/event_review_results.go @@ -41,6 +41,7 @@ func (handler *eventReviewHandler) whereAmI() { state := handler.state state.pending.terminalFocus.clear() state.pending.terminalDeletion.clear() + state.pending.terminalRepaint.clear() // Python's Where Am I is speech-only. Do not suppress the role merely // because ordinary focus presentation replaces it with a sound cue. reviewOptions := state.presentationOptions @@ -209,6 +210,7 @@ func (handler *eventReviewHandler) handle(result review.Result) { } state.pending.terminalFocus.clear() state.pending.terminalDeletion.clear() + state.pending.terminalRepaint.clear() presentFlatReviewResult(logger, speech, result) if reservedWineID(result.FlatFocus) && brailleOutput != nil && result.Err == nil && (result.Moved || result.FlatStep == 0 && result.FlatUnit != review.WordAbove && result.FlatUnit != review.WordBelow) { diff --git a/internal/app/pending_presentation.go b/internal/app/pending_presentation.go index ba69b71..8efdc21 100644 --- a/internal/app/pending_presentation.go +++ b/internal/app/pending_presentation.go @@ -48,6 +48,7 @@ type pendingPresentations struct { caret pendingPresentation terminalFocus pendingPresentation terminalDeletion pendingPresentation + terminalRepaint pendingPresentation browserButtonName pendingBrowserButtonName radioSound pendingRadioSound } @@ -56,6 +57,7 @@ func (pending *pendingPresentations) clear() { pending.caret.clear() pending.terminalFocus.clear() pending.terminalDeletion.clear() + pending.terminalRepaint.clear() pending.browserButtonName.clear() pending.radioSound.clear() } diff --git a/internal/app/terminal_events.go b/internal/app/terminal_events.go index 8987d9b..560e3cc 100644 --- a/internal/app/terminal_events.go +++ b/internal/app/terminal_events.go @@ -5,15 +5,73 @@ package app -import "git.stormux.org/storm/cthulhu/internal/atspi" +import ( + "strings" + "unicode/utf8" + + "git.stormux.org/storm/cthulhu/internal/atspi" + "git.stormux.org/storm/cthulhu/internal/input" +) func should_ignore_terminal_scrollback(event atspi.Event) bool { - // VTE can remove old complete lines at the start of its accessible text - // while appending output at a different offset. These are not edits to - // the prompt. This event pattern occurs on both X11 and Wayland. + // VTE can remove old lines at the start of its accessible text while + // appending output at a different offset. A deletion entirely before + // the caret line is not an edit to the prompt. VTE wrapped rows have + // no trailing newline, so do not require one. This event pattern + // occurs on both X11 and Wayland. return event.Type == atspi.TextDeletedEvent && event.Source.Role == "terminal" && event.Detail1 == 0 && event.Source.TextCaretKnown && event.Source.BrailleLineStart > 0 && - event.TextChangeEndsLine && event.TextChangeLength > 0 && + event.TextChangeLength > 0 && event.TextChangeLength <= event.Source.BrailleLineStart } + +// is_user_terminal_deletion identifies a deletion the user just caused at +// the caret on its own line: a Backspace or Delete edit. The deleted text +// is the user's confirmation that the edit took, so repaint coalescing +// must not swallow it. Program repaints start above the caret line, and +// deletions without a preceding delete key are program output churn. +func is_user_terminal_deletion(event atspi.Event, intent input.Intent) bool { + return intent == input.DeleteIntent && + event.Type == atspi.TextDeletedEvent && event.Source.Role == "terminal" && + event.Source.TextCaretKnown && + event.Detail1 >= event.Source.BrailleLineStart && + event.Detail1 <= event.Source.BrailleCaret +} + +// is_latest_wins_terminal_repaint identifies an in-place rewrite: an +// insertion that replaced a just-deleted range. Status regions update +// this way several times a second and only the newest state is worth +// hearing. Editing feedback on the caret line after a typing or delete +// key stays immediate; appended output has no preceding deletion and +// never qualifies. +func is_latest_wins_terminal_repaint(event atspi.Event, intent input.Intent, replacedDeletion bool) bool { + if !replacedDeletion || event.Type != atspi.TextInsertedEvent || event.Source.Role != "terminal" { + return false + } + if (intent == input.TypingIntent || intent == input.DeleteIntent) && + event.Source.TextCaretKnown && event.Detail1 >= event.Source.BrailleLineStart { + return false + } + return true +} + +// is_trivial_terminal_repaint identifies a held repaint whose payload is +// only the few cells that changed in a ticking status (timer digits such +// as "40"). Uttered alone it has no context; a fuller repaint of the same +// region replaces it before the flush if one follows. +func is_trivial_terminal_repaint(event atspi.Event) bool { + return event.Type == atspi.TextInsertedEvent && event.Source.Role == "terminal" && + utf8.RuneCountInString(strings.TrimSpace(event.Payload)) <= 2 +} + +// terminalReplacementOverlap reports whether an insertion rewrites any of +// the range a pending deletion removed. Offsets are not compared exactly: +// a scroll shift at the start of the buffer between the deletion and its +// replacement insertion moves the insertion's offset, and a repainted +// region is routinely rewritten at a neighboring offset. +func terminalReplacementOverlap(deletion, insertion atspi.Event) bool { + deletionEnd := deletion.Detail1 + max(deletion.TextChangeLength, 1) + insertionEnd := insertion.Detail1 + max(insertion.TextChangeLength, 1) + return insertion.Detail1 < deletionEnd && deletion.Detail1 < insertionEnd +} diff --git a/internal/app/terminal_events_test.go b/internal/app/terminal_events_test.go index 20df10e..43b0e07 100644 --- a/internal/app/terminal_events_test.go +++ b/internal/app/terminal_events_test.go @@ -25,6 +25,7 @@ import ( "time" "git.stormux.org/storm/cthulhu/internal/atspi" + "git.stormux.org/storm/cthulhu/internal/input" ) func TestProcessEventsTerminalScrollbackDeletionBothSessions(t *testing.T) { @@ -111,16 +112,22 @@ func TestTerminalScrollbackDeletionPreservesEditing(t *testing.T) { {"backspace", func(event *atspi.Event) { event.Payload = "x"; event.Detail1 = 109 }, false}, {"delete", func(event *atspi.Event) { event.Payload = "x"; event.Detail1 = 110 }, false}, {"delete word", func(event *atspi.Event) { event.Payload = "command"; event.Detail1 = 100 }, false}, - {"first character", func(event *atspi.Event) { + {"single character at start", func(event *atspi.Event) { + // The caret line starts at 100, so a deletion at offset 0 is a + // wrap/scroll shift, not an edit on the caret line. A real + // first-line backspace has a line start of 0 (see above). *event = atspi.NewEventWithPayload(atspi.TextDeletedEvent, terminal, "x") - }, false}, + }, true}, {"newline", func(event *atspi.Event) { event.Payload = "\n"; event.Detail1 = 100 }, false}, {"current first line", func(event *atspi.Event) { event.Source.BrailleLineStart = 0 }, false}, {"range reaches current line", func(event *atspi.Event) { event.Source.BrailleLineStart = 5 }, false}, {"unknown caret", func(event *atspi.Event) { event.Source.TextCaretKnown = false }, false}, {"partial line", func(event *atspi.Event) { + // VTE wrapped rows have no trailing newline, so a scrollback + // deletion entirely before the caret line is still a scroll, + // never an edit on the caret line. *event = atspi.NewEventWithPayload(atspi.TextDeletedEvent, terminal, "Old prompt") - }, false}, + }, true}, {"editor", func(event *atspi.Event) { event.Source.Role = "text" }, false}, {"insertion", func(event *atspi.Event) { event.Type = atspi.TextInsertedEvent }, false}, } { @@ -179,6 +186,257 @@ func TestProcessEventsCoalescesTerminalDeletionWithReplacementInsertion(t *testi } } +func TestProcessEventsCoalescesTerminalRepaintAcrossScrollShift(t *testing.T) { + speech := &fakeSpeechQueue{spoken: make(chan string, 3)} + adapter := &fakeAdapter{events: make(chan atspi.Event, 3)} + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + completed := make(chan struct{}) + go func() { + processEvents(ctx, slog.New(slog.DiscardHandler), speech, adapter, 0) + close(completed) + }() + + // A TUI repaints its live region: it deletes the old region, a + // one-character scroll shift at the start of the buffer moves every + // later offset back by one, then it inserts the new region. Only the + // insertion may speak. + terminal := atspi.NewSnapshot("/accessible/terminal", "", "terminal", "", "") + terminal.TextCaretKnown = true + terminal.BrailleLineStart = 190 + terminal.BrailleCaret = 195 + repaint := atspi.NewEventWithPayload(atspi.TextDeletedEvent, terminal, strings.Repeat("x", 50)) + repaint.Detail1 = 100 + shift := atspi.NewEventWithPayload(atspi.TextDeletedEvent, terminal, " ") + replacement := atspi.NewEventWithPayload(atspi.TextInsertedEvent, terminal, "new region text") + replacement.Detail1 = 99 + adapter.events <- repaint + adapter.events <- shift + adapter.events <- replacement + + select { + case text := <-speech.spoken: + if text != "new region text" { + t.Fatalf("spoken text = %q, want only the replacement insertion", text) + } + case <-time.After(time.Second): + t.Fatal("timed out waiting for replacement insertion") + } + select { + case text := <-speech.spoken: + t.Fatalf("terminal repaint deletion was presented as %q", text) + case <-time.After(2 * terminalMutationDebounce): + } + + cancel() + select { + case <-completed: + case <-time.After(time.Second): + t.Fatal("event loop did not stop") + } +} + +func repaintSequence(adapter *fakeAdapter, terminal atspi.Snapshot, detail1 int, texts ...string) { + for _, text := range texts { + deletion := atspi.NewEventWithPayload(atspi.TextDeletedEvent, terminal, strings.Repeat("x", len(text))) + deletion.Detail1 = detail1 + insertion := atspi.NewEventWithPayload(atspi.TextInsertedEvent, terminal, text) + insertion.Detail1 = detail1 + adapter.events <- deletion + adapter.events <- insertion + } +} + +func TestProcessEventsLatestTerminalRepaintWins(t *testing.T) { + speech := &fakeSpeechQueue{spoken: make(chan string, 4)} + adapter := &fakeAdapter{events: make(chan atspi.Event, 8)} + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + completed := make(chan struct{}) + go func() { + processEvents(ctx, slog.New(slog.DiscardHandler), speech, adapter, 0) + close(completed) + }() + + terminal := atspi.NewSnapshot("/accessible/terminal", "", "terminal", "", "") + terminal.TextCaretKnown = true + terminal.BrailleLineStart = 200 + terminal.BrailleCaret = 205 + repaintSequence(adapter, terminal, 100, "Thinking (1s)", "Thinking (2s)", "Thinking (3s)") + + select { + case text := <-speech.spoken: + if text != "Thinking (3s)" { + t.Fatalf("spoken text = %q, want only the latest repaint", text) + } + case <-time.After(3 * time.Second): + t.Fatal("latest repaint was not presented") + } + select { + case text := <-speech.spoken: + t.Fatalf("superseded repaint was presented as %q", text) + case <-time.After(terminalRepaintDebounce): + } + + cancel() + select { + case <-completed: + case <-time.After(time.Second): + t.Fatal("event loop did not stop") + } +} + +func TestProcessEventsTerminalRepaintsInDifferentRegionsBothSpeak(t *testing.T) { + speech := &fakeSpeechQueue{spoken: make(chan string, 4)} + adapter := &fakeAdapter{events: make(chan atspi.Event, 8)} + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + completed := make(chan struct{}) + go func() { + processEvents(ctx, slog.New(slog.DiscardHandler), speech, adapter, 0) + close(completed) + }() + + terminal := atspi.NewSnapshot("/accessible/terminal", "", "terminal", "", "") + terminal.TextCaretKnown = true + terminal.BrailleLineStart = 400 + terminal.BrailleCaret = 405 + repaintSequence(adapter, terminal, 100, "First region") + repaintSequence(adapter, terminal, 300, "Second region") + + for _, expected := range []string{"First region", "Second region"} { + select { + case text := <-speech.spoken: + if text != expected { + t.Fatalf("spoken text = %q, want %q", text, expected) + } + case <-time.After(3 * time.Second): + t.Fatalf("timed out waiting for %q", expected) + } + } + + cancel() + select { + case <-completed: + case <-time.After(time.Second): + t.Fatal("event loop did not stop") + } +} + +func TestProcessEventsDropsTrivialTerminalRepaintFragments(t *testing.T) { + speech := &fakeSpeechQueue{spoken: make(chan string, 4)} + adapter := &fakeAdapter{events: make(chan atspi.Event, 8)} + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + completed := make(chan struct{}) + go func() { + processEvents(ctx, slog.New(slog.DiscardHandler), speech, adapter, 0) + close(completed) + }() + + terminal := atspi.NewSnapshot("/accessible/terminal", "", "terminal", "", "") + terminal.TextCaretKnown = true + terminal.BrailleLineStart = 200 + terminal.BrailleCaret = 205 + repaintSequence(adapter, terminal, 100, "40") + select { + case text := <-speech.spoken: + t.Fatalf("trivial repaint fragment was presented as %q", text) + case <-time.After(2 * terminalRepaintDebounce): + } + repaintSequence(adapter, terminal, 100, "Thinking (4s)") + select { + case text := <-speech.spoken: + if text != "Thinking (4s)" { + t.Fatalf("spoken text = %q, want the full repaint", text) + } + case <-time.After(3 * time.Second): + t.Fatal("full repaint was not presented") + } + + cancel() + select { + case <-completed: + case <-time.After(time.Second): + t.Fatal("event loop did not stop") + } +} + +func TestProcessEventsPresentsUserTerminalDeletionDespiteRedraw(t *testing.T) { + speech := &fakeSpeechQueue{spoken: make(chan string, 3)} + adapter := &fakeAdapter{events: make(chan atspi.Event, 3)} + tracker := &input.IntentTracker{} + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + completed := make(chan struct{}) + go func() { + processEventsWithInput(ctx, slog.New(slog.DiscardHandler), speech, adapter, nil, nil, 0, nil, tracker, nil, nil) + close(completed) + }() + + // Backspace in a terminal input redraws the erased cell: the deleted + // character is replaced by a blank at the same offset. The deletion is + // the user's confirmation that the edit took and must still speak. + terminal := atspi.NewSnapshot("/accessible/terminal", "", "terminal", "", "") + terminal.TextCaretKnown = true + terminal.BrailleLineStart = 100 + terminal.BrailleCaret = 105 + tracker.Observe(input.NewKeyEvent(0, true, "BackSpace")) + deletion := atspi.NewEventWithPayload(atspi.TextDeletedEvent, terminal, "x") + deletion.Detail1 = 105 + redraw := atspi.NewEventWithPayload(atspi.TextInsertedEvent, terminal, " ") + redraw.Detail1 = 105 + adapter.events <- deletion + adapter.events <- redraw + + select { + case text := <-speech.spoken: + if text != "x" { + t.Fatalf("spoken text = %q, want the deleted character", text) + } + case <-time.After(time.Second): + t.Fatal("user deletion was not presented") + } + + cancel() + select { + case <-completed: + case <-time.After(time.Second): + t.Fatal("event loop did not stop") + } +} + +func TestUserTerminalDeletionClassification(t *testing.T) { + terminal := atspi.NewSnapshot("/accessible/terminal", "", "terminal", "", "") + terminal.TextCaretKnown = true + terminal.BrailleLineStart = 100 + terminal.BrailleCaret = 105 + for _, test := range []struct { + name string + intent input.Intent + modify func(*atspi.Event) + want bool + }{ + {"backspace at caret", input.DeleteIntent, func(event *atspi.Event) { event.Detail1 = 105 }, true}, + {"backspace before caret", input.DeleteIntent, func(event *atspi.Event) { event.Detail1 = 104 }, true}, + {"typing intent", input.TypingIntent, func(event *atspi.Event) { event.Detail1 = 105 }, false}, + {"no intent", input.NoIntent, func(event *atspi.Event) { event.Detail1 = 105 }, false}, + {"repaint above caret line", input.DeleteIntent, func(event *atspi.Event) { event.Detail1 = 40 }, false}, + {"unknown caret", input.DeleteIntent, func(event *atspi.Event) { + event.Detail1 = 105 + event.Source.TextCaretKnown = false + }, false}, + } { + t.Run(test.name, func(t *testing.T) { + event := atspi.NewEventWithPayload(atspi.TextDeletedEvent, terminal, "x") + test.modify(&event) + if got := is_user_terminal_deletion(event, test.intent); got != test.want { + t.Fatalf("is_user_terminal_deletion = %v, want %v", got, test.want) + } + }) + } +} + func TestProcessEventsPresentsStandaloneTerminalDeletion(t *testing.T) { speech := &fakeSpeechQueue{spoken: make(chan string, 1)} adapter := &fakeAdapter{events: make(chan atspi.Event, 1)} diff --git a/internal/app/window_titles.go b/internal/app/window_titles.go index f1de029..f2441c8 100644 --- a/internal/app/window_titles.go +++ b/internal/app/window_titles.go @@ -8,6 +8,7 @@ import ( "log/slog" "strings" "time" + "unicode" "git.stormux.org/storm/cthulhu/internal/atspi" "git.stormux.org/storm/cthulhu/internal/input" @@ -46,6 +47,7 @@ type windowTitleController struct { inputSource any lastNative atspi.ForegroundTitle lastTitle string + recent map[string]time.Time application string pending *pendingWindowTitle timer *time.Timer @@ -55,6 +57,10 @@ type windowTitleController struct { const ( windowTitleChangeDelay = 25 * time.Millisecond windowTitleIntroductionDelay = 250 * time.Millisecond + // Animated titles (terminal TUI spinners) cycle through the same set + // of frames about once a second. A title already announced within + // this window is another cycle, not a new title. + windowTitleRepeatWindow = 3 * time.Second ) func (titles *windowTitleController) clearPending() { @@ -64,6 +70,10 @@ func (titles *windowTitleController) clearPending() { titles.timer, titles.due, titles.pending = nil, nil, nil } +// The animated-title repeat history (titles.recent) deliberately +// survives reset and activation: it expires by time on its own, and a +// workspace round-trip away from a window with an animated title must +// not restart its announcement cycle on return. func (titles *windowTitleController) reset() { titles.clearPending() titles.lastTitle, titles.application = "", "" @@ -140,6 +150,11 @@ func (titles *windowTitleController) accessible(event atspi.Event) bool { titles.state.owner.changeTitle(event) titles.clearPending() titles.lastTitle = "" + // Keep the repeat history: animated title cycles include a + // blank frame every cycle (observed from terminal TUIs), and + // clearing here would restart the cycle announcement each + // second. A title restored after a genuine removal announces + // again once the repeat window expires. return true } previousTitle := titles.state.owner.title @@ -171,6 +186,16 @@ func (titles *windowTitleController) nativeChanged() { accessibleActivation := titles.state.owner.activated && !wine.IsReviewID(titles.state.owner.window) if switched { titles.clearPending() + // Keep the repeat history: a native sampler can re-report the + // same client as a switch (new epoch or title-window id) on + // every title sample, and clearing here would defeat animated + // title suppression. Genuine window switches clear it through + // AT-SPI activation instead. + if titles.logger != nil { + titles.logger.Debug("native window title switch", + "epoch", latest.Epoch, "window", latest.Title.Window, + "title_window", latest.Title.TitleWindow) + } // An AT-SPI activation may already have introduced this client. titles.lastTitle = titles.state.owner.title if wine.IsReviewID(titles.state.owner.window) { @@ -200,6 +225,46 @@ func (titles *windowTitleController) nativeChanged() { native: latest.Title, changed: changed}) } +// animatedWindowTitleKey reduces a title to the part that carries meaning +// across animation frames. A leading field with no letter or digit is a +// spinner frame, not content; a leading count stays, since it can change +// meaningfully. Announcement suppression uses this only to detect cycles; +// the spoken text remains the original title. +func animatedWindowTitleKey(title string) string { + fields := strings.Fields(strings.ToLower(title)) + if len(fields) > 1 && strings.IndexFunc(fields[0], func(r rune) bool { + return unicode.IsLetter(r) || unicode.IsDigit(r) + }) < 0 { + fields = fields[1:] + } + return strings.Join(fields, " ") +} + +// repeatedWithinWindow reports whether this title was already announced in +// the repeat window. A suppressed sighting refreshes its entry, so a +// running animation stays suppressed instead of breaking through once per +// window; it expires shortly after the animation stops. +func (titles *windowTitleController) repeatedWithinWindow(text string, at time.Time) bool { + key := animatedWindowTitleKey(text) + if key == "" { + return false + } + if at.IsZero() { + at = time.Now() + } + for seen, when := range titles.recent { + if at.Sub(when) > windowTitleRepeatWindow { + delete(titles.recent, seen) + } + } + if titles.recent == nil { + titles.recent = make(map[string]time.Time) + } + _, repeated := titles.recent[key] + titles.recent[key] = at + return repeated +} + func equivalentWindowTitles(first, second string) bool { normalize := func(value string) string { return strings.Join(strings.Fields(strings.ToLower(value)), " ") } first, second = normalize(first), normalize(second) @@ -327,6 +392,13 @@ func (titles *windowTitleController) flush() { if pending.inputSequence != recentInput(titles.inputSource).TitleInterruptionSequence { return } + if pending.changed && titles.repeatedWithinWindow(pending.text, pending.observedAt) { + if titles.logger != nil { + titles.logger.Debug("suppressed repeated window title", + "key", animatedWindowTitleKey(pending.text), "recent", len(titles.recent)) + } + return + } if titles.logger != nil { titles.logger.Debug("queued automatic window title", "title_changed", pending.changed, "observed_to_queue_ms", time.Since(pending.observedAt).Milliseconds()) diff --git a/internal/app/window_titles_test.go b/internal/app/window_titles_test.go index 3c277a7..1778536 100644 --- a/internal/app/window_titles_test.go +++ b/internal/app/window_titles_test.go @@ -76,8 +76,96 @@ func TestAutomaticTitlePolicyCoalescesAndPreservesSpeech(t *testing.T) { changed.Source.Name = "Loading 100%" titles.accessible(changed) titles.flush() + // A title restored within the repeat window is a repeat, not news; + // animated cycles pass through a blank frame every cycle and must + // not revive announcements. It announces again after the window. + if len(queue.requests) != 1 { + t.Fatal("restored title repeated within the repeat window") + } +} + +func TestAutomaticTitleSuppressesAnimatedSpinnerCycle(t *testing.T) { + titles, queue, changed := titleControllerFixture(t) + // Terminal TUIs animate a spinner glyph in the title several times a + // second while the meaningful title text stays the same. The cycle + // includes a blank frame; it must not reset suppression. + for _, text := range []string{"⠋ Terminal", "⠙ Terminal", "⠹ Terminal", "", "⠋ Terminal", "Terminal", "", "⠙ Terminal"} { + changed.Source.Name = text + titles.accessible(changed) + titles.flush() + } + if len(queue.requests) != 1 || queue.requests[0].Text != "⠋ Terminal" { + t.Fatalf("spinner title requests = %+v, want only the first frame", queue.requests) + } + // A different title is not part of the animation and still announces. + changed.Source.Name = "Build finished" + titles.accessible(changed) + titles.flush() + if len(queue.requests) != 2 || queue.requests[1].Text != "Build finished" { + t.Fatalf("title after spinner = %+v", queue.requests) + } + // A leading count is meaningful: each value announces, a rapid repeat + // of an earlier value in the same cycle does not. + for _, text := range []string{"(2) Terminal", "(3) Terminal", "(2) Terminal"} { + changed.Source.Name = text + titles.accessible(changed) + titles.flush() + } + if len(queue.requests) != 4 { + t.Fatalf("counted title requests = %+v, want the cycle repeat suppressed", queue.requests) + } +} + +func TestWindowReactivationDoesNotReviveSpinnerCycle(t *testing.T) { + titles, queue, changed := titleControllerFixture(t) + for _, text := range []string{"9 Terminal", "8 Terminal"} { + changed.Source.Name = text + titles.accessible(changed) + titles.flush() + } if len(queue.requests) != 2 { - t.Fatal("restored title was not announced") + t.Fatalf("initial spinner titles = %+v", queue.requests) + } + // Leaving for another workspace deactivates (reset) and returning + // reactivates the same window. Neither may restart the cycle. + titles.reset() + titles.activated(changed) + for _, text := range []string{"9 Terminal", "8 Terminal"} { + changed.Source.Name = text + titles.accessible(changed) + titles.flush() + } + if len(queue.requests) != 2 { + t.Fatalf("spinner revived by workspace round-trip = %+v", queue.requests) + } +} + +func TestNativeTitleSwitchSamplesDoNotReviveSpinnerCycle(t *testing.T) { + titles, queue, changed := titleControllerFixture(t) + source := &titleProbeSource{title: atspi.ForegroundTitle{Title: xorg.WindowTitle{Window: 10, TitleWindow: 10, Title: "Original"}}} + titles.source, titles.foreground = source, source + titles.nativeChanged() + titles.flush() + changed.Source.Name = "9 Terminal" + titles.accessible(changed) + titles.flush() + changed.Source.Name = "8 Terminal" + titles.accessible(changed) + titles.flush() + if len(queue.requests) != 2 { + t.Fatalf("initial spinner titles = %+v", queue.requests) + } + // A native sampler may re-report the same client as a switch on every + // sample. That must not wipe the animated-title repeat history, so the + // cycle returning to an earlier frame stays suppressed. + for _, text := range []string{"9 Terminal", "8 Terminal"} { + source.title.Epoch++ + source.title.Title.Title = text + titles.nativeChanged() + titles.flush() + } + if len(queue.requests) != 2 { + t.Fatalf("spinner revived by native switch samples = %+v", queue.requests) } }