From db5faba553fdd1aacfeed35688c7e8d06f048410 Mon Sep 17 00:00:00 2001 From: Nathan Nutter Date: Wed, 14 Oct 2026 15:24:59 -0500 Subject: [PATCH 1/2] Add a difft shortcut to git-review log Let users view feature commits with Difftastic without configuring Git for each invocation. The shortcut enables patches and external diffs only for the child Git process, leaving saved configuration unchanged. --- README.md | 4 ++++ internal/cli/log.go | 14 +++++++++++--- internal/cli/log_test.go | 30 ++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 7ccd175..f57c007 100644 --- a/README.md +++ b/README.md @@ -70,12 +70,16 @@ git-review log git-review log -p git-review log -pw git-review log --stat +git-review log --difft ``` This command runs `git log @{u}..HEAD --reverse`, so commits appear in oldest-first order. Use `-p` or `--patch` to show patches. Use `--stat` to show diff statistics, with or without patches. Use `-w` or `--ignore-all-space` to ignore whitespace changes in patches. +Use `--difft` to show patches with [Difftastic](https://difftastic.wilfred.me.uk/), which requires `difft` on `PATH`. +This option implies `--patch` and runs Git with `-c diff.external=difft` and `--ext-diff`. +It does not change your saved Git configuration. Output uses Git's format and pager without reading or changing review state. The command does not support `--json` or revision arguments. Unlike review commands, it uses the upstream range directly and permits merge commits. diff --git a/internal/cli/log.go b/internal/cli/log.go index 5960d4d..7da27a7 100644 --- a/internal/cli/log.go +++ b/internal/cli/log.go @@ -8,7 +8,7 @@ import ( ) func (a *application) logCommand() *cobra.Command { - var patch, ignoreAllSpace, stat bool + var patch, ignoreAllSpace, stat, difft bool cmd := &cobra.Command{ Use: "log", Short: "Show commits after the upstream in oldest-first order", @@ -18,8 +18,15 @@ func (a *application) logCommand() *cobra.Command { if a.json { return fmt.Errorf("log does not support --json") } - args := []string{"log", "@{u}..HEAD", "--reverse"} - if patch { + var args []string + if difft { + args = append(args, "-c", "diff.external=difft") + } + args = append(args, "log", "@{u}..HEAD", "--reverse") + if difft { + args = append(args, "--ext-diff") + } + if patch || difft { args = append(args, "-p") } if ignoreAllSpace { @@ -42,5 +49,6 @@ func (a *application) logCommand() *cobra.Command { cmd.Flags().BoolVarP(&patch, "patch", "p", false, "Show the patch for each commit") cmd.Flags().BoolVarP(&ignoreAllSpace, "ignore-all-space", "w", false, "Ignore whitespace changes in patches") cmd.Flags().BoolVar(&stat, "stat", false, "Show diff statistics for each commit") + cmd.Flags().BoolVar(&difft, "difft", false, "Show patches with difft (requires difft on PATH)") return cmd } diff --git a/internal/cli/log_test.go b/internal/cli/log_test.go index 3ac2759..4a27805 100644 --- a/internal/cli/log_test.go +++ b/internal/cli/log_test.go @@ -69,6 +69,36 @@ func TestLogShowsUpstreamCommitsAndOptionalPatches(t *testing.T) { require.Empty(t, terminal) } +func TestLogDifft(t *testing.T) { + dir := featureRepo(t) + require.NoError(t, os.WriteFile(filepath.Join(dir, "example.txt"), []byte("example content\n"), 0o600)) + git(t, dir, "add", "example.txt") + git(t, dir, "commit", "-m", "Add example") + git(t, dir, "config", "diff.external", "false") + tools := t.TempDir() + difft := filepath.Join(tools, "difft") + require.NoError(t, os.WriteFile(difft, []byte("#!/bin/sh\nprintf 'External diff: %s\\n' \"$1\"\ncat \"$5\"\n"), 0o700)) + t.Setenv("PATH", tools+string(os.PathListSeparator)+os.Getenv("PATH")) + + out, terminal, err := execute(t, dir, "log", "-p") + require.NoError(t, err) + require.Empty(t, terminal) + require.Contains(t, out, "+example content\n") + require.NotContains(t, out, "External diff:") + + out, terminal, err = execute(t, dir, "log", "--difft") + require.NoError(t, err) + require.Empty(t, terminal) + require.Contains(t, out, "External diff: example.txt\nexample content\n") + require.NotContains(t, out, "diff --git") + require.Equal(t, "false", git(t, dir, "config", "diff.external")) + + require.NoError(t, os.WriteFile(difft, []byte("#!/bin/sh\necho 'difft failed' >&2\nexit 1\n"), 0o700)) + _, terminal, err = execute(t, dir, "log", "--difft") + require.ErrorContains(t, err, "git log:") + require.Contains(t, terminal, "difft failed") +} + func TestLogReportsGitAndUsageErrors(t *testing.T) { dir := featureRepo(t) git(t, dir, "branch", "--unset-upstream") From 63e397abc8a1ab2a3efbd7e04820f07c7e6a3837 Mon Sep 17 00:00:00 2001 From: Nathan Nutter Date: Wed, 14 Oct 2026 15:26:52 -0500 Subject: [PATCH 2/2] Add a hunk pager shortcut to git-review log Let users browse feature patches with hunk pager without changing their saved Git configuration. Keep Git responsible for terminal detection so redirected output remains unpaged. The shortcuts can compose: difft produces the diff and hunk receives it as pager input. Tests exercise this through a pseudo-terminal. --- README.md | 8 +++++- internal/cli/log.go | 10 +++++-- internal/cli/log_test.go | 62 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 76 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index f57c007..524f41f 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,7 @@ git-review log -p git-review log -pw git-review log --stat git-review log --difft +git-review log --hunk ``` This command runs `git log @{u}..HEAD --reverse`, so commits appear in oldest-first order. @@ -79,7 +80,12 @@ Use `--stat` to show diff statistics, with or without patches. Use `-w` or `--ignore-all-space` to ignore whitespace changes in patches. Use `--difft` to show patches with [Difftastic](https://difftastic.wilfred.me.uk/), which requires `difft` on `PATH`. This option implies `--patch` and runs Git with `-c diff.external=difft` and `--ext-diff`. -It does not change your saved Git configuration. +Use `--hunk` to show patches through `hunk pager`, which requires `hunk` on `PATH`. +This option implies `--patch` and runs Git with `-c core.pager='hunk pager'`. +Git starts the pager only when stdout is a terminal, so redirected output remains unpaged. +You can combine `--hunk` and `--difft` to send Difftastic output through `hunk pager`. +Neither option changes your saved Git configuration. +Git's normal precedence rules still apply, including overrides from `GIT_EXTERNAL_DIFF` and `GIT_PAGER`. Output uses Git's format and pager without reading or changing review state. The command does not support `--json` or revision arguments. Unlike review commands, it uses the upstream range directly and permits merge commits. diff --git a/internal/cli/log.go b/internal/cli/log.go index 7da27a7..d6ac059 100644 --- a/internal/cli/log.go +++ b/internal/cli/log.go @@ -8,17 +8,20 @@ import ( ) func (a *application) logCommand() *cobra.Command { - var patch, ignoreAllSpace, stat, difft bool + var patch, ignoreAllSpace, stat, difft, hunk bool cmd := &cobra.Command{ Use: "log", Short: "Show commits after the upstream in oldest-first order", - Long: "Run git log @{u}..HEAD --reverse with optional patches, diff statistics, and whitespace filtering.\nOutput uses Git's format and pager, not git-review's review state.", + Long: "Run git log @{u}..HEAD --reverse with optional patches, diff statistics, and whitespace filtering.\nUse --difft for Difftastic or --hunk for hunk pager. Both imply --patch.\nOutput uses Git's format and pager, not git-review's review state.", Args: cobra.NoArgs, RunE: func(cmd *cobra.Command, _ []string) error { if a.json { return fmt.Errorf("log does not support --json") } var args []string + if hunk { + args = append(args, "-c", "core.pager=hunk pager") + } if difft { args = append(args, "-c", "diff.external=difft") } @@ -26,7 +29,7 @@ func (a *application) logCommand() *cobra.Command { if difft { args = append(args, "--ext-diff") } - if patch || difft { + if patch || difft || hunk { args = append(args, "-p") } if ignoreAllSpace { @@ -50,5 +53,6 @@ func (a *application) logCommand() *cobra.Command { cmd.Flags().BoolVarP(&ignoreAllSpace, "ignore-all-space", "w", false, "Ignore whitespace changes in patches") cmd.Flags().BoolVar(&stat, "stat", false, "Show diff statistics for each commit") cmd.Flags().BoolVar(&difft, "difft", false, "Show patches with difft (requires difft on PATH)") + cmd.Flags().BoolVar(&hunk, "hunk", false, "Show patches with hunk pager (requires hunk on PATH)") return cmd } diff --git a/internal/cli/log_test.go b/internal/cli/log_test.go index 4a27805..bb70bd2 100644 --- a/internal/cli/log_test.go +++ b/internal/cli/log_test.go @@ -1,11 +1,16 @@ package cli import ( + "bytes" + "context" "os" + "os/exec" "path/filepath" "strings" "testing" + "time" + "github.com/creack/pty" "github.com/stretchr/testify/require" ) @@ -99,6 +104,63 @@ func TestLogDifft(t *testing.T) { require.Contains(t, terminal, "difft failed") } +func TestLogHunkPager(t *testing.T) { + binary := buildGitReview(t) + dir := featureRepo(t) + require.NoError(t, os.WriteFile(filepath.Join(dir, "example.txt"), []byte("example content\n"), 0o600)) + git(t, dir, "add", "example.txt") + git(t, dir, "commit", "-m", "Add example") + git(t, dir, "config", "core.pager", "false") + git(t, dir, "config", "color.ui", "false") + tools := t.TempDir() + capture := filepath.Join(tools, "pager-output") + require.NoError(t, os.WriteFile(filepath.Join(tools, "hunk"), []byte("#!/bin/sh\n[ \"$1\" = pager ] || exit 1\ncat > \"$HUNK_OUTPUT\"\n"), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(tools, "difft"), []byte("#!/bin/sh\nprintf 'External diff: %s\\n' \"$1\"\ncat \"$5\"\n"), 0o700)) + t.Setenv("PATH", tools+string(os.PathListSeparator)+os.Getenv("PATH")) + t.Setenv("HUNK_OUTPUT", capture) + t.Setenv("TERM", "xterm") + + out, terminal, err := executeBinary(t, binary, dir, "log", "--hunk") + require.NoError(t, err) + require.Empty(t, terminal) + require.Contains(t, out, "+example content\n") + _, err = os.Stat(capture) + require.ErrorIs(t, err, os.ErrNotExist) + + for _, tc := range []struct { + name string + flags []string + want string + }{ + {name: "hunk", flags: []string{"--hunk"}, want: "+example content\n"}, + {name: "hunk and difft", flags: []string{"--hunk", "--difft"}, want: "External diff: example.txt\nexample content\n"}, + } { + t.Run(tc.name, func(t *testing.T) { + master, slave, err := pty.Open() + require.NoError(t, err) + t.Cleanup(func() { + require.NoError(t, slave.Close()) + require.NoError(t, master.Close()) + }) + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + cmd := exec.CommandContext(ctx, binary, append([]string{"log"}, tc.flags...)...) + cmd.Dir = dir + cmd.Stdout = slave + var stderr bytes.Buffer + cmd.Stderr = &stderr + // The fake pager writes to capture, so the terminal needs no reader. + require.NoError(t, cmd.Run(), "%s", stderr.String()) + saved, err := os.ReadFile(capture) + require.NoError(t, err) + require.Contains(t, string(saved), tc.want) + require.Contains(t, string(saved), "Add example") + require.NoError(t, os.Remove(capture)) + }) + } + require.Equal(t, "false", git(t, dir, "config", "core.pager")) +} + func TestLogReportsGitAndUsageErrors(t *testing.T) { dir := featureRepo(t) git(t, dir, "branch", "--unset-upstream")