diff --git a/README.md b/README.md index c8b02d8..45376c5 100644 --- a/README.md +++ b/README.md @@ -239,10 +239,8 @@ Configuration is optional TOML. diple checks: 1. `--config FILE`; 2. `DIPLE_CONFIG`; -3. `GH_THREADS_CONFIG` as a migration fallback; -4. `$XDG_CONFIG_HOME/diple/config.toml`; -5. the operating-system configuration directory; and -6. legacy `gh-threads` paths when no diple configuration exists. +3. `$XDG_CONFIG_HOME/diple/config.toml`; and +4. the operating-system configuration directory. Common default paths: diff --git a/ai_tui.go b/ai_tui.go index 2af75fb..ed68cf8 100644 --- a/ai_tui.go +++ b/ai_tui.go @@ -26,14 +26,16 @@ const ( ) type aiPreparedMsg struct { - preview AIPreview - threadID string - err error + generation uint64 + preview AIPreview + threadID string + err error } type aiCompletedMsg struct { - result AIResult - err error + generation uint64 + result AIResult + err error } type aiStatusMsg struct { @@ -41,15 +43,24 @@ type aiStatusMsg struct { } type aiProgressMsg struct { - progress AIRunProgress + generation uint64 + progress AIRunProgress } type aiProviderTestCompletedMsg struct { - model string - err error + generation uint64 + model string + err error } -type aiAnimationTickMsg time.Time +type aiAnimationTickMsg struct { + generation uint64 +} + +func (m *App) nextAIGeneration() uint64 { + m.aiGeneration++ + return m.aiGeneration +} func (m *App) openAIMenu() { if m.screen == prScreen { @@ -97,6 +108,7 @@ func (m *App) beginAIPrepare(threadID, message string) tea.Cmd { } ctx, cancel := context.WithCancel(context.Background()) m.aiCancel = cancel + generation := m.nextAIGeneration() controller, details := m.ai, m.details m.aiMode = aiPreparing m.err = nil @@ -112,9 +124,11 @@ func (m *App) beginAIPrepare(threadID, message string) tea.Cmd { } prepare := func() tea.Msg { preview, err := controller.Prepare(ctx, details, threadID, message) - return aiPreparedMsg{preview: preview, threadID: threadID, err: err} + return aiPreparedMsg{ + generation: generation, preview: preview, threadID: threadID, err: err, + } } - return tea.Batch(prepare, nextAIAnimationTick()) + return tea.Batch(prepare, nextAIAnimationTick(generation)) } func (m *App) returnFromAIPrepareFailure(threadID string) { @@ -133,6 +147,7 @@ func (m *App) beginAIRun() tea.Cmd { } ctx, cancel := context.WithCancel(context.Background()) m.aiCancel = cancel + generation := m.nextAIGeneration() controller, preview := m.ai, m.aiPreview m.aiMode = aiBusy m.aiSpinner = 0 @@ -146,18 +161,21 @@ func (m *App) beginAIRun() tea.Cmd { m.aiEvents = events work := func() tea.Msg { go func() { + defer close(events) result, err := controller.RunWithProgress(ctx, preview, func(progress AIRunProgress) { select { - case events <- aiProgressMsg{progress: progress}: + case events <- aiProgressMsg{generation: generation, progress: progress}: default: } }) - events <- aiCompletedMsg{result: result, err: err} - close(events) + select { + case events <- aiCompletedMsg{generation: generation, result: result, err: err}: + case <-ctx.Done(): + } }() return <-events } - return tea.Batch(work, nextAIAnimationTick()) + return tea.Batch(work, nextAIAnimationTick(generation)) } func (m *App) beginAIProviderTest() tea.Cmd { @@ -168,6 +186,7 @@ func (m *App) beginAIProviderTest() tea.Cmd { } ctx, cancel := context.WithCancel(context.Background()) m.aiCancel = cancel + generation := m.nextAIGeneration() controller := m.ai m.aiMode = aiProviderTestBusy m.aiSpinner = 0 @@ -181,18 +200,23 @@ func (m *App) beginAIProviderTest() tea.Cmd { m.aiEvents = events work := func() tea.Msg { go func() { + defer close(events) model, err := controller.TestProvider(ctx, func(progress AIRunProgress) { select { - case events <- aiProgressMsg{progress: progress}: + case events <- aiProgressMsg{generation: generation, progress: progress}: default: } }) - events <- aiProviderTestCompletedMsg{model: model, err: err} - close(events) + select { + case events <- aiProviderTestCompletedMsg{ + generation: generation, model: model, err: err, + }: + case <-ctx.Done(): + } }() return <-events } - return tea.Batch(work, nextAIAnimationTick()) + return tea.Batch(work, nextAIAnimationTick(generation)) } func waitAIEvent(events <-chan tea.Msg) tea.Cmd { @@ -204,9 +228,9 @@ func waitAIEvent(events <-chan tea.Msg) tea.Cmd { } } -func nextAIAnimationTick() tea.Cmd { - return tea.Tick(100*time.Millisecond, func(at time.Time) tea.Msg { - return aiAnimationTickMsg(at) +func nextAIAnimationTick(generation uint64) tea.Cmd { + return tea.Tick(100*time.Millisecond, func(time.Time) tea.Msg { + return aiAnimationTickMsg{generation: generation} }) } @@ -215,7 +239,7 @@ func (m App) updateAI(msg tea.Msg) (tea.Model, tea.Cmd, bool) { case tea.WindowSizeMsg: return m, nil, false case aiPreparedMsg: - if m.aiMode != aiPreparing { + if msg.generation != m.aiGeneration || m.aiMode != aiPreparing { return m, nil, true } m.aiCancel = nil @@ -230,15 +254,19 @@ func (m App) updateAI(msg tea.Msg) (tea.Model, tea.Cmd, bool) { } return m, nil, true case aiAnimationTickMsg: + if msg.generation != m.aiGeneration { + return m, nil, true + } switch m.aiMode { case aiPreparing, aiBusy, aiProviderTestBusy: m.aiSpinner++ - return m, nextAIAnimationTick(), true + return m, nextAIAnimationTick(msg.generation), true default: return m, nil, true } case aiProgressMsg: - if m.aiMode != aiBusy && m.aiMode != aiProviderTestBusy { + if msg.generation != m.aiGeneration || + (m.aiMode != aiBusy && m.aiMode != aiProviderTestBusy) { return m, nil, true } m.aiProgress = msg.progress @@ -247,7 +275,7 @@ func (m App) updateAI(msg tea.Msg) (tea.Model, tea.Cmd, bool) { m.aiStatus, m.aiStatusBusy = msg.status, false return m, nil, true case aiCompletedMsg: - if m.aiMode != aiBusy { + if msg.generation != m.aiGeneration || m.aiMode != aiBusy { return m, nil, true } m.aiCancel, m.aiEvents = nil, nil @@ -281,7 +309,7 @@ func (m App) updateAI(msg tea.Msg) (tea.Model, tea.Cmd, bool) { m.recordHealth("AI provider", healthOK, healthMessage) return m, nil, true case aiProviderTestCompletedMsg: - if m.aiMode != aiProviderTestBusy { + if msg.generation != m.aiGeneration || m.aiMode != aiProviderTestBusy { return m, nil, true } m.aiCancel, m.aiEvents = nil, nil @@ -322,6 +350,7 @@ func (m App) updateAI(msg tea.Msg) (tea.Model, tea.Cmd, bool) { m.aiCancel() m.aiCancel = nil } + m.nextAIGeneration() m.aiMode, m.aiInput, m.writeThreadID, m.aiEvents = aiNone, "", "", nil m.aiInputEditor = textEditor{} return m, nil, true diff --git a/cli.go b/cli.go index f7dd4f1..6134005 100644 --- a/cli.go +++ b/cli.go @@ -108,8 +108,7 @@ Other: Boolean options accept explicit values, for example --cache=false. Command-line options override TOML settings. GH_REPO is used only when ---repo is absent. DIPLE_CONFIG selects a configuration file; GH_THREADS_CONFIG -is retained as a migration fallback. +--repo is absent. DIPLE_CONFIG selects a configuration file. Authentication uses GH_TOKEN or GITHUB_TOKEN when set, otherwise the active credential from 'gh auth login'. Run 'diple completion --help' for completion diff --git a/config.go b/config.go index 65366f1..4cb5fc4 100644 --- a/config.go +++ b/config.go @@ -137,16 +137,8 @@ func configPath() (string, error) { if path := os.Getenv("DIPLE_CONFIG"); path != "" { return path, nil } - // Preserve the old override during the rename so existing scripts do not - // silently start with a fresh configuration. - if path := os.Getenv("GH_THREADS_CONFIG"); path != "" { - return path, nil - } if base := os.Getenv("XDG_CONFIG_HOME"); base != "" { - return firstExistingOrDefault( - filepath.Join(base, "diple", "config.toml"), - filepath.Join(base, "gh-threads", "config.toml"), - ), nil + return filepath.Join(base, "diple", "config.toml"), nil } base, err := os.UserConfigDir() if err != nil { @@ -158,19 +150,11 @@ func configPath() (string, error) { return "", fmt.Errorf("find home directory: %w", err) } dotConfig := filepath.Join(home, ".config", "diple", "config.toml") - legacyPreferred := filepath.Join(base, "gh-threads", "config.toml") - legacyDotConfig := filepath.Join(home, ".config", "gh-threads", "config.toml") - return firstExistingOrDefault( - preferred, dotConfig, legacyPreferred, legacyDotConfig, - ), nil + return existingConfigPath(preferred, dotConfig), nil } func existingConfigPath(preferred, fallback string) string { - return firstExistingOrDefault(preferred, fallback) -} - -func firstExistingOrDefault(preferred string, alternatives ...string) string { - for _, candidate := range append([]string{preferred}, alternatives...) { + for _, candidate := range []string{preferred, fallback} { if _, err := os.Stat(candidate); err == nil || !errors.Is(err, os.ErrNotExist) { return candidate } @@ -259,10 +243,7 @@ func defaultCacheDir() (string, error) { if err != nil { return "", fmt.Errorf("find user cache directory: %w", err) } - return firstExistingOrDefault( - filepath.Join(base, "diple"), - filepath.Join(base, "gh-threads"), - ), nil + return filepath.Join(base, "diple"), nil } func validateThreadStatusOrder(order []string) error { diff --git a/config_test.go b/config_test.go index a65f628..a45b4f4 100644 --- a/config_test.go +++ b/config_test.go @@ -233,7 +233,6 @@ func TestLoadConfigRejectsUnknownSettings(t *testing.T) { func TestConfigPathHonorsEnvironmentOverride(t *testing.T) { t.Setenv("DIPLE_CONFIG", "/tmp/custom-diple.toml") - t.Setenv("GH_THREADS_CONFIG", "") got, err := configPath() if err != nil { t.Fatal(err) @@ -243,18 +242,6 @@ func TestConfigPathHonorsEnvironmentOverride(t *testing.T) { } } -func TestConfigPathHonorsLegacyEnvironmentOverride(t *testing.T) { - t.Setenv("DIPLE_CONFIG", "") - t.Setenv("GH_THREADS_CONFIG", "/tmp/legacy-gh-threads.toml") - got, err := configPath() - if err != nil { - t.Fatal(err) - } - if got != "/tmp/legacy-gh-threads.toml" { - t.Fatalf("legacy config path = %q", got) - } -} - func TestExistingConfigPathFallsBackToDotConfig(t *testing.T) { root := t.TempDir() preferred := filepath.Join(root, "Library", "Application Support", "diple", "config.toml") @@ -280,23 +267,7 @@ func TestExistingConfigPathFallsBackToDotConfig(t *testing.T) { } } -func TestFirstExistingConfigPathFallsBackToLegacyName(t *testing.T) { - root := t.TempDir() - current := filepath.Join(root, "diple", "config.toml") - legacy := filepath.Join(root, "gh-threads", "config.toml") - if err := os.MkdirAll(filepath.Dir(legacy), 0o700); err != nil { - t.Fatal(err) - } - if err := os.WriteFile(legacy, []byte("theme = \"dark\"\n"), 0o600); err != nil { - t.Fatal(err) - } - if got := firstExistingOrDefault(current, legacy); got != legacy { - t.Fatalf("migration config path = %q, want %q", got, legacy) - } -} - func TestConfigPathHonorsXDGConfigHome(t *testing.T) { - t.Setenv("GH_THREADS_CONFIG", "") t.Setenv("DIPLE_CONFIG", "") t.Setenv("XDG_CONFIG_HOME", "/tmp/xdg-config") got, err := configPath() diff --git a/github.go b/github.go index 0736a6d..9912b09 100644 --- a/github.go +++ b/github.go @@ -1093,16 +1093,6 @@ func (c *GitHubClient) allCheckContexts( return nodes, nil } -func checkMayHaveUsefulAnnotations(check githubCheckContext) bool { - state := strings.ToUpper(firstNonEmpty(check.Conclusion, check.State, check.Status)) - switch state { - case "FAILURE", "ERROR", "CANCELLED", "TIMED_OUT", "ACTION_REQUIRED", "STALE": - return true - default: - return false - } -} - func (c *GitHubClient) checkAnnotations( ctx context.Context, checkID string, ) ([]githubCheckAnnotation, error) { @@ -1427,6 +1417,17 @@ func (c *GitHubClient) EnrichPullRequest( Owner: details.Owner, Repository: details.Repository, Number: details.Number, HeadOID: details.HeadOID, CheckAnnotations: make(map[string][]CheckAnnotation), } + type annotationJob struct { + index int + check Check + } + type annotationResult struct { + checkID string + annotations []CheckAnnotation + err error + } + var jobs []annotationJob + var annotations []annotationResult for _, check := range details.Checks { if check.ID == "" || !checkStateMayHaveUsefulAnnotations(check) { continue @@ -1435,37 +1436,69 @@ func (c *GitHubClient) EnrichPullRequest( result.CheckAnnotations[check.ID] = annotations continue } - nodes, err := c.checkAnnotations(ctx, check.ID) - if err != nil { - result.Issues = append(result.Issues, DataIssue{ - Component: "check annotations", Message: err.Error(), - }) - continue - } - annotations := make([]CheckAnnotation, 0, len(nodes)) - for _, annotation := range nodes { - annotations = append(annotations, CheckAnnotation{ - Path: annotation.Path, StartLine: annotation.Location.Start.Line, - EndLine: annotation.Location.End.Line, Level: annotation.AnnotationLevel, - Title: annotation.Title, Message: annotation.Message, - }) - } - c.storeAnnotations(check.ID, annotations) - result.CheckAnnotations[check.ID] = annotations + jobs = append(jobs, annotationJob{index: len(annotations), check: check}) + annotations = append(annotations, annotationResult{checkID: check.ID}) } + + var wait sync.WaitGroup + queue := make(chan annotationJob, len(jobs)) + for _, job := range jobs { + queue <- job + } + close(queue) + for range min(4, len(jobs)) { + wait.Add(1) + go func() { + defer wait.Done() + for job := range queue { + nodes, err := c.checkAnnotations(ctx, job.check.ID) + if err != nil { + annotations[job.index].err = err + continue + } + converted := make([]CheckAnnotation, 0, len(nodes)) + for _, annotation := range nodes { + converted = append(converted, CheckAnnotation{ + Path: annotation.Path, StartLine: annotation.Location.Start.Line, + EndLine: annotation.Location.End.Line, Level: annotation.AnnotationLevel, + Title: annotation.Title, Message: annotation.Message, + }) + } + c.storeAnnotations(job.check.ID, converted) + annotations[job.index].annotations = converted + } + }() + } + + var conflictFiles []string + var conflictErr error if details.Mergeable == "CONFLICTING" && c.conflicts != nil { - files, err := c.loadConflictFiles( - ctx, details.RepositoryURL, details.Number, details.BaseRef, - details.BaseOID, details.HeadOID, - ) - if err != nil { + wait.Add(1) + go func() { + defer wait.Done() + conflictFiles, conflictErr = c.loadConflictFiles( + ctx, details.RepositoryURL, details.Number, details.BaseRef, + details.BaseOID, details.HeadOID, + ) + }() + } + wait.Wait() + for _, loaded := range annotations { + if loaded.err != nil { result.Issues = append(result.Issues, DataIssue{ - Component: "conflict file scan", Message: err.Error(), + Component: "check annotations", Message: loaded.err.Error(), }) } else { - result.ConflictFiles = files + result.CheckAnnotations[loaded.checkID] = loaded.annotations } } + if conflictErr != nil { + result.Issues = append(result.Issues, DataIssue{ + Component: "conflict file scan", Message: conflictErr.Error(), + }) + } else if conflictFiles != nil { + result.ConflictFiles = conflictFiles + } level, summary := healthOK, "secondary PR data loaded" if len(result.Issues) > 0 { level, summary = healthWarning, fmt.Sprintf( diff --git a/github_test.go b/github_test.go index 6c0a462..f17a889 100644 --- a/github_test.go +++ b/github_test.go @@ -3,11 +3,13 @@ package main import ( "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "reflect" "strconv" "strings" + "sync/atomic" "testing" "time" ) @@ -406,6 +408,44 @@ func TestCheckContextsAndAnnotationsArePaginated(t *testing.T) { } } +func TestPullRequestEnrichmentBoundsConcurrentAnnotationRequests(t *testing.T) { + var active, maximum atomic.Int32 + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + var request graphQLRequest + if err := json.NewDecoder(r.Body).Decode(&request); err != nil { + t.Error(err) + return + } + current := active.Add(1) + defer active.Add(-1) + for observed := maximum.Load(); current > observed; observed = maximum.Load() { + if maximum.CompareAndSwap(observed, current) { + break + } + } + time.Sleep(20 * time.Millisecond) + _, _ = w.Write([]byte(`{"data":{"node":{"annotations":{ + "pageInfo":{"hasNextPage":false},"nodes":[] + }}}}`)) + })) + defer server.Close() + + client := NewGitHubClient(server.URL, "secret") + details := PRDetails{} + for index := range 6 { + details.Checks = append(details.Checks, Check{ + ID: fmt.Sprintf("check-%d", index), Conclusion: "FAILURE", + }) + } + result := client.EnrichPullRequest(context.Background(), details) + if len(result.CheckAnnotations) != len(details.Checks) || len(result.Issues) != 0 { + t.Fatalf("enrichment = %#v", result) + } + if got := maximum.Load(); got < 2 || got > 4 { + t.Fatalf("maximum concurrent annotation requests = %d, want 2..4", got) + } +} + func TestCheckQueriesUseCurrentGitHubSchemaShape(t *testing.T) { for name, query := range map[string]string{"annotations": checkAnnotationsPageQuery} { if strings.Contains(query, "output {") || diff --git a/go.mod b/go.mod index b313516..a9041e6 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,8 @@ require ( github.com/charmbracelet/glamour v1.0.0 github.com/charmbracelet/lipgloss v1.1.1-0.20250404203927-76690c660834 github.com/charmbracelet/x/ansi v0.10.2 + github.com/muesli/termenv v0.16.0 + github.com/rivo/uniseg v0.4.7 ) require ( @@ -29,8 +31,6 @@ require ( github.com/muesli/ansi v0.0.0-20230316100256-276c6243b2f6 // indirect github.com/muesli/cancelreader v0.2.2 // indirect github.com/muesli/reflow v0.3.0 // indirect - github.com/muesli/termenv v0.16.0 // indirect - github.com/rivo/uniseg v0.4.7 // indirect github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect github.com/yuin/goldmark v1.7.13 // indirect github.com/yuin/goldmark-emoji v1.0.6 // indirect diff --git a/highlight.go b/highlight.go index 4c8891b..ddbe13c 100644 --- a/highlight.go +++ b/highlight.go @@ -6,18 +6,67 @@ import ( "regexp" "strconv" "strings" + "sync" "github.com/alecthomas/chroma/v2/lexers" "github.com/alecthomas/chroma/v2/quick" ) const reviewContextLines = 3 +const highlightedDiffCacheLimit = 256 var codeHighlightTheme = "github-dark" var colorEnabled = true var hunkHeaderPattern = regexp.MustCompile(`^@@ -(\d+)(?:,\d+)? \+(\d+)(?:,\d+)? @@`) +type highlightedDiffCacheKey struct { + path, hunk, side, theme string + startLine, endLine int + color bool +} + +type highlightedDiffCache struct { + mu sync.Mutex + entries map[highlightedDiffCacheKey][]highlightedDiffLine + order []highlightedDiffCacheKey +} + +var highlightedDiffs = highlightedDiffCache{ + entries: make(map[highlightedDiffCacheKey][]highlightedDiffLine), +} + +func (c *highlightedDiffCache) get(key highlightedDiffCacheKey) ([]highlightedDiffLine, bool) { + c.mu.Lock() + defer c.mu.Unlock() + lines, ok := c.entries[key] + return append([]highlightedDiffLine(nil), lines...), ok +} + +func (c *highlightedDiffCache) put( + key highlightedDiffCacheKey, lines []highlightedDiffLine, +) []highlightedDiffLine { + c.mu.Lock() + defer c.mu.Unlock() + if cached, ok := c.entries[key]; ok { + return append([]highlightedDiffLine(nil), cached...) + } + if len(c.entries) >= highlightedDiffCacheLimit { + delete(c.entries, c.order[0]) + c.order = c.order[1:] + } + c.entries[key] = append([]highlightedDiffLine(nil), lines...) + c.order = append(c.order, key) + return append([]highlightedDiffLine(nil), lines...) +} + +func (c *highlightedDiffCache) clear() { + c.mu.Lock() + defer c.mu.Unlock() + c.entries = make(map[highlightedDiffCacheKey][]highlightedDiffLine) + c.order = nil +} + type highlightedDiffLine struct { gutter string code string @@ -34,6 +83,17 @@ type parsedDiffLine struct { } func highlightDiff(path, hunk string, startLine, endLine int, side string) []highlightedDiffLine { + key := highlightedDiffCacheKey{ + path: path, hunk: hunk, side: side, theme: codeHighlightTheme, + startLine: startLine, endLine: endLine, color: colorEnabled, + } + if cached, ok := highlightedDiffs.get(key); ok { + return cached + } + return highlightedDiffs.put(key, highlightDiffUncached(path, hunk, startLine, endLine, side)) +} + +func highlightDiffUncached(path, hunk string, startLine, endLine int, side string) []highlightedDiffLine { if hunk == "" { return []highlightedDiffLine{{code: "(GitHub did not return a diff hunk)"}} } diff --git a/highlight_test.go b/highlight_test.go index 69cffc1..0458c75 100644 --- a/highlight_test.go +++ b/highlight_test.go @@ -106,3 +106,17 @@ func TestHighlightDiffRemovesOnlyCommonIndent(t *testing.T) { t.Fatalf("dedented code:\n%q\nwant:\n%q", got, want) } } + +func TestHighlightDiffCacheReturnsIndependentSlices(t *testing.T) { + highlightedDiffs.clear() + + first := highlightDiff("main.go", "@@ -1 +1 @@\n-old\n+new", 1, 1, "RIGHT") + if len(first) == 0 { + t.Fatal("highlighted diff is empty") + } + first[0].code = "mutated" + second := highlightDiff("main.go", "@@ -1 +1 @@\n-old\n+new", 1, 1, "RIGHT") + if len(second) == 0 || second[0].code == "mutated" { + t.Fatalf("cached highlighted diff shares caller storage: %#v", second) + } +} diff --git a/main.go b/main.go index 10e8984..512fa3d 100644 --- a/main.go +++ b/main.go @@ -63,8 +63,7 @@ func main() { flag.Visit(func(item *flag.Flag) { visited[item.Name] = true }) config, err := loadConfig( *configFile, - visited["config"] || os.Getenv("DIPLE_CONFIG") != "" || - os.Getenv("GH_THREADS_CONFIG") != "", + visited["config"] || os.Getenv("DIPLE_CONFIG") != "", ) if err != nil { exitf("configuration: %v", err) diff --git a/pr_editor.go b/pr_editor.go index 981b36a..bc6cf3d 100644 --- a/pr_editor.go +++ b/pr_editor.go @@ -28,6 +28,7 @@ func (m *App) startPREdit() tea.Cmd { return nil } m.writeMode = writePREdit + m.prEditGeneration++ m.prEditField = prEditBodyField modal := m.editorMode == "vim" m.prEditEditors[prEditTitleField] = newTextEditor(m.details.Title, modal) @@ -71,12 +72,14 @@ func (m *App) loadPREditBranches() tea.Cmd { return nil } m.prEditBranchesLoading = true - owner, repo := m.details.Owner, m.details.Repository + owner, repo, generation := m.details.Owner, m.details.Repository, m.prEditGeneration return func() tea.Msg { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() branches, err := service.ListBranches(ctx, owner, repo) - return branchesLoadedMsg{owner: owner, repo: repo, branches: branches, err: err} + return branchesLoadedMsg{ + generation: generation, owner: owner, repo: repo, branches: branches, err: err, + } } } @@ -87,12 +90,14 @@ func (m *App) loadPREditUsers() tea.Cmd { return nil } m.prEditUsersLoading = true - owner, repo := m.details.Owner, m.details.Repository + owner, repo, generation := m.details.Owner, m.details.Repository, m.prEditGeneration return func() tea.Msg { ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) defer cancel() users, err := service.ListRepositoryUsers(ctx, owner, repo) - return repositoryUsersLoadedMsg{owner: owner, repo: repo, users: users, err: err} + return repositoryUsersLoadedMsg{ + generation: generation, owner: owner, repo: repo, users: users, err: err, + } } } diff --git a/text_editor.go b/text_editor.go index 764f053..599a998 100644 --- a/text_editor.go +++ b/text_editor.go @@ -617,10 +617,6 @@ func normalEditorLineLast(value string, cursor, wrapWidth int) int { return start } -func moveNormalCursorLine(value string, cursor, delta int) int { - return moveEditorCursorLine(value, cursor, delta, 0, true) -} - func nextWordStart(value string, cursor int, big bool) int { runes := []rune(value) cursor = clamp(cursor, 0, len(runes)) @@ -666,10 +662,6 @@ func previousWordStart(value string, cursor int, big bool) int { return cursor } -func wordEnd(value string, cursor int, big bool) int { - return wordEndAtWidth(value, cursor, big, 0) -} - func wordEndAtWidth(value string, cursor int, big bool, wrapWidth int) int { runes := []rune(value) cursor = clamp(cursor, 0, len(runes)) diff --git a/theme.go b/theme.go index 1d2b8a9..aa4a33a 100644 --- a/theme.go +++ b/theme.go @@ -103,6 +103,7 @@ func applyTheme(name string, custom ...CustomThemeConfig) error { commentMarkdownRenderers.Clear() commentMarkdownLines.clear() renderedSuggestions.clear() + highlightedDiffs.clear() return nil } diff --git a/tui.go b/tui.go index ef9597e..a3c3543 100644 --- a/tui.go +++ b/tui.go @@ -102,13 +102,15 @@ type detailsLoadedMsg struct { } type branchesLoadedMsg struct { - owner string - repo string - branches []RepositoryBranch - err error + generation uint64 + owner string + repo string + branches []RepositoryBranch + err error } type repositoryUsersLoadedMsg struct { + generation uint64 owner, repo string users []RepositoryUser err error @@ -171,6 +173,7 @@ type App struct { prEditUsersLoading bool prEditUsersError string prEditUserIndex int + prEditGeneration uint64 cursorOutput *terminalCursorOutput foldResolved bool @@ -209,6 +212,7 @@ type App struct { aiProgress AIRunProgress aiSpinner int aiEvents <-chan tea.Msg + aiGeneration uint64 difflet diffletModel mutations *mutationQueueStore mutationReplayBusy bool @@ -1049,7 +1053,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } return m, tea.Batch(m.startMutationReplay(), m.difflet.setState(diffletSuccess)) case branchesLoadedMsg: - if m.writeMode != writePREdit || + if msg.generation != m.prEditGeneration || m.writeMode != writePREdit || msg.owner != m.details.Owner || msg.repo != m.details.Repository { return m, nil } @@ -1065,7 +1069,7 @@ func (m App) Update(msg tea.Msg) (tea.Model, tea.Cmd) { m.prEditBranchIndex = 0 m.ensurePREditCursorVisible() case repositoryUsersLoadedMsg: - if m.writeMode != writePREdit || + if msg.generation != m.prEditGeneration || m.writeMode != writePREdit || msg.owner != m.details.Owner || msg.repo != m.details.Repository { return m, nil } diff --git a/tui_test.go b/tui_test.go index 6ac395d..7e25d0e 100644 --- a/tui_test.go +++ b/tui_test.go @@ -919,6 +919,63 @@ func TestPullRequestAIPreparationFailureIgnoresOldDiscussionThread(t *testing.T) } } +func TestAIStaleOperationMessagesAreIgnored(t *testing.T) { + m := NewApp(nil, "o", "r", false, 50, time.Second) + m.aiGeneration = 2 + m.aiMode = aiBusy + m.aiProgress = AIRunProgress{Stage: "current run"} + + updated, command, handled := m.updateAI(aiProgressMsg{ + generation: 1, progress: AIRunProgress{Stage: "stale run"}, + }) + m = updated.(App) + if !handled || command != nil || m.aiProgress.Stage != "current run" { + t.Fatalf("stale progress handled=%v command=%v progress=%q", + handled, command, m.aiProgress.Stage) + } + + updated, command, handled = m.updateAI(aiCompletedMsg{generation: 1}) + m = updated.(App) + if !handled || command != nil || m.aiMode != aiBusy { + t.Fatalf("stale completion handled=%v command=%v mode=%v", + handled, command, m.aiMode) + } + + m.aiMode = aiPreparing + updated, command, handled = m.updateAI(aiPreparedMsg{generation: 1}) + m = updated.(App) + if !handled || command != nil || m.aiMode != aiPreparing { + t.Fatalf("stale preparation handled=%v command=%v mode=%v", + handled, command, m.aiMode) + } +} + +func TestPREditStaleRecommendationMessagesAreIgnored(t *testing.T) { + m := NewApp(nil, "o", "r", false, 50, time.Second) + m.details = PRDetails{PullRequest: PullRequest{Owner: "o", Repository: "r"}} + m.writeMode = writePREdit + m.prEditGeneration = 2 + m.prEditBranches = []RepositoryBranch{{Name: "current"}} + + updated, command := m.Update(branchesLoadedMsg{ + generation: 1, owner: "o", repo: "r", + branches: []RepositoryBranch{{Name: "stale"}}, + }) + m = updated.(App) + if command != nil || len(m.prEditBranches) != 1 || m.prEditBranches[0].Name != "current" { + t.Fatalf("stale branches command=%v branches=%#v", command, m.prEditBranches) + } + + updated, command = m.Update(branchesLoadedMsg{ + generation: 2, owner: "o", repo: "r", + branches: []RepositoryBranch{{Name: "accepted"}}, + }) + m = updated.(App) + if command != nil || len(m.prEditBranches) != 1 || m.prEditBranches[0].Name != "accepted" { + t.Fatalf("current branches command=%v branches=%#v", command, m.prEditBranches) + } +} + func TestVimModeUsesGlobalFooterBar(t *testing.T) { m := NewApp(nil, "o", "r", false, 50, time.Second) m.width, m.height = 80, 10 diff --git a/version.go b/version.go index 8be30ba..20e5166 100644 --- a/version.go +++ b/version.go @@ -1,3 +1,3 @@ package main -const dipleVersion = "0.4.0" +const dipleVersion = "0.5.0"