Keep rich composers rich across Reset - #651
Open
jeremy wants to merge 2 commits into
Open
Conversation
Composer.Reset() unconditionally dropped the composer to quick (single-line) mode. That collapse is right for the one quick-constructed composer (chat, where auto-expansion is a temporary state), but five composers are constructed rich — and for them Reset silently demoted below their constructed mode. Two failure paths. The to-do description editor Resets its long-lived rich composer and then SetValues the existing description as Markdown: the single-line textinput sanitizes the newlines, so a multi-paragraph description flattens to one line in the editor and saves back flattened — silent data loss. The detail-comment and check-in-answer composers Reset after each send and, since both disable keystroke auto-expansion, then sat hard-stuck in single-line mode where Enter sends — multi-line replies only worked again via paste or $EDITOR, which expand programmatically. Record the constructed mode and make Reset restore it: a quick composer that auto-expanded still collapses back, and a rich composer stays rich. And close the flattening trap at its entrance — SetValue was the only content entry point that didn't auto-expand for multi-line or Markdown content the way InsertPaste and HandleEditorReturn do. Now it does, and those two reuse it instead of carrying their own expansion checks. The Reset-to-quick contract dated to an incidental hunk in the drag-and-drop commit (#209); TestComposerResetReturnsToQuickMode encoded it and is replaced by TestComposerResetRestoresConstructedMode, which covers both directions.
There was a problem hiding this comment.
Pull request overview
Preserves each composer’s constructed mode across resets, preventing rich content from being flattened.
Changes:
- Restore the initial composer mode on reset.
- Auto-expand quick composers for multiline or Markdown values.
- Add composer and todo-description regression tests.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
internal/tui/workspace/widget/composer.go |
Preserves rich mode and centralizes content expansion. |
internal/tui/workspace/widget/composer_test.go |
Tests reset and multiline behavior. |
internal/tui/workspace/views/todos_test.go |
Verifies multiline descriptions remain intact. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The test exercised the newline trigger and the plain single-line negative case but not the IsMarkdown trigger, so a refactor could drop the Markdown half of the condition without failing anything. Assert that a single-line Markdown value expands to rich mode.
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
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.
Spun off from #650, where it surfaced while testing table editing: the todos description composer flattens any multi-line description to one line on edit.
Diagnosis
Composer.Reset()unconditionally setmode = ComposerQuick. That collapse is correct for the one quick-constructed composer (chat, where rich mode is a temporary auto-expansion), but five composers are constructed rich — and for them Reset silently demoted below the constructed mode. The contract dates to an incidental hunk in the drag-and-drop commit (#209); no rationale ties it to rich composers.Two failure paths:
startEditDescriptionResets its long-lived rich composer, thenSetValues the existing description as Markdown. In quick mode that routes to the single-linetextinput, which sanitizes newlines — a multi-paragraph description displays flattened and saves back flattened.SetValuewas also the only content entry point without the multi-line auto-expand thatInsertPasteandHandleEditorReturnboth have.WithAutoExpand(false), so after the first send the composer sat in quick mode where Enter sends — and with keystroke expansion disabled, only paste or$EDITORcould ever get it back to rich.Fix
NewComposerrecords the constructed mode;Reset()restores it instead of hardcoding quick. Chat's quick→auto-expanded→Reset→quick cycle is unchanged; rich composers stay rich.SetValuenow auto-expands a quick composer for multi-line/Markdown content, closing the flattening trap for every caller — current and future.InsertPasteandHandleEditorReturndrop their own duplicate expansion checks and reuse it.Testing
TestComposerResetRestoresConstructedModereplacesTestComposerResetReturnsToQuickMode(which encoded the bug), covering both directions: rich stays rich, auto-expanded quick collapses back.TestComposerSetValueExpandsForMultilineContent: newlines survive, single-line stays quick.TestTodos_EditDescription_PreservesMultilineMarkdown: view-level regression — a two-paragraph description reaches the composer intact, newlines included.bin/cigreen.#650 is now stacked on this branch: relaxing the todos table-edit guard is only safe with this fix underneath, and its
TestTodos_EditDescription_EntersForSimpleTableasserts the exact multiline pipe table to prove the pairing. Merge this PR first.