From febf902dc4c20c52c95c05611bfa6086e713d464 Mon Sep 17 00:00:00 2001 From: adamdotdevin <2363879+adamdottv@users.noreply.github.com> Date: Mon, 11 Aug 2025 18:36:15 -0500 Subject: [PATCH] Revert "feat: improve file attachment pasting (#1704)" This reverts commit 81a3e02474445bc4d22c7857d4c3b1f591c1cb56. --- .../tui/internal/components/chat/editor.go | 208 ++++--------- .../internal/components/chat/editor_test.go | 277 ------------------ packages/tui/internal/tui/tui.go | 3 - 3 files changed, 51 insertions(+), 437 deletions(-) delete mode 100644 packages/tui/internal/components/chat/editor_test.go diff --git a/packages/tui/internal/components/chat/editor.go b/packages/tui/internal/components/chat/editor.go index 1cfbbc06..509de624 100644 --- a/packages/tui/internal/components/chat/editor.go +++ b/packages/tui/internal/components/chat/editor.go @@ -27,50 +27,6 @@ import ( "github.com/sst/opencode/internal/util" ) -type AttachmentInsertedMsg struct{} - -// unescapeClipboardText trims surrounding quotes from clipboard text and returns the inner content. -// It avoids interpreting backslash escape sequences unless the text is explicitly quoted. -func (m *editorComponent) unescapeClipboardText(s string) string { - t := strings.TrimSpace(s) - if len(t) >= 2 { - first := t[0] - last := t[len(t)-1] - if (first == '"' && last == '"') || (first == '\'' && last == '\'') { - if u, err := strconv.Unquote(t); err == nil { - return u - } - return t[1 : len(t)-1] - } - } - return t -} - -// pathExists checks if the given path exists. Relative paths are resolved against the app CWD. -// Supports expanding '~' to the user's home directory. -func (m *editorComponent) pathExists(p string) bool { - if p == "" { - return false - } - if strings.HasPrefix(p, "~") { - if home, err := os.UserHomeDir(); err == nil { - if p == "~" { - p = home - } else if strings.HasPrefix(p, "~/") { - p = filepath.Join(home, p[2:]) - } - } - } - check := p - if !filepath.IsAbs(check) { - check = filepath.Join(m.app.Info.Path.Cwd, check) - } - if _, err := os.Stat(check); err == nil { - return true - } - return false -} - type EditorComponent interface { tea.Model tea.ViewModel @@ -197,123 +153,60 @@ func (m *editorComponent) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } case tea.PasteMsg: - // Normalize clipboard text first - textRaw := string(msg) - text := m.unescapeClipboardText(textRaw) + text := string(msg) - // Case 1: pasted content contains one or more inline @paths -> insert attachments inline - // We scan the raw pasted text to preserve original content around attachments. - if strings.Contains(textRaw, "@") { - last := 0 - idx := 0 - inserted := 0 - for idx < len(textRaw) { - r, size := utf8.DecodeRuneInString(textRaw[idx:]) - if r != '@' { - idx += size - continue - } - - // Insert preceding chunk before attempting to consume a path - if idx > last { - m.textarea.InsertRunesFromUserInput([]rune(textRaw[last:idx])) - } - - // Extract candidate path after '@' up to whitespace - start := idx + size - end := start - for end < len(textRaw) { - nr, ns := utf8.DecodeRuneInString(textRaw[end:]) - if nr == ' ' || nr == '\t' || nr == '\n' || nr == '\r' { - break - } - end += ns - } - - if end > start { - raw := textRaw[start:end] - // Trim common trailing punctuation that may follow paths in prose - trimmed := strings.TrimRight(raw, ",.;:)]}\\\"'?!") - suffix := raw[len(trimmed):] - p := filepath.Clean(trimmed) - if m.pathExists(p) { - att := m.createAttachmentFromPath(p) - if att != nil { - m.textarea.InsertAttachment(att) - if suffix != "" { - m.textarea.InsertRunesFromUserInput([]rune(suffix)) - } - // Insert a trailing space only if the next rune isn't already whitespace - insertSpace := true - if end < len(textRaw) { - nr, _ := utf8.DecodeRuneInString(textRaw[end:]) - if nr == ' ' || nr == '\t' || nr == '\n' || nr == '\r' { - insertSpace = false - } - } - if insertSpace { - m.textarea.InsertString(" ") - } - inserted++ - last = end - idx = end - continue - } - } - } - - // No valid path -> keep the '@' literally - m.textarea.InsertRune('@') - last = start - idx = start + if filePath := strings.TrimSpace(strings.TrimPrefix(text, "@")); strings.HasPrefix(text, "@") && filePath != "" { + statPath := filePath + if !filepath.IsAbs(filePath) { + statPath = filepath.Join(m.app.Info.Path.Cwd, filePath) } - // Insert any trailing content after the last processed segment - if last < len(textRaw) { - m.textarea.InsertRunesFromUserInput([]rune(textRaw[last:])) - } - if inserted > 0 { - return m, util.CmdHandler(AttachmentInsertedMsg{}) - } - } - - // Case 2: user typed '@' and then pasted a valid path -> replace '@' with attachment - at := m.textarea.LastRuneIndex('@') - if at != -1 && at == m.textarea.CursorColumn()-1 { - p := filepath.Clean(text) - if m.pathExists(p) { - cur := m.textarea.CursorColumn() - m.textarea.ReplaceRange(at, cur, "") - att := m.createAttachmentFromPath(p) - if att != nil { - m.textarea.InsertAttachment(att) + if _, err := os.Stat(statPath); err == nil { + attachment := m.createAttachmentFromPath(filePath) + if attachment != nil { + m.textarea.InsertAttachment(attachment) m.textarea.InsertString(" ") - return m, util.CmdHandler(AttachmentInsertedMsg{}) + return m, nil } } } - // Case 3: plain path pasted (e.g., drag-and-drop) -> attach if image or PDF - { - p := filepath.Clean(text) - if m.pathExists(p) { - mime := getMediaTypeFromExtension(strings.ToLower(filepath.Ext(p))) - if strings.HasPrefix(mime, "image/") || mime == "application/pdf" { - if att := m.createAttachmentFromFile(p); att != nil { - m.textarea.InsertAttachment(att) - m.textarea.InsertString(" ") - return m, util.CmdHandler(AttachmentInsertedMsg{}) - } - } + text = strings.ReplaceAll(text, "\\", "") + text, err := strconv.Unquote(`"` + text + `"`) + if err != nil { + slog.Error("Failed to unquote text", "error", err) + text := string(msg) + if m.shouldSummarizePastedText(text) { + m.handleLongPaste(text) + } else { + m.textarea.InsertRunesFromUserInput([]rune(msg)) } - } - - // Default: do not auto-convert. Insert raw text or summarize long pastes. - if m.shouldSummarizePastedText(textRaw) { - m.handleLongPaste(textRaw) return m, nil } - m.textarea.InsertRunesFromUserInput([]rune(textRaw)) - return m, nil + if _, err := os.Stat(text); err != nil { + slog.Error("Failed to paste file", "error", err) + text := string(msg) + if m.shouldSummarizePastedText(text) { + m.handleLongPaste(text) + } else { + m.textarea.InsertRunesFromUserInput([]rune(msg)) + } + return m, nil + } + + filePath := text + + attachment := m.createAttachmentFromFile(filePath) + if attachment == nil { + if m.shouldSummarizePastedText(text) { + m.handleLongPaste(text) + } else { + m.textarea.InsertRunesFromUserInput([]rune(msg)) + } + return m, nil + } + + m.textarea.InsertAttachment(attachment) + m.textarea.InsertString(" ") case tea.ClipboardMsg: text := string(msg) // Check if the pasted text is long and should be summarized @@ -340,7 +233,7 @@ func (m *editorComponent) Update(msg tea.Msg) (tea.Model, tea.Cmd) { if atIndex == -1 { // Should not happen, but as a fallback, just insert. m.textarea.InsertString(msg.Item.Value + " ") - return m, util.CmdHandler(AttachmentInsertedMsg{}) + return m, nil } // The range to replace is from the '@' up to the current cursor position. @@ -354,13 +247,13 @@ func (m *editorComponent) Update(msg tea.Msg) (tea.Model, tea.Cmd) { attachment := m.createAttachmentFromPath(filePath) m.textarea.InsertAttachment(attachment) m.textarea.InsertString(" ") - return m, util.CmdHandler(AttachmentInsertedMsg{}) + return m, nil case "symbols": atIndex := m.textarea.LastRuneIndex('@') if atIndex == -1 { // Should not happen, but as a fallback, just insert. m.textarea.InsertString(msg.Item.Value + " ") - return m, util.CmdHandler(AttachmentInsertedMsg{}) + return m, nil } cursorCol := m.textarea.CursorColumn() @@ -394,13 +287,13 @@ func (m *editorComponent) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.textarea.InsertAttachment(attachment) m.textarea.InsertString(" ") - return m, util.CmdHandler(AttachmentInsertedMsg{}) + return m, nil case "agents": atIndex := m.textarea.LastRuneIndex('@') if atIndex == -1 { // Should not happen, but as a fallback, just insert. m.textarea.InsertString(msg.Item.Value + " ") - return m, util.CmdHandler(AttachmentInsertedMsg{}) + return m, nil } cursorCol := m.textarea.CursorColumn() @@ -418,7 +311,8 @@ func (m *editorComponent) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.textarea.InsertAttachment(attachment) m.textarea.InsertString(" ") - return m, util.CmdHandler(AttachmentInsertedMsg{}) + return m, nil + default: slog.Debug("Unknown provider", "provider", msg.Item.ProviderID) return m, nil diff --git a/packages/tui/internal/components/chat/editor_test.go b/packages/tui/internal/components/chat/editor_test.go deleted file mode 100644 index f43a4078..00000000 --- a/packages/tui/internal/components/chat/editor_test.go +++ /dev/null @@ -1,277 +0,0 @@ -package chat - -import ( - "os" - "path/filepath" - "strings" - "testing" - - "github.com/charmbracelet/bubbles/v2/spinner" - tea "github.com/charmbracelet/bubbletea/v2" - "github.com/sst/opencode/internal/app" - "github.com/sst/opencode/internal/completions" - "github.com/sst/opencode/internal/components/dialog" - "github.com/sst/opencode/internal/components/textarea" - "github.com/sst/opencode/internal/styles" -) - -func newTestEditor() *editorComponent { - m := &editorComponent{ - app: &app.App{}, - textarea: textarea.New(), - spinner: spinner.New(), - } - return m -} - -func TestPasteAtPathWithTrailingComma_PreservesPunctuation_NoDoubleSpace(t *testing.T) { - m := newTestEditor() - p := createTempTextFile(t, "", "pc.txt", "x") - - paste := "See @" + p + ", next" - _, cmd := m.Update(tea.PasteMsg(paste)) - if cmd == nil { - t.Fatalf("expected command to be returned for comma punctuation paste") - } - if _, ok := cmd().(AttachmentInsertedMsg); !ok { - t.Fatalf("expected AttachmentInsertedMsg for comma punctuation paste") - } - if len(m.textarea.GetAttachments()) != 1 { - t.Fatalf("expected 1 attachment, got %d", len(m.textarea.GetAttachments())) - } - v := m.Value() - if !strings.Contains(v, ", next") { - t.Fatalf("expected comma and following text to be preserved, got: %q", v) - } - if strings.Contains(v, ", next") { - t.Fatalf("did not expect double space after comma, got: %q", v) - } -} - -func TestPasteAtPathWithTrailingQuestion_PreservesPunctuation_NoDoubleSpace(t *testing.T) { - m := newTestEditor() - p := createTempTextFile(t, "", "pq.txt", "x") - - paste := "Check @" + p + "? Done" - _, cmd := m.Update(tea.PasteMsg(paste)) - if cmd == nil { - t.Fatalf("expected command to be returned for question punctuation paste") - } - if _, ok := cmd().(AttachmentInsertedMsg); !ok { - t.Fatalf("expected AttachmentInsertedMsg for question punctuation paste") - } - if len(m.textarea.GetAttachments()) != 1 { - t.Fatalf("expected 1 attachment, got %d", len(m.textarea.GetAttachments())) - } - v := m.Value() - if !strings.Contains(v, "? Done") { - t.Fatalf("expected question mark and following text to be preserved, got: %q", v) - } - if strings.Contains(v, "? Done") { - t.Fatalf("did not expect double space after question mark, got: %q", v) - } -} - -func TestPasteMultipleInlineAtPaths_AttachesEach(t *testing.T) { - m := newTestEditor() - dir := t.TempDir() - p1 := createTempTextFile(t, dir, "m1.txt", "one") - p2 := createTempTextFile(t, dir, "m2.txt", "two") - - // Build a paste with text around, two @paths, and punctuation after the first - paste := "Please check @" + p1 + ", and also @" + p2 + " thanks" - - _, cmd := m.Update(tea.PasteMsg(paste)) - if cmd == nil { - t.Fatalf("expected command to be returned for multi inline paste") - } - if _, ok := cmd().(AttachmentInsertedMsg); !ok { - t.Fatalf("expected AttachmentInsertedMsg for multi inline paste") - } - - atts := m.textarea.GetAttachments() - if len(atts) != 2 { - t.Fatalf("expected 2 attachments, got %d", len(atts)) - } - v := m.Value() - if !strings.Contains(v, "Please check") || !strings.Contains(v, "and also") || !strings.Contains(v, "thanks") { - t.Fatalf("expected surrounding text to be preserved, got: %q", v) - } -} - -func createTempTextFile(t *testing.T, dir, name, content string) string { - t.Helper() - if dir == "" { - td, err := os.MkdirTemp("", "editor-test-*") - if err != nil { - t.Fatalf("failed to make temp dir: %v", err) - } - dir = td - } - p := filepath.Join(dir, name) - if err := os.WriteFile(p, []byte(content), 0o600); err != nil { - t.Fatalf("failed to write temp file: %v", err) - } - abs, err := filepath.Abs(p) - if err != nil { - t.Fatalf("failed to get abs path: %v", err) - } - return abs -} - -func createTempBinFile(t *testing.T, dir, name string, data []byte) string { - t.Helper() - if dir == "" { - td, err := os.MkdirTemp("", "editor-test-*") - if err != nil { - t.Fatalf("failed to make temp dir: %v", err) - } - dir = td - } - p := filepath.Join(dir, name) - if err := os.WriteFile(p, data, 0o600); err != nil { - t.Fatalf("failed to write temp bin file: %v", err) - } - abs, err := filepath.Abs(p) - if err != nil { - t.Fatalf("failed to get abs path: %v", err) - } - return abs -} - -func TestPasteStartsWithAt_AttachesAndEmitsMsg(t *testing.T) { - m := newTestEditor() - p := createTempTextFile(t, "", "a.txt", "hello") - - _, cmd := m.Update(tea.PasteMsg("@" + p)) - if cmd == nil { - t.Fatalf("expected command to be returned") - } - msg := cmd() - if _, ok := msg.(AttachmentInsertedMsg); !ok { - t.Fatalf("expected AttachmentInsertedMsg, got %T", msg) - } - - atts := m.textarea.GetAttachments() - if len(atts) != 1 { - t.Fatalf("expected 1 attachment, got %d", len(atts)) - } - if v := m.Value(); !strings.HasSuffix(v, " ") { - t.Fatalf("expected trailing space after attachment, got value: %q", v) - } -} - -func TestPasteAfterAt_ReplacesAtWithAttachment(t *testing.T) { - m := newTestEditor() - p := createTempTextFile(t, "", "b.txt", "hello") - - m.textarea.SetValue("@") - // Cursor should be at the end after SetValue; paste absolute path - _, cmd := m.Update(tea.PasteMsg(p)) - if cmd == nil { - t.Fatalf("expected command to be returned") - } - if _, ok := cmd().(AttachmentInsertedMsg); !ok { - t.Fatalf("expected AttachmentInsertedMsg from paste after '@'") - } - - // Ensure the raw '@' rune was removed (attachment inserted in its place) - if m.textarea.LastRuneIndex('@') != -1 { - t.Fatalf("'@' rune should have been removed from the text slice") - } - if len(m.textarea.GetAttachments()) != 1 { - t.Fatalf("expected 1 attachment inserted") - } - if v := m.Value(); !strings.HasSuffix(v, " ") { - t.Fatalf("expected trailing space after attachment, got value: %q", v) - } -} - -func TestPlainTextPaste_NoAttachment_NoMsg(t *testing.T) { - m := newTestEditor() - _, cmd := m.Update(tea.PasteMsg("hello")) - if cmd != nil { - t.Fatalf("expected no command for plain text paste") - } - if got := m.Value(); got != "hello" { - t.Fatalf("expected value 'hello', got %q", got) - } - if len(m.textarea.GetAttachments()) != 0 { - t.Fatalf("expected no attachments for plain text paste") - } -} - -func TestPlainPathPng_AttachesImage(t *testing.T) { - m := newTestEditor() - // Minimal bytes; content isn't validated, extension determines mime - p := createTempBinFile(t, "", "img.png", []byte{0x89, 'P', 'N', 'G'}) - - _, cmd := m.Update(tea.PasteMsg(p)) - if cmd == nil { - t.Fatalf("expected command to be returned for image path paste") - } - if _, ok := cmd().(AttachmentInsertedMsg); !ok { - t.Fatalf("expected AttachmentInsertedMsg for image path paste") - } - atts := m.textarea.GetAttachments() - if len(atts) != 1 { - t.Fatalf("expected 1 attachment, got %d", len(atts)) - } - if atts[0].MediaType != "image/png" { - t.Fatalf("expected image/png mime, got %q", atts[0].MediaType) - } - if v := m.Value(); !strings.HasSuffix(v, " ") { - t.Fatalf("expected trailing space after attachment, got value: %q", v) - } -} - -func TestPlainPathPdf_AttachesPDF(t *testing.T) { - m := newTestEditor() - p := createTempBinFile(t, "", "doc.pdf", []byte("%PDF-1.4")) - - _, cmd := m.Update(tea.PasteMsg(p)) - if cmd == nil { - t.Fatalf("expected command to be returned for pdf path paste") - } - if _, ok := cmd().(AttachmentInsertedMsg); !ok { - t.Fatalf("expected AttachmentInsertedMsg for pdf path paste") - } - atts := m.textarea.GetAttachments() - if len(atts) != 1 { - t.Fatalf("expected 1 attachment, got %d", len(atts)) - } - if atts[0].MediaType != "application/pdf" { - t.Fatalf("expected application/pdf mime, got %q", atts[0].MediaType) - } - if v := m.Value(); !strings.HasSuffix(v, " ") { - t.Fatalf("expected trailing space after attachment, got value: %q", v) - } -} - -func TestCompletionFiles_InsertsAttachment_EmitsMsg(t *testing.T) { - m := newTestEditor() - p := createTempTextFile(t, "", "c.txt", "hello") - m.textarea.SetValue("@") - - item := completions.CompletionSuggestion{ - ProviderID: "files", - Value: p, - Display: func(_ styles.Style) string { return p }, - } - // Build the completion selected message as if the user selected from the dialog - msg := dialog.CompletionSelectedMsg{Item: item, SearchString: "@"} - - _, cmd := m.Update(msg) - if cmd == nil { - t.Fatalf("expected command to be returned") - } - if _, ok := cmd().(AttachmentInsertedMsg); !ok { - t.Fatalf("expected AttachmentInsertedMsg from files completion selection") - } - if len(m.textarea.GetAttachments()) != 1 { - t.Fatalf("expected 1 attachment inserted from completion selection") - } - if v := m.Value(); !strings.HasSuffix(v, " ") { - t.Fatalf("expected trailing space after attachment, got value: %q", v) - } -} diff --git a/packages/tui/internal/tui/tui.go b/packages/tui/internal/tui/tui.go index 1899df35..6a56e201 100644 --- a/packages/tui/internal/tui/tui.go +++ b/packages/tui/internal/tui/tui.go @@ -382,9 +382,6 @@ func (a Model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { a.app.Messages = []app.Message{} case dialog.CompletionDialogCloseMsg: a.showCompletionDialog = false - case chat.AttachmentInsertedMsg: - // Close completion dialog when the editor inserts an attachment - a.showCompletionDialog = false case opencode.EventListResponseEventInstallationUpdated: return a, toast.NewSuccessToast( "opencode updated to "+msg.Properties.Version+", restart to apply.",