Review fixes and a codebase pass for style, modern Go, and comments

README.md

@@ -96,6 +96,11 @@ See [herdr-reviewr] for the values and the action names.
 `e` opens the file at the cursor line in `editor`, else in `$VISUAL`, `$EDITOR`, or git's
 `core.editor`. A terminal editor, such as `nvim`, `micro`, or `fresh`, opens in a new pane
 beside rexdiff. A window editor, such as `code`, opens in its window.
+rexdiff opens an editor that it does not know in a pane. To open such an editor in a window,
+start it with `open`, for example `editor = "open -a Zed {file}"`.
+
+rexdiff gets `$VISUAL` and `$EDITOR` from your login shell. With zsh, set them in
+`~/.zprofile` or `~/.zshenv`, not in `~/.zshrc`.
 
 After a change:
 
@@ -105,7 +110,7 @@ After a change:
 ## Requirements
 
 - Rex
-- git
+- git 2.31 or later
 - `gh`, `glab`, or `az` for the pull request tab. You must be logged in.
 
 <!-- references -->

cmd/rexdiff/install.go

@@ -103,8 +103,8 @@ end
 -- open_rexdiff opens rexdiff at placement, beside the agent in ctx, with
 -- the settings in config. rexdiff starts in the current directory of the
 -- agent. The rex package of rexdiff finds the directory the same way.
--- rexdiff starts through the shell of the user, so that it gets the
--- environment of the user, for example EDITOR and PATH.
+-- rexdiff starts through the login shell of the user, so that it gets the
+-- login environment of the user, for example EDITOR and PATH.
 local function open_rexdiff(ctx, placement, config)
   local p = call("process", ctx.block_id)
   local cwd = (p.foreground or {}).cwd or (p.child or {}).cwd
@@ -208,9 +208,8 @@ func splitLua(path, content string) (settings, code string, err error) {
 		}
 		i += len(line)
 	}
-	return "", "", fmt.Errorf("%s has no line %q, so rexdiff cannot tell your "+
-		"settings from its code. Put that line back under your settings, or move "+
-		"the file away and run \"rexdiff install\" again", path, codeMark)
+	return "", "", fmt.Errorf("%s has no line %q, so rexdiff cannot tell your settings from its code. Put that line back under your settings, or move the file away and run \"rexdiff install\" again",
+		path, codeMark)
 }
 
 // newLua returns the content of rexdiff.lua with the code for bin. It
@@ -246,20 +245,14 @@ func executable() (string, error) {
 	}
 	// go run deletes its binary at exit, so the action cannot run it.
 	if strings.Contains(bin, string(filepath.Separator)+"go-build") {
-		return "", errors.New("install the binary first, for example with go install, " +
-			"then run rexdiff install")
+		return "", errors.New("install the binary first, for example with go install, then run rexdiff install")
 	}
 	return bin, nil
 }
 
 // install writes rexdiff.lua with the action that runs bin, the path of
 // rexdiff. It adds the require line to init.lua.
-func install(
-	ctx context.Context,
-	bin string,
-	args []string,
-	stdout, stderr io.Writer,
-) error {
+func install(ctx context.Context, bin string, args []string, stdout, stderr io.Writer) error {
 	fs := flag.NewFlagSet("rexdiff install", flag.ContinueOnError)
 	fs.SetOutput(stderr)
 	if err := fs.Parse(args); err != nil {
@@ -269,8 +262,8 @@ func install(
 		return err
 	}
 	if fs.NArg() > 0 {
-		return fmt.Errorf("unknown argument %s (set the key and the placement "+
-			"in the settings in %s)", fs.Arg(0), luaPath())
+		return fmt.Errorf("unknown argument %s (set the key and the placement in the settings in %s)",
+			fs.Arg(0), luaPath())
 	}
 
 	// Make the new content first. Then a bad rexdiff.lua stops the install
@@ -297,8 +290,7 @@ func install(
 	fmt.Fprintf(stdout, "wrote %s; the settings are at its top\n", lua)
 	fmt.Fprintf(stdout, "added %s to %s\n", requireLine, initPath)
 	fmt.Fprint(stdout, reload(ctx, stdout))
-	fmt.Fprintln(stdout, "press the key of the settings (shift+cmd+r by default), "+
-		"or run \"Toggle rexdiff\" from the command palette, beside an agent")
+	fmt.Fprintln(stdout, "press the key of the settings (shift+cmd+r by default), or run \"Toggle rexdiff\" from the command palette, beside an agent")
 	return nil
 }
 
@@ -380,7 +372,7 @@ func removeRequire(path string) error {
 }
 
 // reload tells Rex to read its configuration file again, and returns the
-// report of Rex. If Rex fails, reload tells the user to reload by hand and
+// report of Rex. If Rex fails, reload tells the user how to reload, and
 // returns "".
 func reload(ctx context.Context, stdout io.Writer) string {
 	out, err := rex.ReloadConfig(ctx)

cmd/rexdiff/main.go

@@ -96,7 +96,7 @@ func run(ctx context.Context, args []string, stdout, stderr io.Writer) error {
 	}
 	var updater tui.Updater
 	if u := update.Detect(bin, version); u != nil {
-		updater = u // a nil *update.Updater would make a non-nil interface
+		updater = u // an interface with a nil *update.Updater is not nil
 	}
 	resume, err := tui.Run(ctx, tui.Options{
 		Repo:    repo,
@@ -168,14 +168,10 @@ func parseFlags(args []string, stderr io.Writer) (flags, error) {
 	fs.Func("scope", "start scope: uncommitted, branch, or last-turn",
 		oneOf(&f.scope, tui.ScopeUncommitted, tui.ScopeBranch, tui.ScopeLastTurn))
 	fs.BoolVar(&f.wrap, "wrap", true, "wrap long lines; w toggles it")
-	fs.StringVar(&f.base, "base", "",
-		"base for the branch scope (default: the repo's default branch)")
-	fs.StringVar(&f.agent, "agent", os.Getenv("REX_AGENT_BLOCK"),
-		"the agent's Rex block (id or label) this pane was opened beside")
-	fs.StringVar(&f.logPath, "log", os.Getenv("REXDIFF_LOG"),
-		"append a debug log to this file")
-	fs.StringVar(&f.config, "config", "",
-		"the settings as JSON; the Rex action passes them from rexdiff.lua")
+	fs.StringVar(&f.base, "base", "", "base for the branch scope (default: the repo's default branch)")
+	fs.StringVar(&f.agent, "agent", os.Getenv("REX_AGENT_BLOCK"), "the agent's Rex block (id or label) this pane was opened beside")
+	fs.StringVar(&f.logPath, "log", os.Getenv("REXDIFF_LOG"), "append a debug log to this file")
+	fs.StringVar(&f.config, "config", "", "the settings as JSON; the Rex action passes them from rexdiff.lua")
 	fs.BoolVar(&f.check, "check", false, "check the settings of -config and exit")
 	fs.BoolVar(&f.version, "version", false, "print the version and exit")
 	fs.Usage = func() {

cmd/rexdiff/script_test.go

@@ -48,18 +48,19 @@ var userEnv = []string{
 	"REXDIFF_RESUME",
 }
 
-// TestScripts runs the scripts in testdata/script. Each script runs in a
-// new directory, $WORK. $WORK is also $HOME, so rexdiff does not see the
-// files of the user. If the real rex tool is installed, $REALREX is its
-// path, and the condition [realrex] is true.
+// TestScripts runs the scripts in testdata/script.
 func TestScripts(t *testing.T) {
+	// If the real rex tool is installed, $REALREX is its path, and the
+	// condition [realrex] is true.
 	realRex := filepath.Join(os.Getenv("REX_BIN_DIR"), "rex")
 	_, err := os.Stat(realRex)
 	hasRex := os.Getenv("REX_BIN_DIR") != "" && err == nil
 	testscript.Run(t, testscript.Params{
 		Dir:                 "testdata/script",
 		RequireExplicitExec: true,
 		Setup: func(env *testscript.Env) error {
+			// Each script runs in a new directory, $WORK. $WORK is also
+			// $HOME, so rexdiff does not see the files of the user.
 			env.Setenv("HOME", env.WorkDir)
 			env.Setenv("XDG_CONFIG_HOME", "")
 			env.Setenv("REALREX", realRex)

cmd/rexdiff/testdata/script/install.txt

@@ -12,8 +12,7 @@ grep '^local settings = \{$' .config/rex/rexdiff.lua
 grep '^-- rexdiff code below. Install and upgrades replace it. Do not edit this line.$' .config/rex/rexdiff.lua
 
 # The action starts rexdiff through the shell of the user, so that
-# rexdiff gets EDITOR, VISUAL, and PATH. With shell "none", it gets only
-# the environment of the Rex server.
+# rexdiff gets EDITOR, VISUAL, and PATH. A "shell" option can stop that.
 ! grep 'shell = ' .config/rex/rexdiff.lua
 
 # Rex accepts the action and its key.

diff/comment_test.go

@@ -34,6 +34,8 @@ func review(c diff.Comment) string {
 	return cs.Review("")
 }
 
+// TestReviewMatchesHerdr checks that Review writes the text format of
+// herdr-reviewr.
 func TestReviewMatchesHerdr(t *testing.T) {
 	var cs diff.Comments
 	cs.Add(comment("c.go", diff.SideNew,
@@ -63,8 +65,7 @@ func TestReviewMatchesHerdr(t *testing.T) {
 	}
 }
 
-// TestReviewContinuedRows checks a comment on only continued rows. The
-// comment has no diff lines, so Review writes the lines of its range.
+// TestReviewContinuedRows checks a comment on only continued rows.
 func TestReviewContinuedRows(t *testing.T) {
 	old, new := "", strings.Repeat("word ", 20)+"\n"
 	d := diff.Render("a.go", old, new, diff.Options{Width: 30, Wrap: true})
@@ -76,12 +77,14 @@ func TestReviewContinuedRows(t *testing.T) {
 	}
 	c := diff.NewComment("a.go", diff.SideNew, cont, old, new)
 	c.Text = "long"
+	// The comment has no diff lines, so Review writes the lines of its range.
 	want := "a.go:1\n+" + strings.TrimSuffix(new, "\n") + "\nlong"
 	if got := review(c); len(cont) == 0 || got != want {
 		t.Errorf("%d continued rows, Review:\n%s\nwant:\n%s", len(cont), got, want)
 	}
 }
 
+// TestComments checks Add, Find, At, EndingAt, Count, and Remove.
 func TestComments(t *testing.T) {
 	var cs diff.Comments
 	a := cs.Add(diff.Comment{Path: "a.go", Side: diff.SideNew, Start: 2, End: 4})
@@ -170,6 +173,8 @@ func TestNewComment(t *testing.T) {
 	}
 }
 
+// TestRelocateFollowsMovedCode checks that a comment moves to the nearest copy
+// of its lines, or becomes stale.
 func TestRelocateFollowsMovedCode(t *testing.T) {
 	var cs diff.Comments
 	c := cs.Add(commentOn("a.go", diff.SideNew, "a\nb\nc\n", 2, 3))
@@ -201,8 +206,8 @@ func TestRelocateFollowsMovedCode(t *testing.T) {
 	}
 }
 
-// TestSnippetIsTheRealDiff checks that the snippet is the real diff, with
-// raw text and tabs, in both layouts. The snippet must match git diff.
+// TestSnippetIsTheRealDiff checks that the snippet matches git diff, with raw
+// text and tabs, in both layouts.
 func TestSnippetIsTheRealDiff(t *testing.T) {
 	old := "func a() {\n\treturn 1\n}\n"
 	new := "func a() {\n\t// two\n\treturn 2\n}\n"

diff/diff.go

@@ -19,7 +19,7 @@ import (
 type Kind int
 
 const (
-	KindHunk   Kind = iota // a hunk header. It shows no source line.
+	KindHunk   Kind = iota // a hunk header, with no source line
 	KindEqual              // a line that is the same on the two sides
 	KindInsert             // a line that only the new side has
 	KindDelete             // a line that only the old side has
@@ -49,8 +49,8 @@ type Row struct {
 }
 
 // Fold is a run of equal lines that hides. Old and New are the line
-// numbers of its first line. Lines is the number of lines, 0 if no line
-// hides.
+// numbers of its first line. Lines is the number of lines. It is 0 if no
+// line hides.
 type Fold struct {
 	Old, New, Lines int
 }
@@ -75,9 +75,9 @@ type Options struct {
 	// Wrap continues a line that is wider than Width on the next lines,
 	// instead of cutting it. Then Offset does not apply.
 	Wrap bool
-	// Show tells if the lines of a fold show. If Show is nil, no fold
-	// shows. RenderFile does not use it.
-	Show func(Fold) bool
+	// Expanded tells if the lines of a fold show. If Expanded is nil, no
+	// fold shows. RenderFile does not use it.
+	Expanded func(Fold) bool
 	// Signs marks a changed line with + or -. Without Signs, a bar in the
 	// colour of the line marks it.
 	Signs bool
@@ -110,8 +110,8 @@ func Render(path, old, new string, o Options) *Diff {
 		Width(o.Width).
 		TabWidth(tabWidth).
 		XOffset(o.Offset)
-	if o.Show != nil {
-		dv = dv.Show(func(f diffview.Fold) bool { return o.Show(Fold{f.Before, f.After, f.Lines}) })
+	if o.Expanded != nil {
+		dv = dv.Expand(func(f diffview.Fold) bool { return o.Expanded(foldOf(f)) })
 	}
 	if o.Split {
 		dv = dv.Split()
@@ -132,7 +132,7 @@ func Render(path, old, new string, o Options) *Diff {
 			Old:       r.Before,
 			New:       r.After,
 			Continued: r.Continued,
-			Fold:      Fold{r.Fold.Before, r.Fold.After, r.Fold.Lines},
+			Fold:      foldOf(r.Fold),
 		})
 	}
 	if len(d.Rows) > 0 {
@@ -142,6 +142,10 @@ func Render(path, old, new string, o Options) *Diff {
 	return d
 }
 
+func foldOf(f diffview.Fold) Fold {
+	return Fold{Old: f.Before, New: f.After, Lines: f.Lines}
+}
+
 func kindOf(k diffview.RowKind) Kind {
 	switch k {
 	case diffview.RowHunk: