diff --git a/README.md b/README.md index 358b032..fcf0fed 100644 --- a/README.md +++ b/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. diff --git a/cmd/rexdiff/install.go b/cmd/rexdiff/install.go index 6c1b584..9e8094c 100644 --- a/cmd/rexdiff/install.go +++ b/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) diff --git a/cmd/rexdiff/main.go b/cmd/rexdiff/main.go index 7315881..584039d 100644 --- a/cmd/rexdiff/main.go +++ b/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() { diff --git a/cmd/rexdiff/script_test.go b/cmd/rexdiff/script_test.go index a1b924e..fb4b519 100644 --- a/cmd/rexdiff/script_test.go +++ b/cmd/rexdiff/script_test.go @@ -48,11 +48,10 @@ 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 @@ -60,6 +59,8 @@ func TestScripts(t *testing.T) { 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) diff --git a/cmd/rexdiff/testdata/script/install.txt b/cmd/rexdiff/testdata/script/install.txt index 7b49654..1880108 100644 --- a/cmd/rexdiff/testdata/script/install.txt +++ b/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 --git a/diff/comment_test.go b/diff/comment_test.go index b47002e..11dd157 100644 --- a/diff/comment_test.go +++ b/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 --git a/diff/diff.go b/diff/diff.go index e3d3610..1050453 100644 --- a/diff/diff.go +++ b/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: diff --git a/diff/diff_test.go b/diff/diff_test.go index 03470ec..f21da29 100644 --- a/diff/diff_test.go +++ b/diff/diff_test.go @@ -38,6 +38,8 @@ func kinds(rows []diff.Row) string { return b.String() } +// TestRenderRowsFollowLines checks the row of each line, in each layout and +// theme. func TestRenderRowsFollowLines(t *testing.T) { const width = 80 themes := []diff.Theme{ @@ -99,6 +101,7 @@ func TestRenderRowsFollowLines(t *testing.T) { } } +// TestRenderSameContentIsEmpty checks that two equal versions give no lines. func TestRenderSameContentIsEmpty(t *testing.T) { d := diff.Render("x.go", oldSrc, oldSrc, diff.Options{Width: 80}) if len(d.Lines) != 0 || len(d.Rows) != 0 { @@ -106,6 +109,8 @@ func TestRenderSameContentIsEmpty(t *testing.T) { } } +// TestRenderTabsAndOffset checks that a tab becomes spaces and that Offset +// scrolls the code. func TestRenderTabsAndOffset(t *testing.T) { d := diff.Render("x.go", "", "\tx := 1\n", diff.Options{Width: 80}) if got := ansi.Strip(d.Lines[1]); !strings.Contains(got, "▌ x := 1") { @@ -118,12 +123,13 @@ func TestRenderTabsAndOffset(t *testing.T) { } } -// TestMarks checks the mark before the code of each line, in the two -// layouts. By default a bar marks a changed line, as in herdr-reviewr. -// [diff.Options.Signs] gives + and -. The mark takes two cells in both -// cases, so the columns do not move. +// TestMarks checks the mark at the start of the code of each line, in the two +// layouts. func TestMarks(t *testing.T) { const old, new = "a\nb\nc\n", "a\nB\nc\n" + // By default a bar marks a changed line, as in herdr-reviewr. Signs gives + // + and -. The mark takes two cells in both cases, so the columns do not + // move. theme := diff.Theme{ Background: color.Black, Green: color.RGBA{0, 0xff, 0, 0xff}, @@ -163,22 +169,13 @@ func TestMarks(t *testing.T) { t.Errorf("%s: line %d = %q, want %q", tt.name, i+1, got, want) } } - // The mark has the colour of the line: ANSI red for a delete, and - // ANSI green for an insert. - for _, c := range []struct { - line int - fg string - }{{2, "31"}, {3, "32"}} { - if tt.o.Split && c.line == 3 { - continue // the split layout has the insert on the same row - } - if got := markColour(d.Lines[c.line]); got != c.fg { - t.Errorf("%s: the mark of line %d has colour %q, want %q", tt.name, c.line, got, c.fg) - } + // The marks have the colour of their lines: ANSI red for the + // delete, then ANSI green for the insert. + if got := markColours(d.Lines[1:]); !slices.Equal(got, []string{"31", "32"}) { + t.Errorf("%s: the marks have colours %q, want 31 and 32", tt.name, got) } } - // With a scroll to the right, the bar keeps its colour and shows an - // ellipsis after it. + // With a scroll to the right, an ellipsis shows after the bar. d := diff.Render("x.txt", "", "abcdefgh\n", diff.Options{Width: 40, Offset: 2}) if got := ansi.Strip(d.Lines[1]); !strings.Contains(got, "▌…cdefgh") { t.Errorf("offset: line = %q, want ▌…cdefgh", got) @@ -193,18 +190,28 @@ func TestMarks(t *testing.T) { } } -// markColour returns the foreground of the SGR sequence just before the -// first bar, + or - in line, for example "31". -func markColour(line string) string { - i := strings.IndexAny(line, "▌+-") - if i < 0 { - return "" +// markColours returns the foreground of each bar, + or - in lines, in +// order, for example "31". The foreground is in the SGR sequence in front +// of the mark. +func markColours(lines []string) []string { + var out []string + for _, l := range lines { + for { + i := strings.IndexAny(l, "▌+-") + if i < 0 { + break + } + sgr := l[strings.LastIndex(l[:i], "\x1b[")+2 : i] + fg, _, _ := strings.Cut(strings.TrimSuffix(sgr, "m"), ";") + out = append(out, fg) + l = l[i+1:] + } } - sgr := line[strings.LastIndex(line[:i], "\x1b[")+2 : i] - fg, _, _ := strings.Cut(strings.TrimSuffix(sgr, "m"), ";") - return fg + return out } +// TestSideAt checks the side of each column, and if the column is on a line +// number. func TestSideAt(t *testing.T) { // The unified layout has " 1 " old numbers, " 1 " new numbers, and the // code. @@ -240,6 +247,7 @@ func TestSideAt(t *testing.T) { } } +// TestCount checks the numbers of added and deleted lines. func TestCount(t *testing.T) { tests := []struct { old, new string @@ -259,6 +267,7 @@ func TestCount(t *testing.T) { } } +// TestRenderFile checks the line numbers and the rows of a file. func TestRenderFile(t *testing.T) { src := "package x\n\nfunc f() {\n\treturn\n}\n" d := diff.RenderFile("x.go", src, diff.Options{Width: 14}) @@ -281,9 +290,8 @@ func TestRenderFile(t *testing.T) { } } -// TestWrap wraps a long line. All of its text shows, and each screen line -// fits the width. The continued rows point at the same source lines, and -// the snippet of a comment does not change. +// TestWrap checks that a wrapped long line shows all of its text and fits the +// width. func TestWrap(t *testing.T) { long := "x := []string{" + strings.Repeat(`"word", `, 12) + "}" old, new := "a\nb\n", "a\n"+long+"\nb\n" @@ -320,6 +328,7 @@ func TestWrap(t *testing.T) { if r.New == 2 { text.WriteString(ansi.Strip(l)) } + // A continued row points at the same source lines as the row above. prev := d.Rows[max(i-1, 0)] sameLines := prev.Old == r.Old && prev.New == r.New if r.Continued && !sameLines { @@ -332,6 +341,7 @@ func TestWrap(t *testing.T) { t.Errorf("%s: the wrapped lines do not show all of the line:\n%s", name, text.String()) } + // Wrap does not change the snippet of a comment. got, want := snippet(d.Rows, old, new), snippet(cut.Rows, old, new) if !slices.Equal(got, want) { t.Errorf("%s: snippet with wrap %q, without %q", name, got, want) diff --git a/diff/file.go b/diff/file.go index b02aaae..1d53ab2 100644 --- a/diff/file.go +++ b/diff/file.go @@ -2,6 +2,7 @@ package diff import ( "fmt" + "strconv" "strings" "charm.land/lipgloss/v2" @@ -27,7 +28,7 @@ func RenderFile(path, src string, o Options) *Diff { ).Replace(src) hl := strings.Split(highlight(path, src), "\n") hl = hl[:min(len(hl), len(Lines(src)))] // chroma can add a line break at the end - digits := len(fmt.Sprint(len(hl))) + digits := len(strconv.Itoa(len(hl))) numWidth := digits + 2 // a space on each side of the number numStyle := lipgloss.NewStyle().Foreground(lipgloss.Color(lineNumberColor)) d := &Diff{newNum: numWidth} @@ -57,8 +58,8 @@ func RenderFile(path, src string, o Options) *Diff { // fileLineParts returns the screen lines of the highlighted line l. If o has // a width, each part fits in code cells. With [Options.Wrap], a long line -// gives more than one part. Else the line skips [Options.Offset] cells and -// is cut. +// gives more than one part. Without Wrap, the line skips [Options.Offset] +// cells and is cut. func fileLineParts(l string, code int, o Options) []string { switch { case o.Width > 0 && code <= 0: diff --git a/diff/fold_test.go b/diff/fold_test.go index d6c420c..9f4b3b5 100644 --- a/diff/fold_test.go +++ b/diff/fold_test.go @@ -11,29 +11,30 @@ import ( "github.com/rednafi/rexdiff/diff" ) -// numbered returns lines "line 1" to "line n", with the lines in upper -// case changed to "LINE n". +// numbered returns lines "line 1" to "line n". The lines in upper are +// "LINE 1" and so on. func numbered(n int, upper ...int) string { var b strings.Builder for i := 1; i <= n; i++ { word := "line" - for _, u := range upper { - if u == i { - word = "LINE" - } + if slices.Contains(upper, i) { + word = "LINE" } fmt.Fprintf(&b, "%s %d\n", word, i) } return b.String() } +// foldText writes a hunk header with fold f, for example "@14/14+13". +func foldText(f diff.Fold) string { return fmt.Sprintf("@%d/%d+%d", f.Old, f.New, f.Lines) } + // layout writes the rows of d as one entry each: "@" and its fold for a // hunk header, else the old and the new line number. func layout(d *diff.Diff) []string { var out []string for _, r := range d.Rows { if r.Kind == diff.KindHunk { - out = append(out, fmt.Sprintf("@%d/%d+%d", r.Fold.Old, r.Fold.New, r.Fold.Lines)) + out = append(out, foldText(r.Fold)) continue } out = append(out, fmt.Sprintf("%d:%d", r.Old, r.New)) @@ -41,12 +42,22 @@ func layout(d *diff.Diff) []string { return out } +// headers returns the text of the hunk headers of d. +func headers(d *diff.Diff) string { + var out []string + for i, r := range d.Rows { + if r.Kind == diff.KindHunk { + out = append(out, strings.TrimSpace(ansi.Strip(d.Lines[i]))) + } + } + return strings.Join(out, "\n") +} + // TestFolds checks that a long run of equal lines folds into the hunk -// header below it. Three lines show beside each change. A fold that Show -// accepts shows all its lines. +// header below it. func TestFolds(t *testing.T) { old, new := numbered(40), numbered(40, 10, 30) - fold := func(old, new, n int) string { return fmt.Sprintf("@%d/%d+%d", old, new, n) } + header := func(o, n, lines int) []string { return []string{foldText(diff.Fold{Old: o, New: n, Lines: lines})} } lines := func(lo, hi int) []string { var out []string for i := lo; i <= hi; i++ { @@ -55,88 +66,79 @@ func TestFolds(t *testing.T) { return out } change := func(n int) []string { return []string{fmt.Sprintf("%d:0", n), fmt.Sprintf("0:%d", n)} } - join := func(parts ...[]string) []string { - var out []string - for _, p := range parts { - out = append(out, p...) - } - return out - } - top, middle, tail := fold(1, 1, 6), fold(14, 14, 13), fold(34, 34, 7) + // Three lines show beside each change. A fold that Expanded accepts shows + // all of its lines. + top, middle, tail, none := header(1, 1, 6), header(14, 14, 13), header(34, 34, 7), header(0, 0, 0) tests := []struct { - name string - show func(diff.Fold) bool - want []string + name string + expanded func(diff.Fold) bool + want []string }{ - {"all folded", nil, join( - []string{top}, lines(7, 9), change(10), lines(11, 13), - []string{middle}, lines(27, 29), change(30), lines(31, 33), - []string{tail}, + {"all folded", nil, slices.Concat( + top, lines(7, 9), change(10), lines(11, 13), + middle, lines(27, 29), change(30), lines(31, 33), + tail, )}, - {"middle shows", func(f diff.Fold) bool { return f.New == 14 }, join( - []string{top}, lines(7, 9), change(10), lines(11, 29), change(30), lines(31, 33), - []string{tail}, + {"middle shows", func(f diff.Fold) bool { return f.New == 14 }, slices.Concat( + top, lines(7, 9), change(10), lines(11, 29), change(30), lines(31, 33), + tail, )}, - {"top and tail show", func(f diff.Fold) bool { return f.New != 14 }, join( - []string{fold(0, 0, 0)}, lines(1, 9), change(10), lines(11, 13), - []string{middle}, lines(27, 29), change(30), lines(31, 40), + {"top and tail show", func(f diff.Fold) bool { return f.New != 14 }, slices.Concat( + none, lines(1, 9), change(10), lines(11, 13), + middle, lines(27, 29), change(30), lines(31, 40), )}, - {"all show", func(diff.Fold) bool { return true }, join( - []string{fold(0, 0, 0)}, lines(1, 9), change(10), lines(11, 29), change(30), lines(31, 40), + {"all show", func(diff.Fold) bool { return true }, slices.Concat( + none, lines(1, 9), change(10), lines(11, 29), change(30), lines(31, 40), )}, } for _, tt := range tests { - d := diff.Render("f.txt", old, new, diff.Options{Width: 60, Show: tt.show}) - if got := layout(d); strings.Join(got, " ") != strings.Join(tt.want, " ") { + d := diff.Render("f.txt", old, new, diff.Options{Width: 60, Expanded: tt.expanded}) + if got := layout(d); !slices.Equal(got, tt.want) { t.Errorf("%s:\n got %v\nwant %v", tt.name, got, tt.want) } } // The header tells how many lines hide. A fold at the end has no hunk // header. In the split layout, the header goes on across the right - // side, and no line is wider than the screen. - headers := func(o diff.Options) string { - d := diff.Render("f.txt", old, new, o) - var out []string - for i, r := range d.Rows { - if w := ansi.StringWidth(d.Lines[i]); w > o.Width { - t.Errorf("split %v: line %d is %d cells wide", o.Split, i, w) - } - if r.Kind == diff.KindHunk { - out = append(out, strings.TrimSpace(ansi.Strip(d.Lines[i]))) + // side. No line is wider than the screen. + for _, tt := range []struct { + o diff.Options + want string + }{ + {diff.Options{Width: 60}, "… … ⋯ 6 unmodified lines @@ -7,7 +7,7 @@\n" + + "… … ⋯ 13 unmodified lines @@ -27,7 +27,7 @@\n" + + "… … ⋯ 7 unmodified lines"}, + {diff.Options{Width: 60, Split: true}, "… ⋯ 6 unmodified lines @@ -7,7 +7,7 @@\n" + + "… ⋯ 13 unmodified lines @@ -27,7 +27,7 @@\n" + + "… ⋯ 7 unmodified lines"}, + } { + d := diff.Render("f.txt", old, new, tt.o) + if got := headers(d); got != tt.want { + t.Errorf("split %v: headers:\n%s\nwant:\n%s", tt.o.Split, got, tt.want) + } + for i, l := range d.Lines { + if w := ansi.StringWidth(l); w > tt.o.Width { + t.Errorf("split %v: line %d is %d cells wide", tt.o.Split, i, w) } } - return strings.Join(out, "\n") - } - want := "… … ⋯ 6 unmodified lines @@ -7,7 +7,7 @@\n" + - "… … ⋯ 13 unmodified lines @@ -27,7 +27,7 @@\n" + - "… … ⋯ 7 unmodified lines" - if got := headers(diff.Options{Width: 60}); got != want { - t.Errorf("headers:\n%s\nwant:\n%s", got, want) - } - want = "… ⋯ 6 unmodified lines @@ -7,7 +7,7 @@\n" + - "… ⋯ 13 unmodified lines @@ -27,7 +27,7 @@\n" + - "… ⋯ 7 unmodified lines" - if got := headers(diff.Options{Width: 60, Split: true}); got != want { - t.Errorf("split headers:\n%s\nwant:\n%s", got, want) } } -// TestFoldEdges checks the folds that show no line. One line between the -// margins of two changes shows, because a fold of one line saves no row. -// After an insert, the old and the new numbers of a fold differ. A file -// without equal lines has no fold. +// TestFoldEdges checks the folds that show no line. func TestFoldEdges(t *testing.T) { tests := []struct { name string old, new string want string }{ - // Changes at 10 and 18 leave line 14 alone between the margins. + // Changes at 10 and 18 leave line 14 alone between the margins. The + // line shows, because a fold of one line saves no row. {"one line", numbered(21), numbered(21, 10, 18), "@1/1+6 7:7 8:8 9:9 10:0 0:10 11:11 12:12 13:13 14:14 15:15 16:16 17:17 18:0 0:18 19:19 20:20 21:21"}, + // After an insert, the old and the new numbers of a fold differ. {"insert at the top", numbered(12), "new\n" + numbered(12), "@0/0+0 0:1 1:2 2:3 3:4 @4/5+9"}, + // A file without equal lines has no fold. {"all new", "", "a\nb\n", "@0/0+0 0:1 0:2"}, } for _, tt := range tests { diff --git a/diff/fuzz_test.go b/diff/fuzz_test.go index e5b7fb7..05d9c28 100644 --- a/diff/fuzz_test.go +++ b/diff/fuzz_test.go @@ -105,6 +105,8 @@ func checkRows(t *testing.T, rows []diff.Row, old, new string) { } } +// FuzzRender checks the invariants of Render, and that Count agrees with the +// snippet. func FuzzRender(f *testing.F) { for i, s := range hostile { f.Add(uint8(i), "a\nb\n", s, uint8(i), uint8(i*7), uint8(i%3), i%2 == 0) @@ -154,6 +156,8 @@ func FuzzRender(f *testing.F) { }) } +// FuzzSnippetSameInLayouts checks that the split and the unified layouts give +// the same snippet. func FuzzSnippetSameInLayouts(f *testing.F) { f.Add(oldSrc, newSrc) f.Add("a\nb\nc\nd\n", "x\nb\ny\nz\nd\n") @@ -169,6 +173,7 @@ func FuzzSnippetSameInLayouts(f *testing.F) { }) } +// FuzzRenderFile checks that RenderFile gives one row for each source line. func FuzzRenderFile(f *testing.F) { for i, s := range hostile { f.Add(uint8(i), s, uint8(i), uint8(i*7), uint8(i%3), i%2 == 0) @@ -198,6 +203,7 @@ func FuzzRenderFile(f *testing.F) { }) } +// FuzzPlain checks that Plain and Quote remove all unsafe control characters. func FuzzPlain(f *testing.F) { for _, s := range hostile { f.Add(s) @@ -217,6 +223,8 @@ func FuzzPlain(f *testing.F) { }) } +// FuzzCount checks that the counts of Count are in the range of the line +// counts. func FuzzCount(f *testing.F) { for _, s := range hostile { f.Add(s, "a\n") @@ -239,6 +247,8 @@ func FuzzCount(f *testing.F) { }) } +// FuzzComments checks that Relocate keeps a valid range, and that the review +// text has no control characters. func FuzzComments(f *testing.F) { for i, s := range hostile { f.Add("a\nb\nc\n", s, "a.go", s, uint8(i), uint8(i%3)) diff --git a/diff/hostile_test.go b/diff/hostile_test.go index 09fd7bf..bd9498d 100644 --- a/diff/hostile_test.go +++ b/diff/hostile_test.go @@ -10,9 +10,8 @@ import ( "github.com/rednafi/rexdiff/diff" ) -// TestHostileContent renders each hostile file in all layouts. No control -// character from the file can reach the terminal, and the rows must point -// at real lines. +// TestHostileContent checks the lines and the rows of each hostile file, in +// all layouts. func TestHostileContent(t *testing.T) { opts := []diff.Options{ {}, @@ -93,6 +92,8 @@ func TestTabToSpacesIsAChange(t *testing.T) { } } +// TestRenderFileLineCount checks the number of rows for each kind of line +// ending. func TestRenderFileLineCount(t *testing.T) { tests := []struct { src string @@ -115,6 +116,7 @@ func TestRenderFileLineCount(t *testing.T) { } } +// TestRenderFileNarrow checks that wide text fits a width of 1 to 7 cells. func TestRenderFileNarrow(t *testing.T) { for w := 1; w < 8; w++ { o := diff.Options{Width: w, Wrap: true} @@ -122,6 +124,7 @@ func TestRenderFileNarrow(t *testing.T) { } } +// TestQuote checks that Quote quotes only the paths that are not safe. func TestQuote(t *testing.T) { tests := []struct{ in, want string }{ {"a.go", "a.go"}, @@ -137,6 +140,8 @@ func TestQuote(t *testing.T) { } } +// TestReviewHostileComment checks that Review quotes a hostile path and +// removes the control characters from the code and the text. func TestReviewHostileComment(t *testing.T) { c := comment("a\x1b]52;c;aGk=\x07.go", diff.SideNew, "", @@ -149,8 +154,8 @@ func TestReviewHostileComment(t *testing.T) { } } -// TestBlackTint renders a theme whose tint of deleted lines is black. An -// equal line with the same text as a deleted line has no background. +// TestBlackTint checks that an equal line with the text of a deleted line has +// no background when the tint of deleted lines is black. func TestBlackTint(t *testing.T) { black := color.RGBA{A: 255} th := diff.Theme{Background: black, Foreground: color.White, Red: black} diff --git a/internal/VENDORED.md b/internal/VENDORED.md index 86d1146..89fca48 100644 --- a/internal/VENDORED.md +++ b/internal/VENDORED.md @@ -31,10 +31,10 @@ `Height`, `YOffset`, `InfiniteYScroll` (and the height cut and the scroll code); `MatchLexer`, `LineStyle` and the `Style` fields are unexported; `RowFile` removed - - a coloured bar `▌` marks a changed line, in place of crush's `+` and `-`; `Signs` gives - the signs back + - a coloured bar `▌` marks a changed line, in place of crush's `+` and `-`; `Signs` shows + `+` and `-` in place of the bar - `fold.go` (added): the diff has the whole file in one hunk, and `foldHunk` folds the - long runs of equal lines, as in herdr-reviewr; `Show` shows a fold, and `Row.Fold` tells + long runs of equal lines, as in herdr-reviewr; `Expand` shows a fold, and `Row.Fold` tells which lines a hunk header hides - the syntax cache key tells "no background" from black, so a black tint does not leak onto equal lines diff --git a/internal/diffview/diffview.go b/internal/diffview/diffview.go index e29339e..8387f48 100644 --- a/internal/diffview/diffview.go +++ b/internal/diffview/diffview.go @@ -23,9 +23,8 @@ const ( ellipsis = "…" // shows where a line is cut ) -// The symbols before the code of a line. By default, a bar in the colour -// of the line marks an inserted or a deleted line. [View.Signs] changes the -// bar to a sign. +// The symbols at the start of the code of a line. A bar in the colour of the line +// marks each changed line. [View.Signs] shows + and - in place of the bar. const ( symNone = " " // an equal line, a missing line, or a continued line symBar = "▌ " @@ -61,7 +60,7 @@ type View struct { err error unified udiff.UnifiedDiff folds []Fold // folds[i] hides above unified.Hunks[i] - show func(Fold) bool // the folds that show, see [View.Show] + expanded func(Fold) bool // the folds that show, see [View.Expand] splitHunks []splitHunk @@ -195,8 +194,8 @@ func (v *View) replaceTabs() { } } -// computeDiff computes the diff of the two versions once. Then it returns -// the error of the first call. +// computeDiff computes the diff of the two versions once. A later call +// returns the error of the first call. func (v *View) computeDiff() error { if v.isComputed { return v.err @@ -213,8 +212,12 @@ func (v *View) computeDiff() error { edits, context, ) - if v.err == nil && len(v.unified.Hunks) == 1 { - v.unified.Hunks, v.folds = foldHunk(v.unified.Hunks[0], v.show) + hunks := v.unified.Hunks + v.unified.Hunks, v.folds = nil, nil + for _, h := range hunks { + hs, fs := foldHunk(h, v.expanded) + v.unified.Hunks = append(v.unified.Hunks, hs...) + v.folds = append(v.folds, fs...) } return v.err } @@ -278,7 +281,7 @@ func codeWidthOf(l *udiff.Line) int { func (v *View) detectUnifiedCodeWidth() { v.codeWidth = 0 for i, h := range v.unified.Hunks { - v.codeWidth = max(v.codeWidth, ansi.StringWidth(v.header(i))) + v.codeWidth = max(v.codeWidth, ansi.StringWidth(v.headerText(i))) for _, l := range h.Lines { v.codeWidth = max(v.codeWidth, codeWidthOf(&l)) } @@ -290,7 +293,7 @@ func (v *View) detectUnifiedCodeWidth() { func (v *View) detectSplitCodeWidth() { v.codeWidth = 0 for i, h := range v.splitHunks { - v.codeWidth = max(v.codeWidth, ansi.StringWidth(v.header(i))) + v.codeWidth = max(v.codeWidth, ansi.StringWidth(v.headerText(i))) for _, l := range h.lines { if l.before != nil { v.codeWidth = max(v.codeWidth, codeWidthOf(l.before)) @@ -355,7 +358,7 @@ func (v *View) renderUnified() string { // unifiedHeader renders the divider line of hunk i in the unified layout. func (v *View) unifiedHeader(i int) string { div := v.style.dividerLine - header := ansi.Truncate(v.header(i), v.fullCodeWidth, ellipsis) + header := ansi.Truncate(v.headerText(i), v.fullCodeWidth, ellipsis) return div.lineNumber.Render(pad(ellipsis, v.beforeNumDigits)) + div.lineNumber.Render(pad(ellipsis, v.afterNumDigits)) + div.code.Width(v.fullCodeWidth).Render(header) @@ -394,9 +397,10 @@ func (v *View) renderSplit() string { // header starts on the left side and can go on across the right side. func (v *View) splitHeader(i int) string { div := v.style.dividerLine + // The header covers the two code columns and the new line numbers. width := 2*v.fullCodeWidth + btoi(v.extraColOnAfter) + v.afterNumDigits + lineNumPadding*2 return div.lineNumber.Render(pad(ellipsis, v.beforeNumDigits)) + - div.code.Width(width).Render(ansi.Truncate(v.header(i), width, ellipsis)) + div.code.Width(width).Render(ansi.Truncate(v.headerText(i), width, ellipsis)) } // splitSide renders line l on one side of the split layout, one string for @@ -433,19 +437,21 @@ func fillTo(lines []string, n int, fill func() string) []string { func (v *View) kindStyle(k udiff.OpKind) (lineStyle, string) { switch k { case udiff.Insert: - if v.signs { - return v.style.insertLine, symInsert - } - return v.style.insertLine, symBar + return v.style.insertLine, v.mark(symInsert) case udiff.Delete: - if v.signs { - return v.style.deleteLine, symDelete - } - return v.style.deleteLine, symBar + return v.style.deleteLine, v.mark(symDelete) } return v.style.equalLine, symNone } +// mark returns sign if the View shows signs, else the bar. +func (v *View) mark(sign string) string { + if v.signs { + return sign + } + return symBar +} + // writeRow writes the screen lines of one row to b, and keeps their number // for rows. func (v *View) writeRow(b *strings.Builder, lines []string) { @@ -482,8 +488,7 @@ func (v *View) codeLines( code = ansi.GraphemeWidth.Cut(code, v.xOffset, len(code)) parts[0] = ansi.Truncate(code, width, ellipsis) if v.xOffset > 0 && strings.TrimSpace(parts[0]) != "" { - sym, _ = strings.CutSuffix(sym, " ") - sym += ellipsis + sym = strings.TrimSuffix(sym, " ") + ellipsis } } full := lipgloss.NewStyle().MaxWidth(width + leadingSymbolsSize) diff --git a/internal/diffview/escape.go b/internal/diffview/escape.go index a9f16b1..c865025 100644 --- a/internal/diffview/escape.go +++ b/internal/diffview/escape.go @@ -10,7 +10,7 @@ import ( // The Unicode Control Pictures that Escape shows in place of a control // character. const ( - controlPictures = '\u2400' // the picture of U+0000; U+0001 to U+001F follow + controlPictures = '\u2400' // the picture of U+0000, then the pictures of U+0001 to U+001F delPicture = '\u2421' ) diff --git a/internal/diffview/fold.go b/internal/diffview/fold.go index 62e3272..6f7319e 100644 --- a/internal/diffview/fold.go +++ b/internal/diffview/fold.go @@ -11,24 +11,25 @@ import ( const foldMargin = 3 // Fold is a run of equal lines that hides. Before and After 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 { Before, After, Lines int } -// Show shows the lines of each fold for which show returns true. -func (v *View) Show(show func(Fold) bool) *View { - v.show = show +// Expand shows the lines of each fold for which expanded returns true. +func (v *View) Expand(expanded func(Fold) bool) *View { + v.expanded = expanded + v.isComputed = false return v } -// foldHunk splits h, the hunk of the whole file, into the hunks that show. -// folds[i] is the run of lines that hides above hunks[i]. If the last run -// of equal lines hides, the last hunk has no lines. A run hides if more -// than one of its lines is farther than foldMargin from a change, and show -// is nil or returns false for it. -func foldHunk(h *udiff.Hunk, show func(Fold) bool) (hunks []*udiff.Hunk, folds []Fold) { +// foldHunk splits h into the hunks that show. folds[i] is the run of lines +// that hides above hunks[i]. If the last run hides, the last hunk has no +// lines. A run of equal lines hides if it has more than one line and no +// line of it is within foldMargin of a change. A run does not hide if +// expanded returns true for it. +func foldHunk(h *udiff.Hunk, expanded func(Fold) bool) (hunks []*udiff.Hunk, folds []Fold) { n := len(h.Lines) keep := make([]bool, n) for i, l := range h.Lines { @@ -41,14 +42,15 @@ func foldHunk(h *udiff.Hunk, show func(Fold) bool) (hunks []*udiff.Hunk, folds [ var cur *udiff.Hunk var above Fold b, a := h.FromLine, h.ToLine - for i := 0; i < n; { + i := 0 + for i < n { if !keep[i] { j := i for j < n && !keep[j] { j++ } f := Fold{Before: b, After: a, Lines: j - i} - if f.Lines > 1 && (show == nil || !show(f)) { + if f.Lines > 1 && (expanded == nil || !expanded(f)) { above, cur = f, nil b, a, i = b+f.Lines, a+f.Lines, j continue @@ -80,10 +82,11 @@ func foldHunk(h *udiff.Hunk, show func(Fold) bool) (hunks []*udiff.Hunk, folds [ return hunks, folds } -// header returns the header line of hunk i: the fold above it, if lines -// hide, and the hunk header, if the hunk has lines. For example -// " ⋯ 12 unmodified lines @@ -15,7 +15,8 @@ ". -func (v *View) header(i int) string { +// headerText returns the text of the header of hunk i, for example +// " ⋯ 12 unmodified lines @@ -15,7 +15,8 @@ ". The text has the fold +// above the hunk if lines hide. It has the hunk header if the hunk has +// lines. The text ends with a space, as the hunk header does. +func (v *View) headerText(i int) string { h, s := v.unified.Hunks[i], "" if f := v.folds[i]; f.Lines > 0 { s = fmt.Sprintf(" ⋯ %d unmodified lines", f.Lines) diff --git a/internal/diffview/rows.go b/internal/diffview/rows.go index 5b7a7cc..27b3b2e 100644 --- a/internal/diffview/rows.go +++ b/internal/diffview/rows.go @@ -5,6 +5,7 @@ import "github.com/aymanbagabas/go-udiff" // RowKind is the type of a rendered row. type RowKind int +// The values of RowKind. const ( RowHunk RowKind = iota // a hunk header RowEqual // a line that is the same in the two versions @@ -119,7 +120,8 @@ func splitKind(l *splitLine) RowKind { return RowInsert } -// Geometry gives the columns of the line numbers in the output of String. +// Geometry gives the columns of the line numbers in the output of +// [View.Render]. // Use it to tell a line number from the code. type Geometry struct { BeforeNum int // the width of the old line-number column, in cells diff --git a/internal/diffview/util.go b/internal/diffview/util.go index 3c388e6..188f3b5 100644 --- a/internal/diffview/util.go +++ b/internal/diffview/util.go @@ -9,7 +9,7 @@ import ( // pad right-aligns v in a column of width cells. func pad(v any, width int) string { - s := fmt.Sprintf("%v", v) + s := fmt.Sprint(v) w := ansi.StringWidth(s) if w >= width { return s diff --git a/rex/rex.go b/rex/rex.go index 115823d..f94eb55 100644 --- a/rex/rex.go +++ b/rex/rex.go @@ -45,10 +45,10 @@ func FromEnv() *Client { } } -// binFromEnv returns the path of the rex command. Rex sets REX_BIN_DIR -// in its terminals. Other programs, for example brew, do not have it. -// Then the path is "" for rex in PATH. If rex is not in PATH, the path -// is the rex of the Rex server app on macOS. +// binFromEnv returns the path of the rex command, or "" for the rex in PATH. +// Rex sets REX_BIN_DIR in its terminals. Other programs, for example brew, +// do not have it. If REX_BIN_DIR is empty and PATH has no rex, binFromEnv +// returns the rex of the Rex server app on macOS, if it is there. func binFromEnv() string { if dir := os.Getenv("REX_BIN_DIR"); dir != "" { return filepath.Join(dir, "rex") @@ -60,8 +60,7 @@ func binFromEnv() string { if err != nil { return "" } - bin := filepath.Join(dir, "com.superlogical.rex", "RexServer", "Versions", - "Current", "Rex Server.app", "Contents", "MacOS", "rex") + bin := filepath.Join(dir, "com.superlogical.rex", "RexServer", "Versions", "Current", "Rex Server.app", "Contents", "MacOS", "rex") if _, err := os.Stat(bin); err != nil { return "" } diff --git a/rex/rex_test.go b/rex/rex_test.go index 549353b..8abe6ff 100644 --- a/rex/rex_test.go +++ b/rex/rex_test.go @@ -216,19 +216,18 @@ func TestFromEnv(t *testing.T) { } // TestReloadConfigOutsideRex checks that ReloadConfig runs the rex of the -// Rex server app outside a Rex terminal. There, for example under brew, -// REX_BIN_DIR is not set and rex is not in PATH. +// Rex server app outside a Rex terminal. func TestReloadConfigOutsideRex(t *testing.T) { if runtime.GOOS != "darwin" { t.Skip("the Rex server app is on macOS") } + // Outside a Rex terminal, for example under brew, REX_BIN_DIR is not set + // and rex is not in PATH. home := t.TempDir() t.Setenv("HOME", home) t.Setenv("REX_BIN_DIR", "") t.Setenv("PATH", t.TempDir()) - dir := filepath.Join(home, "Library", "Application Support", - "com.superlogical.rex", "RexServer", "Versions", "Current", - "Rex Server.app", "Contents", "MacOS") + dir := filepath.Join(home, "Library", "Application Support", "com.superlogical.rex", "RexServer", "Versions", "Current", "Rex Server.app", "Contents", "MacOS") if err := os.MkdirAll(dir, 0o755); err != nil { t.Fatal(err) } diff --git a/tui/bench_test.go b/tui/bench_test.go index ce71da4..166f6d2 100644 --- a/tui/bench_test.go +++ b/tui/bench_test.go @@ -29,8 +29,7 @@ func BenchmarkReview(b *testing.B) { for b.Loop() { m.reload() for range 100 { - next, _ := m.Update(down) - m = *next.(*model) + m = step(b, m, down) _ = m.View() } } diff --git a/tui/config.go b/tui/config.go index 3e68dc1..b1e0403 100644 --- a/tui/config.go +++ b/tui/config.go @@ -55,8 +55,8 @@ var ( ) // config holds the settings of rexdiff. The settings are at the top of -// rexdiff.lua, beside the init.lua of Rex. The toggle action gives them to -// rexdiff as JSON in --config. +// rexdiff.lua, beside the configuration file (init.lua) of Rex. The toggle +// action gives them to rexdiff as JSON in --config. type config struct { Layout Layout `json:"layout"` DefaultScope Scope `json:"default_scope"` @@ -166,8 +166,8 @@ func parseConfig(text string) (config, error) { if text == "" { return c, nil } - // set tells which keys the text has. A blank text that the user sets - // is an error. A text that the user does not set is not an error. + // set tells which keys the text has. A blank text is an error only if + // the user sets it. var set map[string]jsontext.Value if err := json.Unmarshal([]byte(text), &set); err != nil { return config{}, configError(err) @@ -318,6 +318,8 @@ func (c config) checkKeybindings() error { return nil } +// invalid returns the error for a value of the setting key that rexdiff +// does not accept. want tells the values that rexdiff accepts. func invalid(key, want, value string) error { return fmt.Errorf("invalid value for %s: expected %s, not %q", key, want, value) } diff --git a/tui/editor.go b/tui/editor.go index b6fefa4..79884a1 100644 --- a/tui/editor.go +++ b/tui/editor.go @@ -8,6 +8,7 @@ import ( "os/exec" "path/filepath" "runtime" + "slices" "strconv" "strings" "syscall" @@ -34,13 +35,15 @@ type dialect struct { } // dialects has the editors that rexdiff knows, by program name. The table -// is the one of herdr-reviewr, with fresh added. -var dialects = map[string]dialect{} +// is the same as in herdr-reviewr. The table also lists fresh, open, and +// xdg-open. +var dialects = makeDialects() -func init() { - add := func(d dialect, names ...string) { +func makeDialects() map[string]dialect { + d := map[string]dialect{} + add := func(dl dialect, names ...string) { for _, n := range names { - dialects[n] = d + d[n] = dl } } add(dialect{argPlus, false}, "vi", "vim", "nvim", "lvim", "vis", "joe", "nano", "micro", "kak") @@ -54,69 +57,82 @@ func init() { add(dialect{argSuffix, true}, "subl", "sublime_text", "zed", "zeditor", "zedit") add(dialect{argFlag, true}, "mate", "kate", "idea", "pycharm", "webstorm", "goland", "clion", "phpstorm", "rubymine", "rider", "datagrip", "rustrover", "dataspell", "fleet") + add(dialect{argNone, true}, "open", "xdg-open") if runtime.GOOS == "darwin" { add(dialect{argFlag, true}, "xed") // the opener of Xcode } + return d +} + +// dialectOf returns the dialect of the program of command. An editor that +// rexdiff does not know takes +line and draws in the terminal. Most +// terminal editors take +line. A window editor also works in a pane. A +// wait flag tells that the editor has a window. +func dialectOf(command string) dialect { + d := dialect{line: argPlus} + if known, ok := dialects[filepath.Base(program(command))]; ok { + d = known + } + words := strings.Fields(command) + if slices.Contains(words, "--wait") || slices.Contains(words, "--block") { + d.window = true + } + return d +} + +// program returns the first word of command. A word in quotes can have +// spaces, for example "/Applications/Visual Studio Code.app/.../code". +func program(command string) string { + command = strings.TrimSpace(command) + for _, q := range []string{`"`, "'"} { + if rest, ok := strings.CutPrefix(command, q); ok { + word, _, _ := strings.Cut(rest, q) + return word + } + } + word, _, _ := strings.Cut(command, " ") + return word } // editorCommand is a command that opens a file at a line. type editorCommand struct { - script string // the command, for sh -c - terminal bool // the editor draws in the terminal + script string // the command, for sh -c + window bool // the editor opens a window, not in the terminal } -// resolveEditor returns the command that opens the file abs at line. It -// uses template, the editor of the settings, else value, the editor of -// the environment or of git. A template gets abs in place of {file} and -// line in place of {line}. For value, rexdiff adds abs and line as the -// editor expects them. -func resolveEditor(template, value, abs string, line int) editorCommand { - if template != "" { - r := strings.NewReplacer("{file}", shellQuote(abs), "{line}", strconv.Itoa(line)) - script := r.Replace(template) - if !strings.Contains(template, "{file}") { - script += " " + shellQuote(abs) - } - return editorCommand{script, wantsTerminal(template)} +// templateCommand returns the command that opens the file abs at line with +// template, the editor of the settings. The template gets abs in place of +// {file}, and line in place of {line}. Without {file}, abs goes at the end. +func templateCommand(template, abs string, line int) editorCommand { + r := strings.NewReplacer("{file}", shellQuote(abs), "{line}", strconv.Itoa(line)) + script := r.Replace(template) + if !strings.Contains(template, "{file}") { + script += " " + shellQuote(abs) } + return editorCommand{script, dialectOf(template).window} +} + +// envCommand returns the command that opens the file abs at line with +// editor, the editor of the environment or of git. rexdiff adds abs and +// line in the form that the editor expects. +func envCommand(editor, abs string, line int) editorCommand { + d := dialectOf(editor) n, file := strconv.Itoa(line), shellQuote(abs) + at := shellQuote(abs + ":" + n) var args string - switch dialects[program(value)].line { + switch d.line { case argNone: args = file case argPlus: args = "+" + n + " " + file case argSuffix: - args = shellQuote(abs + ":" + n) + args = at case argGoto: - args = "-g " + shellQuote(abs+":"+n) + args = "-g " + at case argFlag: args = "--line " + n + " " + file } - return editorCommand{value + " " + args, wantsTerminal(value)} -} - -// program returns the name of the program of the command, without its -// directory. -func program(command string) string { - words := strings.Fields(command) - if len(words) == 0 { - return "" - } - return filepath.Base(words[0]) -} - -// wantsTerminal reports whether the command draws in the terminal. A wait -// flag tells that the editor has a window. An editor that rexdiff does not -// know draws in the terminal, because in a pane a window editor also works. -func wantsTerminal(command string) bool { - for _, w := range strings.Fields(command) { - if w == "--wait" || w == "--block" { - return false - } - } - d, ok := dialects[program(command)] - return !ok || !d.window + return editorCommand{editor + " " + args, d.window} } // openEditor opens the file at path, at line. It uses the editor of the @@ -125,18 +141,20 @@ func wantsTerminal(command string) bool { // screen until it stops. A window editor starts in the background. func (m *model) openEditor(path string, line int) tea.Cmd { abs := filepath.Join(m.repo.Root(), path) - value := "" - if m.cfg.Editor == "" { - value = cmp.Or( + var ed editorCommand + if m.cfg.Editor != "" { + ed = templateCommand(m.cfg.Editor, abs, line) + } else { + editor := cmp.Or( strings.TrimSpace(os.Getenv("VISUAL")), strings.TrimSpace(os.Getenv("EDITOR")), - strings.TrimSpace(m.repo.Editor(m.ctx)), + m.repo.Editor(m.ctx), "vi", ) + ed = envCommand(editor, abs, line) } - ed := resolveEditor(m.cfg.Editor, value, abs, line) - m.log.Debug("editor", "command", ed.script, "terminal", ed.terminal) - if !ed.terminal { + m.log.Debug("editor", "command", ed.script, "window", ed.window) + if ed.window { return runWindowEditor(ed.script) } if m.rex != nil { @@ -151,8 +169,9 @@ func (m *model) openEditor(path string, line int) tea.Cmd { } // runWindowEditor starts a window editor. The editor gets its own session -// and no terminal, so that it cannot read the keys of rexdiff. The command -// reports an error if the editor fails, with the last line of its stderr. +// and no terminal, so that it cannot read the keys of rexdiff. If the +// editor fails, the returned command gives the error and the last line of +// the stderr of the editor. func runWindowEditor(script string) tea.Cmd { cmd := exec.Command("sh", "-c", script) var stderr bytes.Buffer diff --git a/tui/editor_test.go b/tui/editor_test.go index ee164c5..9261b64 100644 --- a/tui/editor_test.go +++ b/tui/editor_test.go @@ -5,64 +5,88 @@ import ( "path/filepath" "strings" "testing" - "time" tea "charm.land/bubbletea/v2" "github.com/rednafi/rexdiff/rex" ) -// TestResolveEditor checks the command and the kind of each editor. -func TestResolveEditor(t *testing.T) { +// TestEditorCommands checks the command and the kind of each editor. +func TestEditorCommands(t *testing.T) { const abs = "/r/a b.go" tests := []struct { - template, value string - script string - terminal bool + template, editor string + script string + window bool }{ // The editor of the settings gets the placeholders. - {"/opt/homebrew/bin/nvim +{line} {file}", "", `/opt/homebrew/bin/nvim +12 '/r/a b.go'`, true}, - {"code -g {file}:{line}", "", `code -g '/r/a b.go':12`, false}, - {"zed {file}:{line}", "", `zed '/r/a b.go':12`, false}, - {"myeditor --at {line}", "", `myeditor --at 12 '/r/a b.go'`, true}, + {"/opt/homebrew/bin/nvim +{line} {file}", "", `/opt/homebrew/bin/nvim +12 '/r/a b.go'`, false}, + {"code -g {file}:{line}", "", `code -g '/r/a b.go':12`, true}, + {"zed {file}:{line}", "", `zed '/r/a b.go':12`, true}, + {"open -a Zed {file}", "", `open -a Zed '/r/a b.go'`, true}, + {"myeditor --at {line}", "", `myeditor --at 12 '/r/a b.go'`, false}, // The editor of the environment gets the line as it expects it. - {"", "nvim", `nvim +12 '/r/a b.go'`, true}, - {"", "/usr/bin/vim -u NONE", `/usr/bin/vim -u NONE +12 '/r/a b.go'`, true}, - {"", "micro", `micro +12 '/r/a b.go'`, true}, - {"", "hx", `hx '/r/a b.go:12'`, true}, - {"", "fresh", `fresh '/r/a b.go:12'`, true}, - {"", "code --wait", `code --wait -g '/r/a b.go:12'`, false}, - {"", "subl", `subl '/r/a b.go:12'`, false}, - {"", "idea", `idea --line 12 '/r/a b.go'`, false}, - {"", "mvim", `mvim +12 '/r/a b.go'`, false}, - // An unknown editor gets only the file, in the terminal. - {"", "myeditor", `myeditor '/r/a b.go'`, true}, + {"", "nvim", `nvim +12 '/r/a b.go'`, false}, + {"", "/usr/bin/vim -u NONE", `/usr/bin/vim -u NONE +12 '/r/a b.go'`, false}, + {"", "micro", `micro +12 '/r/a b.go'`, false}, + {"", "hx", `hx '/r/a b.go:12'`, false}, + {"", "fresh", `fresh '/r/a b.go:12'`, false}, + {"", "code --wait", `code --wait -g '/r/a b.go:12'`, true}, + {"", "subl", `subl '/r/a b.go:12'`, true}, + {"", "idea", `idea --line 12 '/r/a b.go'`, true}, + {"", "mvim", `mvim +12 '/r/a b.go'`, true}, + // A path in quotes can have spaces. + {"", `"/Apps/VS Code.app/bin/code" -w`, `"/Apps/VS Code.app/bin/code" -w -g '/r/a b.go:12'`, true}, + {"", `'/my apps/nvim'`, `'/my apps/nvim' +12 '/r/a b.go'`, false}, + // An unknown editor gets +line, and draws in the terminal. + {"", "vim.basic", `vim.basic +12 '/r/a b.go'`, false}, // A wait flag tells that an unknown editor has a window. - {"", "myeditor --wait", `myeditor --wait '/r/a b.go'`, false}, + {"", "myeditor --wait", `myeditor --wait +12 '/r/a b.go'`, true}, } for _, tt := range tests { - got := resolveEditor(tt.template, tt.value, abs, 12) - if got.script != tt.script || got.terminal != tt.terminal { - t.Errorf("resolveEditor(%q, %q) = %q, terminal %v; want %q, terminal %v", - tt.template, tt.value, got.script, got.terminal, tt.script, tt.terminal) + var got editorCommand + if tt.template != "" { + got = templateCommand(tt.template, abs, 12) + } else { + got = envCommand(tt.editor, abs, 12) + } + if got.script != tt.script || got.window != tt.window { + t.Errorf("editor %q%q = %q, window %v; want %q, window %v", + tt.template, tt.editor, got.script, got.window, tt.script, tt.window) } } } -// fakeRex makes m use a rex in dir that writes its arguments to rex.log -// in dir. It returns the path of rex.log. -func fakeRex(t *testing.T, m *model, dir string) string { +// writeScript writes an executable sh script with body to dir/name, and +// returns its path. +func writeScript(t *testing.T, dir, name, body string) string { t.Helper() - log := filepath.Join(dir, "rex.log") - script := "#!/bin/sh\nprintf '%s\\n' \"$*\" >> '" + log + "'\n" - if err := os.WriteFile(filepath.Join(dir, "rex"), []byte(script), 0o755); err != nil { + path := filepath.Join(dir, name) + if err := os.WriteFile(path, []byte("#!/bin/sh\n"+body), 0o755); err != nil { t.Fatal(err) } + return path +} + +// fakeRex makes m use a rex in a new directory that runs body. It returns +// the directory. +func fakeRex(t *testing.T, m *model, body string) string { + t.Helper() + dir := t.TempDir() + writeScript(t, dir, "rex", body) t.Setenv("REX_SESSION", "S") t.Setenv("REX_BLOCK", "B") t.Setenv("REX_BIN_DIR", dir) m.rex = rex.FromEnv() - return log + return dir +} + +// logRex makes m use a rex that writes its arguments to a log. It returns +// the path of the log. +func logRex(t *testing.T, m *model) string { + t.Helper() + dir := fakeRex(t, m, `printf '%s\n' "$*" >> "$(dirname "$0")/rex.log"`+"\n") + return filepath.Join(dir, "rex.log") } // pressE moves the cursor to the inserted line 4 of a.go and presses e. It @@ -74,15 +98,14 @@ func pressE(t *testing.T, m model) (model, tea.Cmd) { return *next.(*model), cmd } -// TestTerminalEditorInRex checks that a terminal editor opens in a new Rex -// block, at the line. A terminal editor in the background does not show, -// and it takes keys from rexdiff. +// TestTerminalEditorInRex checks that a terminal editor opens at the line +// in a new Rex block, and that e returns no command. func TestTerminalEditorInRex(t *testing.T) { tests := []struct { name, editor, env, want string }{ - {"settings", "/opt/homebrew/bin/nvim +{line} {file}", "", "sh -c /opt/homebrew/bin/nvim +4 '%s'"}, - {"environment", "", "nvim", "sh -c nvim +4 '%s'"}, + {"settings", "/opt/homebrew/bin/nvim +{line} {file}", "", "/opt/homebrew/bin/nvim +4 "}, + {"environment", "", "nvim", "nvim +4 "}, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { @@ -90,7 +113,7 @@ func TestTerminalEditorInRex(t *testing.T) { m.cfg.Editor = tt.editor t.Setenv("VISUAL", "") t.Setenv("EDITOR", tt.env) - log := fakeRex(t, &m, t.TempDir()) + log := logRex(t, &m) if _, cmd := pressE(t, m); cmd != nil { t.Errorf("e returned a command %T, want none", cmd) } @@ -98,9 +121,9 @@ func TestTerminalEditorInRex(t *testing.T) { if err != nil { t.Fatal(err) } - abs := filepath.Join(m.repo.Root(), "a.go") - want := "split -s S -b B --cwd " + m.repo.Root() + " -- " + - strings.Replace(tt.want, "%s", abs, 1) + root := m.repo.Root() + want := "split -s S -b B --cwd " + root + " -- sh -c " + tt.want + + "'" + filepath.Join(root, "a.go") + "'" if got := strings.TrimSpace(string(b)); got != want { t.Errorf("rex got\n%s\nwant\n%s", got, want) } @@ -109,18 +132,14 @@ func TestTerminalEditorInRex(t *testing.T) { } // TestWindowEditor checks that a window editor gets the file and the line, -// and runs without a terminal. So it cannot take the keys of rexdiff. +// and runs without a terminal. func TestWindowEditor(t *testing.T) { m, _ := newTestModel(t) bin := t.TempDir() out := filepath.Join(bin, "out") - script := "#!/bin/sh\n" + - "{ echo \"$*\"; if (exec 3/dev/null; then echo tty; else echo no tty; fi; } > '" + out + "'\n" - if err := os.WriteFile(filepath.Join(bin, "code"), []byte(script), 0o755); err != nil { - t.Fatal(err) - } - m.cfg.Editor = filepath.Join(bin, "code") + " -g {file}:{line}" - log := fakeRex(t, &m, t.TempDir()) + code := writeScript(t, bin, "code", `{ echo "$*"; if (exec 3/dev/null; then echo tty; else echo no tty; fi; } > '`+out+"'\n") + m.cfg.Editor = code + " -g {file}:{line}" + log := logRex(t, &m) m, cmd := pressE(t, m) if cmd == nil { t.Fatal("e returned no command") @@ -143,15 +162,11 @@ func TestWindowEditor(t *testing.T) { // TestEditorError checks that the footer tells why an editor failed. func TestEditorError(t *testing.T) { - bin := t.TempDir() - script := "#!/bin/sh\necho 'code: no such file' >&2\nexit 3\n" - if err := os.WriteFile(filepath.Join(bin, "code"), []byte(script), 0o755); err != nil { - t.Fatal(err) - } + code := writeScript(t, t.TempDir(), "code", "echo 'code: no such file' >&2\nexit 3\n") tests := []struct { name, editor, path, want string }{ - {"fails", filepath.Join(bin, "code") + " -g {file}", os.Getenv("PATH"), "editor: exit status 3: code: no such file"}, + {"fails", code + " -g {file}", os.Getenv("PATH"), "editor: exit status 3: code: no such file"}, {"cannot start", "code -g {file}", "", "editor: "}, // sh is not in PATH } for _, tt := range tests { @@ -163,15 +178,7 @@ func TestEditorError(t *testing.T) { if cmd == nil { t.Fatal("e returned no command") } - done := make(chan tea.Msg) - go func() { done <- cmd() }() - select { - case msg := <-done: - m = step(t, m, msg) - case <-time.After(5 * time.Second): - t.Fatal("the command did not return") - } - if !strings.Contains(view(m), tt.want) { + if m = step(t, m, cmd()); !strings.Contains(view(m), tt.want) { t.Errorf("the footer does not show %q:\n%s", tt.want, view(m)) } }) diff --git a/tui/fold_test.go b/tui/fold_test.go index b980b56..6690a29 100644 --- a/tui/fold_test.go +++ b/tui/fold_test.go @@ -2,11 +2,14 @@ package tui import ( "fmt" + "os" + "path/filepath" "slices" "strings" "testing" tea "charm.land/bubbletea/v2" + "github.com/charmbracelet/x/ansi" "github.com/rednafi/rexdiff/diff" "github.com/rednafi/rexdiff/vcs" @@ -54,14 +57,8 @@ func foldsOf(p *pane) []int { return out } -// rowOf returns the row of new line n in p, or -1. -func rowOf(p *pane, n int) int { - return slices.IndexFunc(p.rows, func(r diff.Row) bool { return r.New == n && r.Kind != diff.KindHunk }) -} - -// TestExpandFold checks that → on a fold shows its lines, and moves the -// cursor to the first of them. The footer tells about the key. On a line -// of code, → scrolls sideways. +// TestExpandFold checks that → shows the lines of a fold and scrolls +// sideways on a line of code. func TestExpandFold(t *testing.T) { m := newFoldModel(t) p := &m.changes @@ -88,7 +85,7 @@ func TestExpandFold(t *testing.T) { t.Errorf("after →, the cursor is on %+v, want line 14", r) } for n := 14; n <= 26; n++ { - if rowOf(p, n) < 0 { + if p.rowOf(diff.SideNew, n) < 0 { t.Errorf("line %d does not show", n) } } @@ -116,6 +113,106 @@ func TestGotoLineInFold(t *testing.T) { } } +// TestExpandKeepsPlaces checks the cursor, the selection, and the hits of +// a find after a fold shows. +func TestExpandKeepsPlaces(t *testing.T) { + m := newFoldModel(t) + m.changes.cur = m.changes.rowOf(diff.SideNew, 29) + m = press(t, m, "v") // the selection starts on line 29 + m = step(t, m, tea.KeyPressMsg{Code: 'f', Mod: tea.ModCtrl}) + m = step(t, m, tea.KeyPressMsg{Text: "line 3", Code: 'l'}) + m = press(t, m, "enter") + if !m.find.active() { + t.Fatal("the find has no hits") + } + m.changes.cur = 0 // the fold at the top + m = step(t, m, tea.KeyPressMsg{Code: tea.KeyRight}) + p := &m.changes + if r := p.rows[p.cur]; r.New != 1 { + t.Errorf("the cursor is on %+v, want line 1", r) + } + if r := p.rows[p.anchor]; r.New != 29 { + t.Errorf("the selection starts on %+v, want line 29", r) + } + if len(m.find.hits) == 0 { + t.Error("the find has no hits after the fold shows") + } + for _, h := range m.find.hits { + if l := strings.ToLower(ansi.Strip(p.lines[h])); !strings.Contains(l, "line 3") { + t.Errorf("a hit is on %q, want a line with line 3", l) + } + } +} + +// TestFoldStaysOpen checks that an expanded fold stays open after edits of +// the agent and closes after a commit. +func TestFoldStaysOpen(t *testing.T) { + m := newFoldModel(t) + m.changes.cur = slices.IndexFunc(m.changes.rows, func(r diff.Row) bool { return r.Fold.Lines == 13 }) + m = step(t, m, tea.KeyPressMsg{Code: tea.KeyRight}) + dir := m.repo.Root() + edit := func(f func(string) string) { + b, err := os.ReadFile(filepath.Join(dir, "f.txt")) + if err != nil { + t.Fatal(err) + } + write(t, dir, "f.txt", f(string(b))) + m.reload() + } + edit(func(s string) string { return "new\n" + s }) + if got := foldsOf(&m.changes); !slices.Equal(got, []int{3, 7}) { + t.Errorf("after a line above, folds = %v, want [3 7]", got) + } + edit(func(s string) string { return strings.Replace(s, "line 20\n", "LINE 20\n", 1) }) + if got := foldsOf(&m.changes); !slices.Equal(got, []int{3, 7}) { + t.Errorf("after a change in the fold, folds = %v, want [3 7]", got) + } + // An undo of the change at line 31 makes one fold of old lines 14 to + // 40. It overlaps the open range, so it shows. + edit(func(s string) string { return strings.Replace(s, "LINE 30\n", "line 30\n", 1) }) + if got := foldsOf(&m.changes); !slices.Equal(got, []int{3}) { + t.Errorf("after an undo next to the fold, folds = %v, want [3]", got) + } + // After the commit, the file has 41 lines. Changes at lines 12 and 26 + // make a fold of old lines 16 to 22, inside the open range. + git(t, dir, "commit", "-qam", "agent") + edit(func(s string) string { + lines := strings.Split(s, "\n") + lines[11], lines[25] = "changed", "changed" + return strings.Join(lines, "\n") + }) + if got := foldsOf(&m.changes); !slices.Equal(got, []int{8, 7, 12}) { + t.Errorf("after a commit, folds = %v, want [8 7 12]", got) + } +} + +// TestFoldsSurviveRestart checks that the expanded folds and the cursor +// line come back after a restart for an update. +func TestFoldsSurviveRestart(t *testing.T) { + m := newFoldModel(t) + m.changes.cur = slices.IndexFunc(m.changes.rows, func(r diff.Row) bool { return r.Fold.Lines == 13 }) + m = step(t, m, tea.KeyPressMsg{Code: tea.KeyRight}) + m.changes.cur = m.changes.rowOf(diff.SideNew, 20) + path, err := m.saveState() + if err != nil { + t.Fatal(err) + } + saved, err := loadState(path) + if err != nil { + t.Fatal(err) + } + m2 := newModel(t.Context(), Options{Repo: m.repo, Layout: LayoutUnified}, defaultConfig()) + m2.restoreSettings(saved) + m2.reload() + m2.restorePlaces(saved) + if got := foldsOf(&m2.changes); !slices.Equal(got, []int{6, 7}) { + t.Errorf("after the restart, folds = %v, want [6 7]", got) + } + if r := m2.changes.rows[m2.changes.cur]; r.New != 20 { + t.Errorf("after the restart, the cursor is on %+v, want line 20", r) + } +} + // TestClickFold checks that a click on a fold shows its lines, also in the // split layout and with wrap. func TestClickFold(t *testing.T) { @@ -126,20 +223,19 @@ func TestClickFold(t *testing.T) { if got := foldsOf(&m.changes); !slices.Equal(got, []int{6, 13}) { t.Errorf("keys %v: after the click, folds = %v, want [6 13]", keys, got) } - if rowOf(&m.changes, 40) < 0 { + if m.changes.rowOf(diff.SideNew, 40) < 0 { t.Errorf("keys %v: line 40 does not show", keys) } } } // TestFoldKeepsComment checks that a fold with a comment on one of its -// lines shows. A new scope folds the lines again, but the comment must -// stay on the screen. +// lines shows, also after a new scope closes the expanded folds. func TestFoldKeepsComment(t *testing.T) { m := newFoldModel(t) m.changes.cur = slices.IndexFunc(m.changes.rows, func(r diff.Row) bool { return r.Fold.Lines == 13 }) m = step(t, m, tea.KeyPressMsg{Code: tea.KeyRight}) - m.changes.cur = rowOf(&m.changes, 20) + m.changes.cur = m.changes.rowOf(diff.SideNew, 20) m = press(t, m, "c") m = step(t, m, tea.KeyPressMsg{Text: "why", Code: 'w'}) m = press(t, m, "enter") @@ -150,7 +246,7 @@ func TestFoldKeepsComment(t *testing.T) { if m.folds != nil { t.Errorf("a new scope keeps the expanded folds %v", m.folds) } - if rowOf(&m.changes, 20) < 0 || !shows(m, "why") { + if m.changes.rowOf(diff.SideNew, 20) < 0 || !shows(m, "why") { t.Errorf("the commented line 20 does not show:\n%s", view(m)) } if got := foldsOf(&m.changes); !slices.Equal(got, []int{6, 7}) { diff --git a/tui/model.go b/tui/model.go index e56ffd1..2d902e0 100644 --- a/tui/model.go +++ b/tui/model.go @@ -51,8 +51,8 @@ const ( type pane struct { lines []string rows []diff.Row // rows[i] is the source line that lines[i] shows - diff *diff.Diff // the render, for mouse hits; nil for markdown and the PR - path string // the file of the rows; "" if the pane takes no comments + diff *diff.Diff // the render, for mouse hits, or nil for markdown and the PR + path string // the file of the rows, or "" if the pane takes no comments cur int // the row of the cursor top int // the first row on the screen anchor int // the start of the visual selection, or -1 @@ -164,7 +164,7 @@ type prState struct { // agentState is what the program knows about the agent in a block. type agentState struct { state rex.State // "" after the block closed - cwd string // the working directory; "" if unknown + cwd string // the working directory, or "" if unknown hasCwd bool // the program asked Rex for cwd } @@ -196,9 +196,7 @@ type model struct { tab tab scope Scope - // folds has the folds that the user expanded in the Changes tab, by - // path and by the new line number of their first line. - folds map[string]map[int]bool + folds map[string]*openFolds // the folds that the user expanded in the Changes tab, by path base string from string // the commits scope: the oldest commit that the user picked to string // the commits scope: the newest commit that the user picked @@ -367,8 +365,7 @@ func (m *model) trees() (from, to, note string, err error) { return from, to, "", err case ScopeLastTurn: if m.turn == nil { - note := "no agent turn seen yet: the next turn's changes will show here" - return "", "", note, nil + return "", "", "no agent turn seen yet: the next turn's changes will show here", nil } from = m.turn.tree case ScopeBranch: @@ -461,7 +458,7 @@ func (m *model) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } // A tick only starts the next tick, so it is no work. pending := cmd - if _, tick := msg.(tickMsg); tick { + if _, ok := msg.(tickMsg); ok { pending = nil } if m.restartIfReady(pending) { diff --git a/tui/mouse.go b/tui/mouse.go index abc401f..524db0f 100644 --- a/tui/mouse.go +++ b/tui/mouse.go @@ -58,7 +58,7 @@ func (m *model) onClick(mouse tea.Mouse) tea.Cmd { m.focusList = false p.cur, p.anchor = p.head(rowAt[y]), -1 if m.onFold() { - m.expandFold(p.cur) + m.expandFold(p.rows[p.cur].Fold) return nil } side, onNumber := p.side(mouse.X - m.mainX() - gutter) diff --git a/tui/nav.go b/tui/nav.go index 9b05ca1..5b0bd31 100644 --- a/tui/nav.go +++ b/tui/nav.go @@ -2,6 +2,7 @@ package tui import ( "fmt" + "hash/fnv" "slices" "strconv" "strings" @@ -145,7 +146,7 @@ func (m *model) moveKey(msg tea.KeyPressMsg) (tea.Cmd, bool) { case key.Matches(msg, k.PrevFile): m.pickFile(m.listSel() - 1) case key.Matches(msg, k.Right) && m.onFold(): - m.expandFold(p.cur) + m.expandFold(p.rows[p.cur].Fold) case key.Matches(msg, k.Right) && !m.wrap: p.xoff += scrollStep m.rerender() @@ -294,13 +295,36 @@ func (m *model) onFold() bool { return m.tab == tabChanges && p.cur < len(p.rows) && p.rows[p.cur].Fold.Lines > 0 } -// showFold tells if the lines of fold f of the file at path show: the user -// expanded the fold, or a comment is on one of its lines. -func (m *model) showFold(path string, f diff.Fold) bool { - if m.folds[path][f.New] { - return true +// openFolds has the folds that the user expanded in one file, as ranges of +// old line numbers. The old side does not change while the agent edits the +// file, so an edit keeps the lines of a range open. The ranges are for the +// old content with the hash Old. A different old content, for example +// after a commit, closes them. +type openFolds struct { + Old uint64 `json:"old"` + Ranges [][2]int `json:"ranges"` // the first and the last line of each range +} + +// hash returns the FNV-1a hash of s. +func hash(s string) uint64 { + h := fnv.New64a() + h.Write([]byte(s)) // a hash.Hash does not fail + return h.Sum64() +} + +// expanded tells if the lines of fold f of the file at path show: the +// user expanded the fold, or one of comments is on one of its lines. +func (m *model) expanded(path string, comments []*diff.Comment, f diff.Fold) bool { + // A fold that overlaps a range shows. An undo of a change next to a + // range makes a larger fold, which shows as a whole. + if o := m.folds[path]; o != nil { + for _, r := range o.Ranges { + if f.Old <= r[1] && f.Old+f.Lines-1 >= r[0] { + return true + } + } } - for _, c := range m.comments.All() { + for _, c := range comments { lo := f.New if c.Side == diff.SideOld { lo = f.Old @@ -312,18 +336,31 @@ func (m *model) showFold(path string, f diff.Fold) bool { return false } -// expandFold shows the lines of the fold at row i of the Changes tab. A -// cursor on the fold moves to the first of them. -func (m *model) expandFold(i int) { +// expandFold shows the lines of fold f in the Changes tab, and moves the +// cursor to the first of them. The selection and the hits of a find stay +// on their lines. +func (m *model) expandFold(f diff.Fold) { p := &m.changes + file := m.changedFile() + if file == nil { + return + } if m.folds == nil { - m.folds = map[string]map[int]bool{} + m.folds = map[string]*openFolds{} } - if m.folds[p.path] == nil { - m.folds[p.path] = map[int]bool{} + o := m.folds[file.Path] + if o == nil { + o = &openFolds{Old: hash(file.Old)} + m.folds[file.Path] = o + } + o.Ranges = append(o.Ranges, [2]int{f.Old, f.Old + f.Lines - 1}) + m.rerender() + p.cur = max(p.rowOf(diff.SideNew, f.New), 0) + if m.find.active() { + cur := p.cur + m.runFind(m.find.query) + p.cur = cur } - m.folds[p.path][p.rows[i].Fold.New] = true - m.renderChanges() } // listSel returns the index of the selected file in the file list. @@ -472,15 +509,12 @@ func (m *model) runFind(q string) { } lq := strings.ToLower(q) for i, l := range p.lines { - if !strings.Contains(strings.ToLower(ansi.Strip(l)), lq) { - continue - } - // The rows of a wrapped line make one hit. - hits := m.find.hits - if h := p.head(i); len(hits) == 0 || hits[len(hits)-1] != h { - m.find.hits = append(hits, h) + if strings.Contains(strings.ToLower(ansi.Strip(l)), lq) { + m.find.hits = append(m.find.hits, p.head(i)) } } + // The rows of a wrapped line make one hit. + m.find.hits = slices.Compact(m.find.hits) if !m.find.active() { m.flash("no match for " + q) return @@ -507,11 +541,13 @@ func (m *model) gotoLine(s string) { m.flash("not a line number: " + s) return } - // A line in a fold shows after the fold expands. - for i, r := range p.rows { - if f := r.Fold; p == &m.changes && n >= f.New && n < f.New+f.Lines { - m.expandFold(i) - break + if m.tab == tabChanges { + // Show the fold that has line n. + for _, r := range p.rows { + if f := r.Fold; n >= f.New && n < f.New+f.Lines { + m.expandFold(f) + break + } } } for i, r := range p.rows { diff --git a/tui/release_test.go b/tui/release_test.go index 079b2dc..d2dfa42 100644 --- a/tui/release_test.go +++ b/tui/release_test.go @@ -3,6 +3,7 @@ package tui import ( "context" "errors" + "io/fs" "os" "path/filepath" "strings" @@ -20,8 +21,8 @@ import ( type fakeUpdater struct { latest update.Release newer bool - err error // of Latest - installErr error + err error // the error of Latest + installErr error // the error of Install installed []string // the tags that Install got } @@ -97,7 +98,7 @@ func TestUpdate(t *testing.T) { if err != nil { t.Fatal(err) } - if _, err := os.Stat(m.restartState); !os.IsNotExist(err) { + if _, err := os.Stat(m.restartState); !errors.Is(err, fs.ErrNotExist) { t.Error("loadState did not remove the file") } m2 := newModel(t.Context(), Options{Repo: m.repo}, defaultConfig()) @@ -179,9 +180,8 @@ func TestNoUpdate(t *testing.T) { } } -// TestCommentsJSON checks that comments keep their code and their rows -// in JSON, so that they follow their lines and give the same review after -// a restart. +// TestCommentsJSON checks that the comments give the same review after +// JSON encoding and decoding. func TestCommentsJSON(t *testing.T) { m, _ := newTestModel(t) m = press(t, m, "j", "j", "j", "j", "c", "x", "enter") @@ -255,10 +255,10 @@ func TestUpdateClickRule(t *testing.T) { u := &fakeUpdater{latest: update.Release{Tag: "v0.2.0"}, newer: true} m := updaterModel(t, u) rule := bodyY + m.bodyHeight() - if next, _ := m.Update(tea.MouseClickMsg{Button: tea.MouseLeft, X: 3, Y: rule}); next.(*model).phase == phaseInstalling { + if got := step(t, m, tea.MouseClickMsg{Button: tea.MouseLeft, X: 3, Y: rule}); got.phase == phaseInstalling { t.Error("a click on the rule started the install") } - if next, _ := m.Update(tea.MouseClickMsg{Button: tea.MouseLeft, X: 3, Y: rule + 1}); next.(*model).phase != phaseInstalling { + if got := step(t, m, tea.MouseClickMsg{Button: tea.MouseLeft, X: 3, Y: rule + 1}); got.phase != phaseInstalling { t.Error("a click on the footer did not start the install") } } @@ -292,14 +292,15 @@ func TestRestartOnTick(t *testing.T) { m := updaterModel(t, u) m.phase = phaseInstalled next, cmd := m.Update(tickMsg{}) - if next.(*model).restartState == "" || cmd == nil { + m = *next.(*model) + if m.restartState == "" || cmd == nil { t.Fatal("no restart on a tick") } - os.Remove(next.(*model).restartState) + os.Remove(m.restartState) } -// TestSavePendingPRPlace checks that a restart before the PR loads keeps -// the PR place of the restart before it. +// TestSavePendingPRPlace checks that a restart before the PR loads saves +// the PR place of the previous restart. func TestSavePendingPRPlace(t *testing.T) { m, _ := newTestModel(t) m.prPlace = &savedPane{Cur: 7, Top: 3} diff --git a/tui/render.go b/tui/render.go index 69cfa9c..e3deab7 100644 --- a/tui/render.go +++ b/tui/render.go @@ -27,7 +27,7 @@ func (m *model) listWidth() int { return min(m.listW, max(m.w/3, minListWidth)) } -// mainWidth is the width of the main pane, with its gutter. +// mainWidth returns the width of the main pane, with its gutter. func (m *model) mainWidth() int { w := m.w if lw := m.listWidth(); lw > 0 { @@ -36,7 +36,7 @@ func (m *model) mainWidth() int { return max(w, 24) } -// codeWidth is the width of the main pane, without its gutter. +// codeWidth returns the width of the main pane, without its gutter. func (m *model) codeWidth() int { return m.mainWidth() - gutter } // rerender renders the current pane again, for example after a change of @@ -120,8 +120,12 @@ func (m *model) renderChanges() { case m.rendered && isMarkdown(f.Path) && f.Status != vcs.StatusDeleted: m.setMarkdown(p, f.New) default: + if o := m.folds[f.Path]; o != nil && o.Old != hash(f.Old) { + delete(m.folds, f.Path) + } o := m.diffOptions(p.xoff) - o.Show = func(fd diff.Fold) bool { return m.showFold(f.Path, fd) } + comments := m.comments.All() + o.Expanded = func(fd diff.Fold) bool { return m.expanded(f.Path, comments, fd) } p.setDiff(diff.Render(f.Path, f.Old, f.New, o)) } } @@ -186,7 +190,8 @@ func (m *model) markdown(wrap int) *glamour.TermRenderer { return r } -// renderMarkdown renders src with r. If r is nil or fails, it returns src. +// renderMarkdown returns the lines of src, rendered with r. If r is nil or +// fails, it returns the lines of src as they are. func renderMarkdown(r *glamour.TermRenderer, src string) []string { if r != nil { if out, err := r.Render(src); err == nil { diff --git a/tui/resume.go b/tui/resume.go index 9daaf80..8fdc554 100644 --- a/tui/resume.go +++ b/tui/resume.go @@ -7,13 +7,15 @@ package tui import ( "encoding/json/v2" "os" + "slices" "time" "github.com/rednafi/rexdiff/diff" + "github.com/rednafi/rexdiff/vcs" ) -// savedState holds the comments, the scope, the view, and the places in -// the panes for a restart. +// savedState holds the comments, the scope, the view, the expanded folds, +// and the places in the panes for a restart. type savedState struct { Tab tab `json:"tab"` Scope Scope `json:"scope"` @@ -38,6 +40,8 @@ type savedState struct { FocusList bool `json:"focus_list"` ListW int `json:"list_w"` + Folds map[string]*openFolds `json:"folds"` + Comments *diff.Comments `json:"comments"` LastSent string `json:"last_sent"` Beside string `json:"beside"` @@ -76,7 +80,7 @@ func (m *model) saveState() (string, error) { PR: m.prPane.place(), Split: m.split, Wrap: m.wrap, Rendered: m.rendered, ListLeft: m.listLeft, ListHidden: m.listHidden, FocusList: m.focusList, - ListW: m.listW, Comments: &m.comments, LastSent: m.lastSent, Beside: m.beside, + ListW: m.listW, Folds: m.folds, Comments: &m.comments, LastSent: m.lastSent, Beside: m.beside, } if t := m.turn; t != nil { s.Turn = &savedTurn{t.block, t.app, t.tree, t.running, t.at} @@ -129,25 +133,22 @@ func (m *model) restoreSettings(s *savedState) { } m.split, m.wrap, m.rendered = s.Split, s.Wrap, s.Rendered m.listLeft, m.listHidden, m.focusList, m.listW = s.ListLeft, s.ListHidden, s.FocusList, s.ListW + m.folds = s.Folds if s.Comments != nil { m.comments = *s.Comments } m.lastSent, m.beside = s.LastSent, s.Beside } -// restorePlaces selects the files of s and puts the cursors where they -// were. Call it after the first reload. +// restorePlaces selects the files of s and moves the cursors to the places +// in s. Call it after the first reload. func (m *model) restorePlaces(s *savedState) { - for i, f := range m.files { - if f.Path == s.ChangePath { - m.changeSel = i - } + if i := slices.IndexFunc(m.files, func(f changed) bool { return f.Path == s.ChangePath }); i >= 0 { + m.changeSel = i } m.allFiles, _ = m.repo.Files(m.ctx) // an error shows an empty list - for i, e := range m.allFiles { - if e.Path == s.FilePath { - m.allSel = i - } + if i := slices.IndexFunc(m.allFiles, func(e vcs.Entry) bool { return e.Path == s.FilePath }); i >= 0 { + m.allSel = i } m.renderChanges() m.refreshViewer() diff --git a/tui/tui_test.go b/tui/tui_test.go index 138225f..34f32ba 100644 --- a/tui/tui_test.go +++ b/tui/tui_test.go @@ -130,6 +130,7 @@ func paths(m model) []string { return ps } +// TestScopes checks the files that each scope shows. func TestScopes(t *testing.T) { m, dir := newTestModel(t) if got := paths(m); !slices.Equal(got, []string{"a.go"}) { @@ -174,6 +175,8 @@ func TestScopes(t *testing.T) { } } +// TestComment checks comments in the two layouts, and the send, copy, and +// quit keys with unsent comments. func TestComment(t *testing.T) { m, _ := newTestModel(t) @@ -213,6 +216,7 @@ func TestComment(t *testing.T) { } } +// TestTabs checks the Files tab and the PR tab. func TestTabs(t *testing.T) { m, _ := newTestModel(t) @@ -250,8 +254,8 @@ func TestPRChecks(t *testing.T) { } } -// TestSmallScreens presses every key on screens down to 1x1. The program -// must not panic, and the view must fit the screen. +// TestSmallScreens checks that every key works on screens down to 1x1 and +// that the view fits the screen. func TestSmallScreens(t *testing.T) { keys := []string{ "w", "j", "v", "j", "c", "\x1b[31mx\x07", "enter", "f", "F", "]", "[", "n", "N", @@ -280,8 +284,8 @@ func TestSmallScreens(t *testing.T) { } } -// TestDragAfterReload drags while the pane gets shorter. The program must -// not panic. +// TestDragAfterReload checks that a drag does not panic when the pane gets +// shorter. func TestDragAfterReload(t *testing.T) { m, _ := newTestModel(t) m.drag = dragState{active: true, gutter: true, moved: true, start: 48} @@ -311,10 +315,11 @@ func click(t *testing.T, m model, x, y int) model { return step(t, m, tea.MouseReleaseMsg{Button: tea.MouseLeft, X: x, Y: y}) } -// TestClickOldNumber clicks the old line number of an unchanged line. The -// comment goes on the new side, so that it shows and d can delete it. +// TestClickOldNumber checks a comment from a click on the old line number of +// an unchanged line. func TestClickOldNumber(t *testing.T) { m, _ := newTestModel(t) + // The comment goes on the new side, so that it shows and d can delete it. equal := func(r diff.Row) bool { return r.Kind == diff.KindEqual } i := slices.IndexFunc(m.changes.rows, equal) m = click(t, m, m.mainX()+gutter, screenY(t, m, i)) @@ -327,8 +332,8 @@ func TestClickOldNumber(t *testing.T) { } } -// TestClickCommentBox clicks while the comment box is open. No click -// changes the draft or closes the box. +// TestClickCommentBox checks that a click does not change the draft or close +// the comment box. func TestClickCommentBox(t *testing.T) { m, _ := newTestModel(t) m = press(t, m, "]", "c", "h", "i") @@ -350,18 +355,11 @@ func TestClickCommentBox(t *testing.T) { } } -// TestSendError sends the review when Rex fails. The footer shows the -// error of Rex. +// TestSendError checks that the footer shows the error of Rex after a send +// fails. func TestSendError(t *testing.T) { - m, dir := newTestModel(t) - bin := filepath.Join(dir, "rex") - script := "#!/bin/sh\necho 'Error: no Rex server' >&2\nexit 1\n" - if err := os.WriteFile(bin, []byte(script), 0o755); err != nil { - t.Fatal(err) - } - t.Setenv("REX_SESSION", "S") - t.Setenv("REX_BIN_DIR", dir) - m.rex = rex.FromEnv() + m, _ := newTestModel(t) + fakeRex(t, &m, "echo 'Error: no Rex server' >&2\nexit 1\n") m = press(t, m, "]", "c", "h", "i", "enter") m = step(t, m, m.deliver(rex.Agent{Block: "b", Name: "pi"})()) if !strings.Contains(m.status, "no Rex server") { @@ -369,8 +367,8 @@ func TestSendError(t *testing.T) { } } -// TestMissingHead removes the object of the HEAD commit. The Changes tab -// shows the error, and not each file as added. +// TestMissingHead checks that the Changes tab shows an error, and not each +// file as added, when the object of the HEAD commit is missing. func TestMissingHead(t *testing.T) { m, dir := newTestModel(t) sha, err := exec.Command("git", "-C", dir, "rev-parse", "HEAD").Output() @@ -402,10 +400,8 @@ func errorHas(err error, texts ...string) bool { } // TestParseConfig checks the settings that the Rex action sends as JSON. -// Each action of herdr-reviewr has a key. An unknown key or a bad value is -// an error. func TestParseConfig(t *testing.T) { - // The actions in herdr-reviewr's src/keymap.rs. + // Each action in src/keymap.rs of herdr-reviewr has a key. names := strings.Fields(`base-pick collapse comment comments commit-pick copy delete down edit expand find goto-line half-down half-up keys navigator-grow navigator-hide navigator-position navigator-shrink next-comment next-file @@ -425,6 +421,7 @@ func TestParseConfig(t *testing.T) { t.Errorf("every action: %v", err) } + // An unknown key is an error. unknown := []struct{ text, key string }{ {`{"layuot": "split"}`, `"layuot"`}, {`{"keybindings": {"next-fiel": ["x"]}}`, `"keybindings.next-fiel"`}, @@ -500,8 +497,8 @@ func TestParseConfig(t *testing.T) { } } -// TestDiffSigns checks that a bar marks a changed line by default, and -// that the setting diff_signs changes the bar to + and -. +// TestDiffSigns checks that a bar marks a changed line by default, and that +// the setting diff_signs changes the bar to + and -. func TestDiffSigns(t *testing.T) { line := func(m model, code string) string { for l := range strings.Lines(view(m)) { @@ -529,7 +526,7 @@ func TestDiffSigns(t *testing.T) { } } -// TestConfigFlags checks that the flags win over the settings, and that +// TestConfigFlags checks that the flags override the settings, and that // the settings set the keys and the file list. func TestConfigFlags(t *testing.T) { m, _ := newTestModel(t) @@ -560,8 +557,8 @@ func TestConfigFlags(t *testing.T) { } } -// TestNotice checks that the footer shows all of a long notice, on its own -// lines, so that the user can read what went wrong. +// TestNotice checks that the footer shows all of a long notice on its own +// lines. func TestNotice(t *testing.T) { m, _ := newTestModel(t) // The path makes the first line end in "--" at column 82, past the @@ -571,19 +568,19 @@ func TestNotice(t *testing.T) { strings.Repeat("the end word ", 20) + "LAST " + strings.Repeat("x", 100) m = newModel(t.Context(), Options{Repo: m.repo, Notice: notice}, defaultConfig()) m = step(t, m, tea.WindowSizeMsg{Width: 80, Height: 30}) - view := ansi.Strip(m.View().Content) + v := view(m) // Each word of the notice is in the view, so a hyphen does not push // "--" past the edge of the screen. - words := strings.Fields(view) + words := strings.Fields(v) for _, w := range strings.Fields(notice)[:30] { if !slices.Contains(words, w) { - t.Errorf("the view does not show %q of the notice:\n%s", w, view) + t.Errorf("the view does not show %q of the notice:\n%s", w, v) } } - if !strings.Contains(view, strings.Repeat("x", 80)) { + if !strings.Contains(v, strings.Repeat("x", 80)) { t.Errorf("the view does not show the start of a word wider than the screen") } - for line := range strings.SplitSeq(view, "\n") { + for line := range strings.SplitSeq(v, "\n") { if ansi.StringWidth(line) > 80 { t.Errorf("line wider than the screen: %q", line) } @@ -630,9 +627,7 @@ func twoHunks(t *testing.T, m model, dir, long string) model { return m } -// TestWrap checks that the w key toggles wrap, and that the cursor moves -// by source lines. A comment on wrapped lines gets the same -// snippet. +// TestWrap checks that the w key turns wrap on and off. func TestWrap(t *testing.T) { m, _, long := wrapModel(t) cut := len(m.changes.lines) @@ -645,6 +640,7 @@ func TestWrap(t *testing.T) { if v := view(m); !strings.Contains(v, "x + 1") { t.Errorf("the end of the long line does not show:\n%s", v) } + // The cursor moves by source lines. m = press(t, m, "]") for range len(m.changes.lines) { if m.changes.cont(m.changes.cur) { @@ -653,6 +649,7 @@ func TestWrap(t *testing.T) { m = press(t, m, "j") } + // A comment on wrapped lines gets the same snippet. m = press(t, m, "home", "]", "v", "j", "c", "o", "k", "enter") cs := m.comments.All() want := "\n-\treturn 1\n+" + long + "\nok" @@ -739,9 +736,8 @@ func TestTallLine(t *testing.T) { m.changes.top, m.changes.cur) } - // A cursor on the line shows its first rows. The - // comment box shows below the last row. A c on a line with a comment - // edits the comment. + // A cursor on the line shows its first rows. The comment box shows below + // the last row. A c on a line with a comment edits the comment. m = step(t, m, tea.WindowSizeMsg{Width: 30, Height: 12}) m = press(t, m, "home") for m.changes.rows[m.changes.cur].New != 4 { @@ -764,10 +760,10 @@ func TestTallLine(t *testing.T) { } } -// TestHostileText puts escape sequences in outside data: a file name, a -// comment, an agent, and an error from Rex. The screen must not get them -// raw. The diff tests check the review text. +// TestHostileText checks that the screen does not show raw escape sequences +// from a file name, a comment, an agent, or an error from Rex. func TestHostileText(t *testing.T) { + // The diff tests check the review text. const osc = "\x1b]0;owned\x07" // sets the title of the terminal raw := func(where string, m model) { t.Helper() @@ -796,8 +792,8 @@ func TestHostileText(t *testing.T) { raw("the turn in the header", m) } -// TestMouseOffScreen sends mouse events outside the screen, also while an -// overlay is open and the screen changes size. The program must not panic. +// TestMouseOffScreen checks that mouse events outside the screen do not +// panic, also while an overlay is open and the screen changes size. func TestMouseOffScreen(t *testing.T) { m, _ := newTestModel(t) for _, keys := range [][]string{{}, {"G"}, {"/"}, {"2"}, {"3"}} { @@ -821,9 +817,8 @@ func TestMouseOffScreen(t *testing.T) { } } -// TestEmptyRepo presses keys in each tab of a repository with no commits -// and no files, with key bindings that conflict. The program must not -// panic. +// TestEmptyRepo checks that keys do not panic in a repository with no +// commits and no files, with key bindings that conflict. func TestEmptyRepo(t *testing.T) { dir := t.TempDir() git(t, dir, "init", "-q", "-b", "main") @@ -854,8 +849,8 @@ func TestEmptyRepo(t *testing.T) { } } -// TestFooterHints checks the footer in the select and the find modes. It -// shows the keys of the mode. +// TestFooterHints checks that the footer shows the keys of the select and the +// find modes. func TestFooterHints(t *testing.T) { m, _ := newTestModel(t) if m = press(t, m, "]", "v"); !strings.Contains(view(m), "extend") { diff --git a/tui/view.go b/tui/view.go index 5c0f092..08be596 100644 --- a/tui/view.go +++ b/tui/view.go @@ -136,7 +136,7 @@ func (m *model) headerScopes(l *headerLine) { } } -// commitsLabel names the commits of the commits scope in the header. +// commitsLabel returns the name of the commits of the commits scope in the header. func (m *model) commitsLabel() string { if m.to == "" { return "pick ▾" @@ -165,7 +165,8 @@ func (m *model) turnLabel() string { return app + dimStyle.Render(" · "+ago.String()+" ago") } -// agentLabel tells how many agents can get the next send. +// agentLabel returns the header text that tells how many agents can get +// the next send. func (m *model) agentLabel() string { if m.rex == nil { return dimStyle.Render("not in Rex") @@ -432,7 +433,7 @@ func (m *model) ensureVisible() { // Each row takes one line or more. So the last row cannot show if it is // h rows or more below the top. p.top = max(p.top, last-h+1) - p.top = min(last, min(p.top, max(len(p.lines)-1, 0))) + p.top = min(last, p.top, max(len(p.lines)-1, 0)) for p.top < last { _, rowAt := m.mainLines(h) idx := slices.Index(rowAt, p.end(p.cur)) @@ -523,7 +524,7 @@ func (m *model) footerLine() string { return m.help.View(hk) } -// bodyHeight is the number of lines between the header and the footer. +// bodyHeight returns the number of lines between the header and the footer. func (m *model) bodyHeight() int { return m.heightAbove(m.footer()) } // heightAbove returns the height of the body above footer. @@ -553,10 +554,10 @@ func (m *model) View() tea.View { screen := hdr + "\n" + body + "\n" + rule + "\n" + footer if m.mode == modeOverlay && m.overlay != nil { - box, x, y, _ := m.overlayBox() + ov, x, y, _ := m.overlayBox() c := lipgloss.NewCanvas(m.w, m.h) c.Compose(lipgloss.NewLayer(screen)) - c.Compose(lipgloss.NewLayer(box).X(x).Y(y).Z(1)) + c.Compose(lipgloss.NewLayer(ov).X(x).Y(y).Z(1)) screen = c.Render() } @@ -577,7 +578,7 @@ func box(lines []string, w, h int) string { return st.Render(strings.Join(lines, "\n")) } -// programStatus is the status that rexdiff reports to Rex with OSC 7501. +// programStatus returns the status that rexdiff reports to Rex with OSC 7501. // Rex shows it in its tabs and its sidebar. func (m *model) programStatus() *tea.ProgramStatus { msg := "reviewing " + plural(len(m.files), "file") @@ -592,7 +593,7 @@ func (m *model) programStatus() *tea.ProgramStatus { } } -// mainX is the screen column where the main pane starts. +// mainX returns the screen column where the main pane starts. func (m *model) mainX() int { if lw := m.listWidth(); lw > 0 && m.listLeft { return lw + sepWidth @@ -600,7 +601,7 @@ func (m *model) mainX() int { return 0 } -// listX is the screen column where the file list starts. +// listX returns the screen column where the file list starts. func (m *model) listX() int { if !m.listLeft { return m.mainWidth() + sepWidth diff --git a/update/update.go b/update/update.go index e6f1e4a..4446572 100644 --- a/update/update.go +++ b/update/update.go @@ -26,6 +26,7 @@ import ( "path/filepath" "runtime" "runtime/debug" + "slices" "strconv" "strings" "syscall" @@ -58,8 +59,8 @@ const ( ) // Release is a release of rexdiff on GitHub. Get one from [Updater.Latest]. -// A Release that you make yourself has no files, so Install cannot install -// it. +// A Release that you make yourself has no files. Install can give it to +// Homebrew or go install, but it cannot download it. type Release struct { Tag string // for example "v0.2.0" assets []asset @@ -400,15 +401,7 @@ func Restart(bin string, args []string) error { func newer(tag, have string) bool { t, ok1 := numbers(tag) h, ok2 := numbers(have) - if !ok1 || !ok2 { - return false - } - for i := range t { - if t[i] != h[i] { - return t[i] > h[i] - } - } - return false + return ok1 && ok2 && slices.Compare(t[:], h[:]) > 0 } // numbers returns the three numbers of a version like "v1.2.3". diff --git a/update/update_test.go b/update/update_test.go index abc9dda..60910eb 100644 --- a/update/update_test.go +++ b/update/update_test.go @@ -14,6 +14,7 @@ import ( "os" "path/filepath" "runtime" + "slices" "strings" "testing" ) @@ -154,7 +155,7 @@ func TestInstallCommands(t *testing.T) { "HOMEBREW_NO_ENV_HINTS=1 NONINTERACTIVE=1 HOMEBREW_NO_AUTO_UPDATE=1 brew upgrade --cask rexdiff", "/opt/homebrew/bin/rexdiff -version", } - if strings.Join(brew.calls, "\n") != strings.Join(want, "\n") { + if !slices.Equal(brew.calls, want) { t.Errorf("brew calls:\n%s\nwant:\n%s", strings.Join(brew.calls, "\n"), strings.Join(want, "\n")) } @@ -301,7 +302,7 @@ func TestDownload(t *testing.T) { func TestInstallOutlivesCancel(t *testing.T) { ctx, cancel := context.WithCancel(t.Context()) var sawCancel bool - run := func(ctx context.Context, _ []string, name string, args ...string) ([]byte, error) { + run := func(ctx context.Context, _ []string, name string, _ ...string) ([]byte, error) { if name == "brew" { cancel() // the user quits rexdiff while brew runs sawCancel = ctx.Err() != nil diff --git a/vcs/hostile_test.go b/vcs/hostile_test.go index c00a549..5bf83fe 100644 --- a/vcs/hostile_test.go +++ b/vcs/hostile_test.go @@ -173,9 +173,10 @@ func TestSnapshotNestedRepo(t *testing.T) { } // TestReadFileStaysInRoot checks that ReadFile reads only the regular files -// in the root. A repository can link to a secret file, or to a device that -// does not end, for example /dev/zero. +// in the root. func TestReadFileStaysInRoot(t *testing.T) { + // A repository can link to a secret file, or to a device that does not + // end, for example /dev/zero. r, dir := newRepo(t) secret := filepath.Join(filepath.Dir(dir), "secret") write(t, filepath.Dir(dir), "secret", "secret\n") @@ -224,9 +225,10 @@ func TestOpenRootWithNewline(t *testing.T) { } } -// TestGitWorktree checks a working tree that "git worktree add" made. Such -// a working tree has its own index. +// TestGitWorktree checks Diff and Snapshot in a working tree that +// "git worktree add" creates. func TestGitWorktree(t *testing.T) { + // Such a working tree has its own index. Snapshot must not change it. _, dir := newRepo(t) ctx := t.Context() wt := filepath.Join(t.TempDir(), "wt") diff --git a/vcs/pr.go b/vcs/pr.go index fc246b6..18b1c52 100644 --- a/vcs/pr.go +++ b/vcs/pr.go @@ -53,6 +53,7 @@ type Check struct { // the service. type Forge string +// The forges that [Repo.PullRequest] supports. const ( ForgeGitHub Forge = "GitHub" ForgeGitLab Forge = "GitLab" @@ -62,6 +63,7 @@ const ( // State is the state of a pull request. It is the same for each forge. type State string +// The values of State. const ( StateOpen State = "open" StateDraft State = "draft" // open, but not ready for review @@ -72,6 +74,7 @@ const ( // Result is the result of a CI check. It is the same for each forge. type Result string +// The values of Result. const ( ResultPass Result = "pass" ResultFail Result = "fail" // also an error, a cancel, or a timeout diff --git a/vcs/refs.go b/vcs/refs.go index 96751e7..30334b3 100644 --- a/vcs/refs.go +++ b/vcs/refs.go @@ -114,7 +114,9 @@ func (r *Repo) Files(ctx context.Context) ([]Entry, error) { args []string ignored bool }{ - {[]string{"ls-files", "-co", "--exclude-standard", "-z"}, false}, + // In a merge conflict, git lists a path once for each stage. + // --deduplicate lists it once. + {[]string{"ls-files", "-co", "--deduplicate", "--exclude-standard", "-z"}, false}, {[]string{"ls-files", "-oi", "--exclude-standard", "--directory", "-z"}, true}, } var es []Entry diff --git a/vcs/repo.go b/vcs/repo.go index 4eef588..66ea07e 100644 --- a/vcs/repo.go +++ b/vcs/repo.go @@ -285,7 +285,7 @@ func (r *Repo) Diff(ctx context.Context, from, to string) ([]File, error) { // then its path. A rename has its old and new paths. fields := strings.Split(strings.TrimSuffix(out, "\x00"), "\x00") var files []File - var ids []string // old and new object of each file, or "" for none + var ids []string // the old and the new object of each file, or "" for none var blobs []blob // the content of each ID for len(fields) >= 2 { meta := strings.Fields(strings.TrimPrefix(fields[0], ":")) @@ -434,7 +434,7 @@ func checkBlob(s string) (string, error) { // isBinary reports whether s has a NUL byte near its start, as git does. func isBinary(s string) bool { - return strings.IndexByte(s[:min(len(s), binaryCheckLen)], 0) >= 0 + return strings.Contains(s[:min(len(s), binaryCheckLen)], "\x00") } // ReadFile reads the file at path in the working tree. The path is relative diff --git a/vcs/repo_test.go b/vcs/repo_test.go index 070e417..1b17875 100644 --- a/vcs/repo_test.go +++ b/vcs/repo_test.go @@ -6,6 +6,7 @@ import ( "os/exec" "path/filepath" "reflect" + "slices" "strings" "testing" "time" @@ -180,9 +181,10 @@ func TestScopes(t *testing.T) { } // TestMissingCommit checks that Head and Parent fail when a commit object -// is missing. The empty tree is not correct, because it shows every file -// as added. +// is missing. func TestMissingCommit(t *testing.T) { + // The empty tree is not a correct result, because it shows every file as + // added. r, dir := newRepo(t) ctx := t.Context() cs, err := r.Commits(ctx, 2) @@ -240,6 +242,43 @@ func TestDiffSkipsBinaryAndBigFiles(t *testing.T) { } } +// TestFilesInConflict checks that Files lists a path in a merge conflict +// once. +func TestFilesInConflict(t *testing.T) { + dir := t.TempDir() + run := func(args ...string) { git(t, dir, args...) } + run("init", "-q", "-b", "main") + write(t, dir, "f.txt", "base\n") + run("add", ".") + run("commit", "-qm", "base") + run("checkout", "-qb", "other") + write(t, dir, "f.txt", "other\n") + run("commit", "-qam", "other") + run("checkout", "-q", "main") + write(t, dir, "f.txt", "main\n") + run("commit", "-qam", "main") + cmd := exec.Command("git", "merge", "other") + cmd.Dir = dir + if err := cmd.Run(); err == nil { + t.Fatal("the merge has no conflict") + } + r, err := vcs.Open(t.Context(), dir) + if err != nil { + t.Fatal(err) + } + es, err := r.Files(t.Context()) + if err != nil { + t.Fatal(err) + } + var got []string + for _, e := range es { + got = append(got, e.Path) + } + if want := []string{"f.txt"}; !slices.Equal(got, want) { + t.Errorf("Files = %q, want %q", got, want) + } +} + // TestWorktree checks Files, Grep, ReadFile, and Contains in the working // tree. func TestWorktree(t *testing.T) { @@ -394,14 +433,12 @@ func TestNewRepo(t *testing.T) { } } -// TestSnapshotRacyEdit checks that Snapshot sees an edit that keeps the -// size of a file. The edit is in the same second as the last write of the -// index. -// -// git checks such "racily clean" files by their content. It finds them by -// the modification time of the index, so the copy of the index must keep -// that time. +// TestSnapshotRacyEdit checks that Snapshot sees a same-size edit in the +// same second as the last write of the index. func TestSnapshotRacyEdit(t *testing.T) { + // git checks such "racily clean" files by their content. It finds them by + // the modification time of the index, so the copy of the index must keep + // that time. r, dir := newRepo(t) git(t, dir, "config", "core.trustctime", "false") git(t, dir, "add", "-A")