[codex] refactor for antislopus linter

cmd/diffnotes/main.go

@@ -14,21 +14,54 @@ var version = "dev"
 
 const defaultCommitLimit = 10
 
+var (
+	repoPathFlag    = flag.String("repo", ".", "path inside the git repository to review")
+	commitLimitFlag = flag.Int("commits", defaultCommitLimit, "number of recent commits to show in the sidebar")
+	showVersionFlag = flag.Bool("version", false, "print version and exit")
+)
+
 func main() {
-	repoPath := flag.String("repo", ".", "path inside the git repository to review")
-	commitLimit := flag.Int("commits", defaultCommitLimit, "number of recent commits to show in the sidebar")
-	showVersion := flag.Bool("version", false, "print version and exit")
+	if err := run(); err != nil {
+		fmt.Fprintf(os.Stderr, "diffnotes: %v\n", err)
+		os.Exit(1)
+	}
+}
+
+func run() error {
+	options := parseOptions()
+	if options.showVersion {
+		printVersion()
+		return nil
+	}
+
+	return startProgram(options.repoPath, options.commitLimit)
+}
+
+type cliOptions struct {
+	repoPath    string
+	commitLimit int
+	showVersion bool
+}
+
+func parseOptions() cliOptions {
 	flag.Parse()
 
-	if *showVersion {
-		fmt.Println(version)
-		return
+	return cliOptions{
+		repoPath:    *repoPathFlag,
+		commitLimit: *commitLimitFlag,
+		showVersion: *showVersionFlag,
 	}
+}
 
-	model := tui.NewModel(*repoPath, *commitLimit)
+func printVersion() {
+	fmt.Println(version)
+}
+
+func startProgram(repoPath string, commitLimit int) error {
+	model := tui.NewModel(repoPath, commitLimit)
 	program := tea.NewProgram(model, tea.WithAltScreen(), tea.WithMouseCellMotion())
 	if _, err := program.Run(); err != nil {
-		fmt.Fprintf(os.Stderr, "diffnotes: %v\n", err)
-		os.Exit(1)
+		return err
 	}
+	return nil
 }

internal/clipboard/clipboard.go

@@ -31,14 +31,21 @@ const (
 // system clipboard, which is the behavior users expect from an SSH session.
 func Write(text string) (string, error) {
 	mode := clipboardMode()
-	if mode == clipboardModeOSC52 || shouldPreferOSC52() {
-		if err := writeOSC52(text); err == nil {
-			return "OSC 52 terminal clipboard", nil
-		} else if mode == clipboardModeOSC52 {
-			return "", fmt.Errorf("OSC 52 clipboard copy failed: %w", err)
-		}
+	if copied, err := writePreferredOSC52(text, mode); copied || err != nil {
+		return osc52Result(err)
 	}
 
+	return writeWithNativeFallback(text, mode)
+}
+
+func osc52Result(err error) (string, error) {
+	if err != nil {
+		return "", err
+	}
+	return "OSC 52 terminal clipboard", nil
+}
+
+func writeWithNativeFallback(text string, mode string) (string, error) {
 	tool, err := writeNative(text)
 	if err == nil {
 		return tool, nil
@@ -54,34 +61,67 @@ func Write(text string) (string, error) {
 	return "", err
 }
 
+func writePreferredOSC52(text string, mode string) (bool, error) {
+	if mode != clipboardModeOSC52 && !shouldPreferOSC52() {
+		return false, nil
+	}
+	if err := writeOSC52(text); err != nil {
+		return false, forcedOSC52Error(mode, err)
+	}
+	return true, nil
+}
+
+func forcedOSC52Error(mode string, err error) error {
+	if mode != clipboardModeOSC52 {
+		return nil
+	}
+	return fmt.Errorf("OSC 52 clipboard copy failed: %w", err)
+}
+
 func writeNative(text string) (string, error) {
 	for _, tool := range candidates() {
-		path, err := exec.LookPath(tool.name)
-		if err != nil {
-			continue
+		if tryNativeTool(tool, text) {
+			return toolLabel(tool), nil
 		}
+	}
 
-		cmd := exec.Command(path, tool.args...)
-		cmd.Stdin = strings.NewReader(text)
-		if err := cmd.Run(); err != nil {
-			continue
-		}
-		return toolLabel(tool), nil
+	return "", nativeClipboardError()
+}
+
+func tryNativeTool(tool candidate, text string) bool {
+	path, err := exec.LookPath(tool.name)
+	if err != nil {
+		return false
 	}
 
-	switch runtime.GOOS {
-	case "linux":
-		if isSSHSession() {
-			return "", fmt.Errorf("no remote Linux clipboard command found and OSC 52 failed; enable OSC 52 clipboard access in your local terminal")
-		}
-		return "", fmt.Errorf("no Linux clipboard command found; install wl-clipboard, xclip, or xsel, or set %s=osc52", clipboardModeEnv)
-	case "darwin":
-		return "", fmt.Errorf("pbcopy was not found")
-	case "windows":
-		return "", fmt.Errorf("clip was not found")
-	default:
-		return "", fmt.Errorf("no clipboard command configured for %s", runtime.GOOS)
+	cmd := exec.Command(path, tool.args...)
+	cmd.Stdin = strings.NewReader(text)
+	return cmd.Run() == nil
+}
+
+func nativeClipboardError() error {
+	if runtime.GOOS == "linux" {
+		return linuxClipboardError()
+	}
+	return nativeClipboardErrorByOS(runtime.GOOS)
+}
+
+func nativeClipboardErrorByOS(goos string) error {
+	messages := map[string]string{
+		"darwin":  "pbcopy was not found",
+		"windows": "clip was not found",
 	}
+	if message, ok := messages[goos]; ok {
+		return fmt.Errorf("%s", message)
+	}
+	return fmt.Errorf("no clipboard command configured for %s", goos)
+}
+
+func linuxClipboardError() error {
+	if isSSHSession() {
+		return fmt.Errorf("no remote Linux clipboard command found and OSC 52 failed; enable OSC 52 clipboard access in your local terminal")
+	}
+	return fmt.Errorf("no Linux clipboard command found; install wl-clipboard, xclip, or xsel, or set %s=osc52", clipboardModeEnv)
 }
 
 func writeOSC52(text string) error {
@@ -138,22 +178,29 @@ func candidates() []candidate {
 	case "darwin":
 		return []candidate{{name: "pbcopy"}}
 	case "linux":
-		var out []candidate
-		if os.Getenv("WAYLAND_DISPLAY") != "" {
-			out = append(out, candidate{name: "wl-copy"})
-		}
-		out = append(out,
-			candidate{name: "xclip", args: []string{"-selection", "clipboard"}},
-			candidate{name: "xsel", args: []string{"--clipboard", "--input"}},
-		)
-		return out
+		return linuxCandidates()
 	case "windows":
 		return []candidate{{name: "clip"}}
 	default:
 		return nil
 	}
 }
 
+func linuxCandidates() []candidate {
+	out := linuxWaylandCandidates()
+	return append(out,
+		candidate{name: "xclip", args: []string{"-selection", "clipboard"}},
+		candidate{name: "xsel", args: []string{"--clipboard", "--input"}},
+	)
+}
+
+func linuxWaylandCandidates() []candidate {
+	if os.Getenv("WAYLAND_DISPLAY") == "" {
+		return nil
+	}
+	return []candidate{{name: "wl-copy"}}
+}
+
 func toolLabel(tool candidate) string {
 	if len(tool.args) == 0 {
 		return tool.name

internal/comments/comments.go

@@ -7,15 +7,23 @@ import (
 )
 
 type Note struct {
-	ID             string
+	ID string
+	NoteSource
+	NoteLocation
+	Message string
+	Code    string
+}
+
+type NoteSource struct {
 	SourceID       string
 	SourceTitle    string
 	SourceSubtitle string
-	File           string
-	Side           string
-	Line           int
-	Message        string
-	Code           string
+}
+
+type NoteLocation struct {
+	File string
+	Side string
+	Line int
 }
 
 type Store struct {
@@ -59,62 +67,91 @@ func (s *Store) List() []Note {
 	for _, note := range s.notes {
 		out = append(out, note)
 	}
-	sort.Slice(out, func(i, j int) bool {
-		a, b := out[i], out[j]
-		if a.SourceTitle != b.SourceTitle {
-			return a.SourceTitle < b.SourceTitle
-		}
-		if a.File != b.File {
-			return a.File < b.File
-		}
-		if a.Line != b.Line {
-			return a.Line < b.Line
-		}
-		return a.Side < b.Side
-	})
+	sort.Slice(out, func(i, j int) bool { return before(out[i], out[j]) })
 	return out
 }
 
+func before(a Note, b Note) bool {
+	if a.SourceTitle != b.SourceTitle {
+		return a.SourceTitle < b.SourceTitle
+	}
+	return beforeInSource(a, b)
+}
+
+func beforeInSource(a Note, b Note) bool {
+	if a.File != b.File {
+		return a.File < b.File
+	}
+	if a.Line != b.Line {
+		return a.Line < b.Line
+	}
+	return a.Side < b.Side
+}
+
 func Format(notes []Note) string {
 	var b strings.Builder
 	b.WriteString("Review comments for coding agent\n")
+	writeNotes(&b, notes)
 
+	return strings.TrimRight(b.String(), "\n") + "\n"
+}
+
+func writeNotes(b *strings.Builder, notes []Note) {
 	lastSource := ""
 	for _, note := range notes {
-		if note.SourceTitle != lastSource {
-			if lastSource != "" {
-				b.WriteByte('\n')
-			}
-			b.WriteString("\nSource: ")
-			b.WriteString(note.SourceTitle)
-			if note.SourceSubtitle != "" {
-				b.WriteString(" (")
-				b.WriteString(note.SourceSubtitle)
-				b.WriteString(")")
-			}
-			b.WriteByte('\n')
-			lastSource = note.SourceTitle
-		}
-
-		b.WriteString("- ")
-		b.WriteString(note.File)
-		b.WriteByte(':')
-		b.WriteString(fmt.Sprintf("%d", note.Line))
-		if note.Side != "" {
-			b.WriteString(" [")
-			b.WriteString(note.Side)
-			b.WriteByte(']')
-		}
-		b.WriteByte('\n')
-		b.WriteString("  Message: ")
-		b.WriteString(note.Message)
+		lastSource = writeSourceIfChanged(b, note, lastSource)
+		writeNote(b, note)
+	}
+}
+
+func writeSourceIfChanged(b *strings.Builder, note Note, lastSource string) string {
+	if note.SourceTitle == lastSource {
+		return lastSource
+	}
+	writeSourceHeader(b, note, lastSource != "")
+	return note.SourceTitle
+}
+
+func writeSourceHeader(b *strings.Builder, note Note, withGap bool) {
+	if withGap {
 		b.WriteByte('\n')
-		if note.Code != "" {
-			b.WriteString("  Code: ")
-			b.WriteString(strings.TrimSpace(note.Code))
-			b.WriteByte('\n')
-		}
 	}
+	b.WriteString("\nSource: ")
+	b.WriteString(note.SourceTitle)
+	if note.SourceSubtitle != "" {
+		b.WriteString(" (")
+		b.WriteString(note.SourceSubtitle)
+		b.WriteString(")")
+	}
+	b.WriteByte('\n')
+}
 
-	return strings.TrimRight(b.String(), "\n") + "\n"
+func writeNote(b *strings.Builder, note Note) {
+	writeNoteLocation(b, note)
+	b.WriteString("  Message: ")
+	b.WriteString(note.Message)
+	b.WriteByte('\n')
+	writeNoteCode(b, note.Code)
+}
+
+func writeNoteLocation(b *strings.Builder, note Note) {
+	b.WriteString("- ")
+	b.WriteString(note.File)
+	b.WriteByte(':')
+	b.WriteString(fmt.Sprintf("%d", note.Line))
+	if note.Side != "" {
+		b.WriteString(" [")
+		b.WriteString(note.Side)
+		b.WriteByte(']')
+	}
+	b.WriteByte('\n')
+}
+
+func writeNoteCode(b *strings.Builder, code string) {
+	if code == "" {
+		return
+	}
+	b.WriteString("  Code: ")
+	b.WriteString(strings.TrimSpace(code))
+	b.WriteByte('\n')
 }