* fix(diff): add --no-pager and package header to git diff Previously, each package's diff was shown via a separate `git diff` invocation with Show(), allowing git to invoke its own pager (e.g. less) independently for each package. This caused inconsistent pagination: large diffs opened a pager, small diffs scrolled past in the terminal. Changes: - Add --no-pager flag to git diff so git doesn't open its own pager per package; all diffs now stream consistently to stdout. - Print a package name header before each diff so users can easily identify which package each diff belongs to when reviewing multiple packages. Closes #1243 * bunch up diffs and render in one go Co-authored-by: Jo Garnier <me@jguer.space>
This commit is contained in:
1 parent
f76bbba9be
commit
45964c4b42
8 files changed
+246
-14
No files matched your search
@@ -119,11 +119,12 @@ pacman -S --needed git base-devel yay
|
||||
Make sure you have the `Color` option in your `/etc/pacman.conf`
|
||||
(see issue [#123](https://github.com/Jguer/yay/issues/123)).
|
||||
|
||||
- **Sometimes diffs are printed to the terminal, and other times they are paged via less. How do I fix this?**
|
||||
- **How do I change the pager used for PKGBUILD diffs?**
|
||||
|
||||
yay uses `git diff` to display diffs, which by default tells less not to
|
||||
page if the output can fit into one terminal length. This behavior can be
|
||||
overridden by exporting your own flags (`export LESS=SRX`).
|
||||
Diffs for all selected packages are collected and shown in a single pager
|
||||
session. Set `pager` in `config.json` or `yay.opt.pager` in `init.lua`, or
|
||||
set `PAGER` to override the default (`less`, or `cat` if less is unavailable).
|
||||
Example: `yay.opt.pager = "delta"` or `export PAGER=less`.
|
||||
|
||||
- **yay is not asking me to edit PKGBUILDS, and I don't like the diff menu! What can I do?**
|
||||
|
||||
|
||||
@@ -9,6 +9,7 @@ yay.opt.aurrpcurl = ""
|
||||
yay.opt.build_dir = os.getenv("HOME") .. "/.cache/yay"
|
||||
yay.opt.editor = os.getenv("EDITOR") or os.getenv("VISUAL") or "vi"
|
||||
yay.opt.editor_flags = ""
|
||||
yay.opt.pager = os.getenv("PAGER") or ""
|
||||
yay.opt.makepkg_bin = "makepkg"
|
||||
yay.opt.makepkg_conf = ""
|
||||
yay.opt.pacman_bin = "pacman"
|
||||
|
||||
+1
-1
@@ -66,7 +66,7 @@ startup and reports the offending keys/values so misconfigurations fail fast.
|
||||
|
||||
**Strings**
|
||||
|
||||
`aururl`, `aurrpcurl`, `build_dir`, `editor`, `editor_flags`, `makepkg_bin`,
|
||||
`aururl`, `aurrpcurl`, `build_dir`, `editor`, `editor_flags`, `pager`, `makepkg_bin`,
|
||||
`makepkg_conf`, `pacman_bin`, `pacman_conf`, `redownload`, `rebuild`, `git_bin`,
|
||||
`gpg_bin`, `gpg_flags`, `mflags`, `sort_by`, `search_by`, `git_flags`,
|
||||
`remove_make`, `sudo_bin`, `sudo_flags`
|
||||
|
||||
@@ -307,9 +307,10 @@ using '--cleanmenu=false' on the command line
|
||||
Show the diff menu. This menu gives you the option to view diffs from
|
||||
build files before building.
|
||||
|
||||
Diffs are shown via \fBgit diff\fR which uses
|
||||
less by default. This behaviour can be changed via git's config, the
|
||||
\fB$GIT_PAGER\fR or \fB$PAGER\fR environment variables.
|
||||
Diffs for all selected packages are collected and shown through a single
|
||||
pager. The pager is taken from the \fBpager\fR config option (also settable
|
||||
via Lua as \fByay.opt.pager\fR), then \fB$PAGER\fR, then \fBless\fR (or
|
||||
\fBcat\fR if less is unavailable).
|
||||
|
||||
.TP
|
||||
.B \-\-editmenu
|
||||
@@ -540,6 +541,11 @@ Overridden by \-\-builddir.
|
||||
When editor is not configured, use these variables to pick what editor
|
||||
to use when editing PKGBUILDS.
|
||||
|
||||
.TP
|
||||
.B PAGER
|
||||
When pager is not configured, use this variable when showing PKGBUILD diffs.
|
||||
If unset, yay uses \fBless\fR when available, otherwise \fBcat\fR.
|
||||
|
||||
.SH FILES
|
||||
.TP
|
||||
.B CONFIG DIRECTORY
|
||||
|
||||
+46
-6
@@ -23,9 +23,35 @@ const (
|
||||
)
|
||||
|
||||
func showPkgbuildDiffs(ctx context.Context, cmdBuilder exe.ICmdBuilder, logger *text.Logger,
|
||||
pkgbuildDirs map[string]string, bases []string,
|
||||
pkgbuildDirs map[string]string, bases []string, pagerConfig string,
|
||||
) error {
|
||||
var errs []error
|
||||
combined, errs := collectPkgbuildDiffs(ctx, cmdBuilder, logger, pkgbuildDirs, bases)
|
||||
if combined == "" {
|
||||
return errors.Join(errs...)
|
||||
}
|
||||
|
||||
if !isStdoutTerminal() {
|
||||
logger.Print(combined)
|
||||
|
||||
return errors.Join(errs...)
|
||||
}
|
||||
|
||||
if err := runPager(ctx, combined, pagerConfig); err != nil {
|
||||
errs = append(errs, err)
|
||||
}
|
||||
|
||||
return errors.Join(errs...)
|
||||
}
|
||||
|
||||
// collectPkgbuildDiffs captures each selected package's git diff with --no-pager
|
||||
// and joins them with package headers into one buffer for a single pager session.
|
||||
func collectPkgbuildDiffs(ctx context.Context, cmdBuilder exe.ICmdBuilder, logger *text.Logger,
|
||||
pkgbuildDirs map[string]string, bases []string,
|
||||
) (string, []error) {
|
||||
var (
|
||||
errs []error
|
||||
buf strings.Builder
|
||||
)
|
||||
|
||||
for _, pkg := range bases {
|
||||
dir := pkgbuildDirs[pkg]
|
||||
@@ -53,7 +79,7 @@ func showPkgbuildDiffs(ctx context.Context, cmdBuilder exe.ICmdBuilder, logger *
|
||||
}
|
||||
|
||||
args := []string{
|
||||
"diff",
|
||||
"--no-pager", "diff",
|
||||
start + "..HEAD@{upstream}", "--src-prefix",
|
||||
dir + "/", "--dst-prefix", dir + "/", "--", ".", ":(exclude).SRCINFO",
|
||||
}
|
||||
@@ -63,10 +89,24 @@ func showPkgbuildDiffs(ctx context.Context, cmdBuilder exe.ICmdBuilder, logger *
|
||||
args = append(args, "--color=never")
|
||||
}
|
||||
|
||||
_ = cmdBuilder.Show(cmdBuilder.BuildGitCmd(ctx, dir, args...))
|
||||
stdout, stderr, err := cmdBuilder.Capture(cmdBuilder.BuildGitCmd(ctx, dir, args...))
|
||||
if err != nil {
|
||||
errs = append(errs, fmt.Errorf("%s%w", stderr, err))
|
||||
|
||||
continue
|
||||
}
|
||||
|
||||
if stdout == "" {
|
||||
continue
|
||||
}
|
||||
|
||||
buf.WriteString(logger.SprintOperationInfo(gotext.Get("Showing diff for %s", text.Bold(pkg))))
|
||||
buf.WriteByte('\n')
|
||||
buf.WriteString(stdout)
|
||||
buf.WriteString("\n\n")
|
||||
}
|
||||
|
||||
return errors.Join(errs...)
|
||||
return buf.String(), errs
|
||||
}
|
||||
|
||||
// Check whether or not a diff exists between the last reviewed diff and
|
||||
@@ -163,7 +203,7 @@ func DiffFn(ctx context.Context, run *runtime.Runtime, w io.Writer,
|
||||
return errMenu
|
||||
}
|
||||
|
||||
if errD := showPkgbuildDiffs(ctx, run.CmdBuilder, run.Logger, pkgbuildDirsByBase, toDiff); errD != nil {
|
||||
if errD := showPkgbuildDiffs(ctx, run.CmdBuilder, run.Logger, pkgbuildDirsByBase, toDiff, run.Cfg.Pager); errD != nil {
|
||||
return errD
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,54 @@
|
||||
package menus
|
||||
|
||||
import (
|
||||
"context"
|
||||
"os"
|
||||
"os/exec"
|
||||
"strings"
|
||||
|
||||
"golang.org/x/term"
|
||||
)
|
||||
|
||||
// runPager pages content through a single pager process. Overridable in tests.
|
||||
var runPager = pageThroughPager
|
||||
|
||||
// isStdoutTerminal reports whether stdout is a terminal. Overridable in tests.
|
||||
var isStdoutTerminal = func() bool {
|
||||
return term.IsTerminal(int(os.Stdout.Fd()))
|
||||
}
|
||||
|
||||
// resolvePager returns the pager command string.
|
||||
// Priority: config pager → PAGER → less → cat.
|
||||
func resolvePager(pagerConfig string) string {
|
||||
if pagerConfig != "" {
|
||||
return pagerConfig
|
||||
}
|
||||
|
||||
if pager := os.Getenv("PAGER"); pager != "" {
|
||||
return pager
|
||||
}
|
||||
|
||||
if _, err := exec.LookPath("less"); err == nil {
|
||||
return "less"
|
||||
}
|
||||
|
||||
return "cat"
|
||||
}
|
||||
|
||||
// pageThroughPager runs the configured pager with content on stdin.
|
||||
// The pager command is user-controlled (config / $PAGER), same model as $EDITOR.
|
||||
func pageThroughPager(ctx context.Context, content, pagerConfig string) error {
|
||||
pager := resolvePager(pagerConfig)
|
||||
cmd := exec.CommandContext(ctx, "sh", "-c", pager)
|
||||
cmd.Stdin = strings.NewReader(content)
|
||||
cmd.Stdout = os.Stdout
|
||||
cmd.Stderr = os.Stderr
|
||||
cmd.Env = os.Environ()
|
||||
|
||||
if os.Getenv("LESS") == "" {
|
||||
// S: chop long lines; R: raw ANSI; X: no termcap init; F: quit if one screen
|
||||
cmd.Env = append(cmd.Env, "LESS=SRXF")
|
||||
}
|
||||
|
||||
return cmd.Run()
|
||||
}
|
||||
@@ -0,0 +1,127 @@
|
||||
//go:build !integration
|
||||
|
||||
package menus
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
"os/exec"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
|
||||
"github.com/Jguer/yay/v13/pkg/text"
|
||||
)
|
||||
|
||||
func TestResolvePager(t *testing.T) {
|
||||
// Not parallel: t.Setenv cannot be used after t.Parallel.
|
||||
tests := []struct {
|
||||
name string
|
||||
pagerConfig string
|
||||
pager string
|
||||
want string
|
||||
}{
|
||||
{name: "config wins", pagerConfig: "delta", pager: "less", want: "delta"},
|
||||
{name: "PAGER when config empty", pagerConfig: "", pager: "most", want: "most"},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
t.Setenv("PAGER", tt.pager)
|
||||
|
||||
assert.Equal(t, tt.want, resolvePager(tt.pagerConfig))
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
func TestCollectPkgbuildDiffs(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var warnOut bytes.Buffer
|
||||
logger := text.NewLogger(&bytes.Buffer{}, &warnOut, strings.NewReader(""), false, "test")
|
||||
|
||||
builder := &fakeMenusCmdBuilder{
|
||||
captureFn: func(cmd *exec.Cmd) (string, string, error) {
|
||||
joined := strings.Join(cmd.Args, " ")
|
||||
|
||||
switch {
|
||||
case strings.Contains(joined, "--quiet --verify AUR_SEEN"):
|
||||
return "", "", nil
|
||||
case strings.Contains(joined, "rev-parse AUR_SEEN HEAD@{upstream}"):
|
||||
return "aaa\nbbb\n", "", nil
|
||||
case strings.Contains(joined, "rev-parse AUR_SEEN"):
|
||||
return "aaa\n", "", nil
|
||||
case strings.Contains(joined, "--no-pager diff"):
|
||||
require.Contains(t, joined, "--no-pager")
|
||||
pkg := "alpha"
|
||||
if strings.Contains(joined, "beta/") {
|
||||
pkg = "beta"
|
||||
}
|
||||
|
||||
return "diff for " + pkg, "", nil
|
||||
default:
|
||||
t.Fatalf("unexpected git args: %s", joined)
|
||||
|
||||
return "", "", nil
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
dirs := map[string]string{
|
||||
"alpha": "/tmp/alpha",
|
||||
"beta": "/tmp/beta",
|
||||
}
|
||||
|
||||
out, errs := collectPkgbuildDiffs(context.Background(), builder, logger, dirs, []string{"alpha", "beta"})
|
||||
require.Empty(t, errs)
|
||||
require.Contains(t, out, "Showing diff for")
|
||||
require.Contains(t, out, "diff for alpha")
|
||||
require.Contains(t, out, "diff for beta")
|
||||
// Headers for both packages appear before their diffs in one buffer.
|
||||
alphaIdx := strings.Index(out, "diff for alpha")
|
||||
betaIdx := strings.Index(out, "diff for beta")
|
||||
require.Positive(t, alphaIdx)
|
||||
require.Greater(t, betaIdx, alphaIdx)
|
||||
}
|
||||
|
||||
func TestShowPkgbuildDiffsPagesOnce(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
var paged strings.Builder
|
||||
origRunPager := runPager
|
||||
origIsTTY := isStdoutTerminal
|
||||
t.Cleanup(func() {
|
||||
runPager = origRunPager
|
||||
isStdoutTerminal = origIsTTY
|
||||
})
|
||||
|
||||
isStdoutTerminal = func() bool { return true }
|
||||
runPager = func(_ context.Context, content, _ string) error {
|
||||
paged.WriteString(content)
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
logger := text.NewLogger(&bytes.Buffer{}, &bytes.Buffer{}, strings.NewReader(""), false, "test")
|
||||
builder := &fakeMenusCmdBuilder{
|
||||
captureFn: func(cmd *exec.Cmd) (string, string, error) {
|
||||
joined := strings.Join(cmd.Args, " ")
|
||||
switch {
|
||||
case strings.Contains(joined, "--quiet --verify AUR_SEEN"):
|
||||
return "", "", exec.ErrNotFound // no AUR_SEEN → empty tree, always show
|
||||
case strings.Contains(joined, "--no-pager diff"):
|
||||
return "+++ changed", "", nil
|
||||
default:
|
||||
return "", "", nil
|
||||
}
|
||||
},
|
||||
}
|
||||
|
||||
err := showPkgbuildDiffs(context.Background(), builder, logger,
|
||||
map[string]string{"pkg": "/tmp/pkg"}, []string{"pkg"}, "")
|
||||
require.NoError(t, err)
|
||||
require.Contains(t, paged.String(), "+++ changed")
|
||||
require.Contains(t, paged.String(), "Showing diff for")
|
||||
}
|
||||
@@ -28,6 +28,7 @@ type Configuration struct {
|
||||
BuildDir string `json:"buildDir" lua:"build_dir"`
|
||||
Editor string `json:"editor" lua:"editor"`
|
||||
EditorFlags string `json:"editorflags" lua:"editor_flags"`
|
||||
Pager string `json:"pager" lua:"pager"`
|
||||
MakepkgBin string `json:"makepkgbin" lua:"makepkg_bin"`
|
||||
MakepkgConf string `json:"makepkgconf" lua:"makepkg_conf"`
|
||||
PacmanBin string `json:"pacmanbin" lua:"pacman_bin"`
|
||||
@@ -115,6 +116,7 @@ func (c *Configuration) expandEnv() {
|
||||
c.BuildDir = expandEnvOrHome(c.BuildDir)
|
||||
c.Editor = expandEnvOrHome(c.Editor)
|
||||
c.EditorFlags = os.ExpandEnv(c.EditorFlags)
|
||||
c.Pager = os.ExpandEnv(c.Pager)
|
||||
c.MakepkgBin = expandEnvOrHome(c.MakepkgBin)
|
||||
c.MakepkgConf = expandEnvOrHome(c.MakepkgConf)
|
||||
c.PacmanBin = expandEnvOrHome(c.PacmanBin)
|
||||
@@ -196,6 +198,7 @@ func DefaultConfig(version string) *Configuration {
|
||||
KeepSrc: false,
|
||||
Editor: "",
|
||||
EditorFlags: "",
|
||||
Pager: "",
|
||||
Devel: false,
|
||||
MakepkgBin: "makepkg",
|
||||
MakepkgConf: "",
|
||||
|
||||
Reference in new issue
Block a user