Revert "tui: improve tmux experience and simplify keyboard enhancements"#2098
Revert "tui: improve tmux experience and simplify keyboard enhancements"#2098dgageot merged 1 commit intodocker:mainfrom
Conversation
This reverts commit 552362d. Signed-off-by: Guillaume Tardif <guillaume.tardif@gmail.com>
gtardif
force-pushed
the
revert_tui_changes
branch
from
2ffce2d to
f946c5d
Compare
![docker-agent[bot]](https://avatars.githubusercontent.com/in/2787757?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
Assessment: 🟡 NEEDS ATTENTION
This revert PR removes keyboard enhancement features that were causing issues with VSCode and AZERTY keyboard layouts. The revert itself is mostly clean, but introduces 2 confirmed bugs and 1 likely issue related to incomplete state management of keyboard enhancements.
Key Issues
🔴 CONFIRMED (2 findings):
- Chat page lacks keyboard enhancement tracking field (line 260)
- View configuration removal may break keyboard enhancement detection (toFullscreenView function)
🟡 LIKELY (1 finding):
- Message forwarding may not reach all nested components (line 597)
Details
The revert adds a new keyboardEnhancementsSupported field to track terminal capabilities, but:
- The chat page component doesn't track this state (unlike the editor component)
- The view configuration no longer requests keyboard enhancement events, which may prevent the new tracking field from being updated
- It's unclear if all nested components (especially inline edit textareas) properly receive keyboard enhancement configuration
Recommendation
Consider adding keyboard enhancement state tracking to the chat page component and verifying that the new keyboardEnhancementsSupported field will be properly updated without the view-level KeyboardEnhancements.ReportEventTypes setting.
| updated, _ := m.chatPage.Update(*m.keyboardEnhancements) | ||
| m.chatPage = updated.(chat.Page) | ||
| editorModel, _ := m.editor.Update(*m.keyboardEnhancements) | ||
| m.editor = editorModel.(editor.Editor) |
There was a problem hiding this comment.
🔴 CONFIRMED: Chat page lacks keyboard enhancement tracking
The reapplyKeyboardEnhancements() method forwards the keyboard enhancements message to both the chat page and editor. However, the chat page component (pkg/tui/page/chat/chat.go) does not have a keyboardEnhancementsSupported field like the editor does.
Impact: If the chat page needs to make decisions based on keyboard enhancement support (e.g., keybinding configuration for inline editing), it won't have that information available after reapply.
Recommendation: Consider adding a keyboardEnhancementsSupported field to the chat page component, similar to the editor component.
| return m, cmd | ||
| return m, tea.Batch(cmd, editorCmd) | ||
|
|
||
| // --- Keyboard input --- |
There was a problem hiding this comment.
🟡 LIKELY: Keyboard enhancements may not reach all nested components
The code forwards KeyboardEnhancementsMsg to both chatPage and editor, then batches the commands. While the forwarding to top-level components is correct, it's unclear whether all nested components that need keyboard enhancement configuration (especially inline edit textareas within the messages component) properly receive and configure their keybindings.
Impact: Some nested UI components may not properly configure keyboard shortcuts that depend on terminal capabilities.
Recommendation: Verify that the messages component and its inline edit textarea properly handle keyboard enhancement messages.
| view.KeyboardEnhancements.ReportEventTypes = true | ||
| if working { | ||
| view.ProgressBar = tea.NewProgressBar(tea.ProgressBarIndeterminate, 0) | ||
| } |
There was a problem hiding this comment.
🔴 CONFIRMED: Removed view config may prevent keyboard enhancement detection
This revert removes view.ReportFocus = true and view.KeyboardEnhancements.ReportEventTypes = true from the view configuration. However, the PR also adds a new keyboardEnhancementsSupported field (line 124) that is set based on received KeyboardEnhancementsMsg messages (line 590).
The problem: If the view no longer requests keyboard enhancement events via KeyboardEnhancements.ReportEventTypes, the terminal may not send KeyboardEnhancementsMsg at all, leaving the new tracking field uninitialized or stale.
Impact: The keyboardEnhancementsSupported field may never be updated, causing the application to incorrectly assume keyboard enhancements are unavailable even when the terminal supports them.
Recommendation: Either keep view.KeyboardEnhancements.ReportEventTypes = true or verify that keyboard enhancement messages are received through another mechanism.
2 participants
This reverts commit 552362d.
This was causing weird behaviour in VSCode, with azerty layout, capitalised letters (with Shift on) where using querty ; without shift, lowercase letters using azerty.