From c185dc84d6810ee6a56b656640c213f65150d30f Mon Sep 17 00:00:00 2001 From: Kujtim Hoxha Date: Fri, 4 Apr 2025 14:36:57 +0200 Subject: [PATCH] Enhance UI feedback and improve file diff visualization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Improve diff display in permission dialogs with better formatting - Add visual indicators for focus changes in permission dialogs - Increase diagnostics timeout from 5 to 10 seconds - Fix write tool to show proper diffs for existing files - Update status component to properly handle messages 🤖 Generated with termai Co-Authored-By: termai --- internal/llm/tools/diagnostics.go | 5 +- internal/llm/tools/edit.go | 33 +++++++++-- internal/llm/tools/write.go | 11 +++- internal/tui/components/dialog/permission.go | 58 +++++++++++++++----- internal/tui/tui.go | 2 + 5 files changed, 87 insertions(+), 22 deletions(-) diff --git a/internal/llm/tools/diagnostics.go b/internal/llm/tools/diagnostics.go index 97ac149b..3eb72eaf 100644 --- a/internal/llm/tools/diagnostics.go +++ b/internal/llm/tools/diagnostics.go @@ -72,6 +72,7 @@ func notifyLspOpenFile(ctx context.Context, filePath string, lsps map[string]*ls // Create a notification handler that will signal when diagnostics are received handler := func(params json.RawMessage) { + lsp.HandleDiagnostics(client, params) var diagParams protocol.PublishDiagnosticsParams if err := json.Unmarshal(params, &diagParams); err != nil { return @@ -103,8 +104,8 @@ func notifyLspOpenFile(ctx context.Context, filePath string, lsps map[string]*ls select { case <-diagChan: // Diagnostics received - case <-time.After(5 * time.Second): - // Timeout after 2 seconds - this is a fallback in case no diagnostics are published + case <-time.After(10 * time.Second): + // Timeout after 5 seconds - this is a fallback in case no diagnostics are published case <-ctx.Done(): // Context cancelled } diff --git a/internal/llm/tools/edit.go b/internal/llm/tools/edit.go index f12a1eb2..394ce13b 100644 --- a/internal/llm/tools/edit.go +++ b/internal/llm/tools/edit.go @@ -303,23 +303,46 @@ func GenerateDiff(oldContent, newContent string) string { diffs = dmp.DiffCharsToLines(diffs, dmpStrings) diffs = dmp.DiffCleanupSemantic(diffs) buff := strings.Builder{} + + // Add a header to make the diff more readable + buff.WriteString("Changes:\n") + for _, diff := range diffs { text := diff.Text switch diff.Type { case diffmatchpatch.DiffInsert: - for line := range strings.SplitSeq(text, "\n") { + for _, line := range strings.Split(text, "\n") { + if line == "" { + continue + } _, _ = buff.WriteString("+ " + line + "\n") } case diffmatchpatch.DiffDelete: - for line := range strings.SplitSeq(text, "\n") { + for _, line := range strings.Split(text, "\n") { + if line == "" { + continue + } _, _ = buff.WriteString("- " + line + "\n") } case diffmatchpatch.DiffEqual: - if len(text) > 40 { - _, _ = buff.WriteString(" " + text[:20] + "..." + text[len(text)-20:] + "\n") + // Only show a small context for unchanged text + lines := strings.Split(text, "\n") + if len(lines) > 3 { + // Show only first and last line of context with a separator + if lines[0] != "" { + _, _ = buff.WriteString(" " + lines[0] + "\n") + } + _, _ = buff.WriteString(" ...\n") + if lines[len(lines)-1] != "" { + _, _ = buff.WriteString(" " + lines[len(lines)-1] + "\n") + } } else { - for line := range strings.SplitSeq(text, "\n") { + // Show all lines for small contexts + for _, line := range lines { + if line == "" { + continue + } _, _ = buff.WriteString(" " + line + "\n") } } diff --git a/internal/llm/tools/write.go b/internal/llm/tools/write.go index 3d66d64e..1c32410f 100644 --- a/internal/llm/tools/write.go +++ b/internal/llm/tools/write.go @@ -101,6 +101,15 @@ func (w *writeTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error } notifyLspOpenFile(ctx, filePath, w.lspClients) + // Get old content for diff if file exists + oldContent := "" + if fileInfo != nil && !fileInfo.IsDir() { + oldBytes, readErr := os.ReadFile(filePath) + if readErr == nil { + oldContent = string(oldBytes) + } + } + p := permission.Default.Request( permission.CreatePermissionRequest{ Path: filePath, @@ -109,7 +118,7 @@ func (w *writeTool) Run(ctx context.Context, call ToolCall) (ToolResponse, error Description: fmt.Sprintf("Create file %s", filePath), Params: WritePermissionsParams{ FilePath: filePath, - Content: GenerateDiff("", params.Content), + Content: GenerateDiff(oldContent, params.Content), }, }, ) diff --git a/internal/tui/components/dialog/permission.go b/internal/tui/components/dialog/permission.go index 29f1ff70..856f045c 100644 --- a/internal/tui/components/dialog/permission.go +++ b/internal/tui/components/dialog/permission.go @@ -79,10 +79,18 @@ func (p *permissionDialogCmp) Update(msg tea.Msg) (tea.Model, tea.Cmd) { p.isViewportFocus = !p.isViewportFocus if p.isViewportFocus { p.selectOption.Blur() + // Add a visual indicator for focus change + cmds = append(cmds, tea.Batch( + util.CmdHandler(util.InfoMsg("Viewing content - use arrow keys to scroll")), + )) } else { p.selectOption.Focus() + // Add a visual indicator for focus change + cmds = append(cmds, tea.Batch( + util.CmdHandler(util.InfoMsg("Select an action")), + )) } - return p, nil + return p, tea.Batch(cmds...) } } @@ -133,34 +141,55 @@ func (p *permissionDialogCmp) render() string { case tools.BashToolName: pr := p.permission.Params.(tools.BashPermissionsParams) headerParts = append(headerParts, keyStyle.Render("Command:")) - content, _ = r.Render(fmt.Sprintf("```bash\n%s\n```", pr.Command)) + content = fmt.Sprintf("```bash\n%s\n```", pr.Command) case tools.EditToolName: pr := p.permission.Params.(tools.EditPermissionsParams) headerParts = append(headerParts, keyStyle.Render("Update")) - content, _ = r.Render(fmt.Sprintf("```diff\n%s\n```", pr.Diff)) + content = fmt.Sprintf("```\n%s\n```", pr.Diff) case tools.WriteToolName: pr := p.permission.Params.(tools.WritePermissionsParams) headerParts = append(headerParts, keyStyle.Render("Content")) - content, _ = r.Render(fmt.Sprintf("```diff\n%s\n```", pr.Content)) + content = fmt.Sprintf("```\n%s\n```", pr.Content) case tools.FetchToolName: pr := p.permission.Params.(tools.FetchPermissionsParams) headerParts = append(headerParts, keyStyle.Render("URL: "+pr.URL)) default: - content, _ = r.Render(p.permission.Description) + content = p.permission.Description } + + renderedContent, _ := r.Render(content) headerContent := lipgloss.NewStyle().Padding(0, 1).Render(lipgloss.JoinVertical(lipgloss.Left, headerParts...)) p.contentViewPort.Width = p.width - 2 - 2 p.contentViewPort.Height = p.height - lipgloss.Height(headerContent) - lipgloss.Height(form) - 2 - 2 - 1 - p.contentViewPort.SetContent(content) - contentBorder := lipgloss.RoundedBorder() + p.contentViewPort.SetContent(renderedContent) + + // Make focus change more apparent with different border styles and colors + var contentBorder lipgloss.Border + var borderColor lipgloss.TerminalColor + if p.isViewportFocus { contentBorder = lipgloss.DoubleBorder() + borderColor = styles.Blue + } else { + contentBorder = lipgloss.RoundedBorder() + borderColor = styles.Flamingo } - cotentStyle := lipgloss.NewStyle().MarginTop(1).Padding(0, 1).Border(contentBorder).BorderForeground(styles.Flamingo) - contentFinal := cotentStyle.Render(p.contentViewPort.View()) - if content == "" { + + contentStyle := lipgloss.NewStyle(). + MarginTop(1). + Padding(0, 1). + Border(contentBorder). + BorderForeground(borderColor) + + if p.isViewportFocus { + contentStyle = contentStyle.BorderBackground(styles.Surface0) + } + + contentFinal := contentStyle.Render(p.contentViewPort.View()) + if renderedContent == "" { contentFinal = "" } + return lipgloss.JoinVertical( lipgloss.Top, headerContent, @@ -241,12 +270,13 @@ func NewPermissionDialogCmd(permission permission.PermissionRequest) tea.Cmd { minWidth := 100 minHeight := 30 + // Make the dialog size more appropriate for bash commands switch permission.ToolName { case tools.BashToolName: - widthRatio = 0.5 - heightRatio = 0.3 - minWidth = 80 - minHeight = 20 + widthRatio = 0.7 + heightRatio = 0.5 + minWidth = 100 + minHeight = 30 } // Return the dialog command return util.CmdHandler(core.DialogMsg{ diff --git a/internal/tui/tui.go b/internal/tui/tui.go index 0e24613d..afbd9413 100644 --- a/internal/tui/tui.go +++ b/internal/tui/tui.go @@ -166,6 +166,8 @@ func (a appModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { a.dialog = d.(core.DialogCmp) return a, cmd } + s, _ := a.status.Update(msg) + a.status = s p, cmd := a.pages[a.currentPage].Update(msg) a.pages[a.currentPage] = p return a, cmd