Reusable editor (More)#3047
Conversation
… rune ansi.TruncateLeft keeps a wide rune that straddles the cut, which shifted the rest of the spliced line one cell to the right. Cut the straddled rune entirely and pad its uncovered cell with a space, what a terminal shows for a half-overwritten cell. Covered by new spliceLine unit tests (plain/styled/wide-rune/padding cases). Signed-off-by: David Gageot <david.gageot@docker.com>
SetRecording(false) reset the placeholder to the hardcoded default, defeating WithPlaceholder for embedders that use recording mode. Track the configured placeholder on the editor and restore that instead. Signed-off-by: David Gageot <david.gageot@docker.com>
docker-agent
left a comment
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟡 NEEDS ATTENTION
One LIKELY issue was found in the changed code. One finding was dismissed after analysis confirmed the arithmetic is always safe.
| return e.tickRecordingDots() | ||
| } | ||
| e.textarea.Placeholder = "Type your message here…" | ||
| e.textarea.Placeholder = e.placeholder |
There was a problem hiding this comment.
[MEDIUM] SetRecording(false) overwrites the read-only placeholder with the default placeholder
The new e.placeholder field (introduced in this PR) is restored here when recording ends. However, WithReadOnly() sets e.textarea.Placeholder = "Session is read-only" directly without updating e.placeholder. As a result, if SetRecording(true) is called on a read-only editor and then SetRecording(false) is called, this line will restore e.placeholder — which holds defaultPlaceholder ("Type your message here…") rather than the read-only text — silently removing the read-only indicator from the UI.
Fix: Update WithReadOnly to also set e.placeholder:
func WithReadOnly() Option {
return func(e *editor) {
e.placeholder = "Session is read-only"
e.textarea.Placeholder = "Session is read-only"
e.textarea.KeyMap.InsertNewline.SetEnabled(false)
}
}3 participants
No description provided.