Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ require (
github.com/yuin/goldmark v1.7.16
github.com/zalando/go-keyring v0.2.6
golang.org/x/crypto v0.48.0
golang.org/x/sync v0.19.0
golang.org/x/sync v0.20.0
golang.org/x/term v0.40.0
golang.org/x/text v0.34.0
google.golang.org/grpc v1.79.2
Expand Down
4 changes: 2 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -593,8 +593,8 @@ golang.org/x/oauth2 v0.34.0/go.mod h1:lzm5WQJQwKZ3nwavOZ3IS5Aulzxi68dUSgRHujetwE
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.19.0 h1:vV+1eWNmZ5geRlYjzm2adRgW2/mcpevXNg50YZtPCE4=
golang.org/x/sync v0.19.0/go.mod h1:9KTHXmSnoGruLpwFjVSX0lNNA75CykiMECbovNTZqGI=
golang.org/x/sync v0.20.0 h1:e0PTpb7pjO8GAtTs2dQ6jYa5BWYlMuX047Dco/pItO4=
golang.org/x/sync v0.20.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
Expand Down
95 changes: 93 additions & 2 deletions pkg/cmd/pr/diff/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,12 @@ package diff

import (
"bufio"
"bytes"
"errors"
"fmt"
"io"
"net/http"
"path"
"regexp"
"strings"
"unicode"
Expand Down Expand Up @@ -36,6 +38,7 @@ type DiffOptions struct {
Patch bool
NameOnly bool
BrowserMode bool
Exclude []string
}

func NewCmdDiff(f *cmdutil.Factory, runF func(*DiffOptions) error) *cobra.Command {
Expand All @@ -57,7 +60,24 @@ func NewCmdDiff(f *cmdutil.Factory, runF func(*DiffOptions) error) *cobra.Comman
is selected.

With %[1]s--web%[1]s flag, open the pull request diff in a web browser instead.

Use %[1]s--exclude%[1]s to filter out files matching a glob pattern. The pattern
uses forward slashes as path separators on all platforms. You can repeat
the flag to exclude multiple patterns.
`, "`"),
Example: heredoc.Doc(`
# See diff for current branch
$ gh pr diff

# See diff for a specific PR
$ gh pr diff 123

# Exclude files from diff output
$ gh pr diff --exclude '*.yml' --exclude 'generated/*'

# Exclude matching files by name
$ gh pr diff --name-only --exclude '*.generated.*'
`),
Args: cobra.MaximumNArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
opts.Finder = shared.NewFinder(f)
Expand Down Expand Up @@ -92,6 +112,7 @@ func NewCmdDiff(f *cmdutil.Factory, runF func(*DiffOptions) error) *cobra.Comman
cmd.Flags().BoolVar(&opts.Patch, "patch", false, "Display diff in patch format")
cmd.Flags().BoolVar(&opts.NameOnly, "name-only", false, "Display only names of changed files")
cmd.Flags().BoolVarP(&opts.BrowserMode, "web", "w", false, "Open the pull request diff in the browser")
cmd.Flags().StringSliceVarP(&opts.Exclude, "exclude", "e", nil, "Exclude files matching glob `patterns` from the diff")

return cmd
}
Expand Down Expand Up @@ -135,6 +156,13 @@ func diffRun(opts *DiffOptions) error {
defer diffReadCloser.Close()

var diff io.Reader = diffReadCloser
if len(opts.Exclude) > 0 {
filtered, err := filterDiff(diff, opts.Exclude)
if err != nil {
return err
}
diff = filtered
}
if opts.IO.IsStdoutTTY() {
diff = sanitizedReader(diff)
}
Expand Down Expand Up @@ -292,8 +320,7 @@ func changedFilesNames(w io.Writer, r io.Reader) error {
// `"`` + hello-\360\237\230\200-world"
//
// Where I'm using the `` to indicate a string to avoid confusion with the " character.
pattern := regexp.MustCompile(`(?:^|\n)diff\s--git.*\s(["]?)b/(.*)`)
matches := pattern.FindAllStringSubmatch(string(diff), -1)
matches := diffHeaderRegexp.FindAllStringSubmatch(string(diff), -1)

for _, val := range matches {
name := strings.TrimSpace(val[1] + val[2])
Expand Down Expand Up @@ -357,3 +384,67 @@ func (t sanitizer) Transform(dst, src []byte, atEOF bool) (nDst, nSrc int, err e
func isPrint(r rune) bool {
return r == '\n' || r == '\r' || r == '\t' || unicode.IsPrint(r)
}

var diffHeaderRegexp = regexp.MustCompile(`(?:^|\n)diff\s--git.*\s("?)b/(.*)`)

// filterDiff reads a unified diff and returns a new reader with file entries
// matching any of the exclude patterns removed.
func filterDiff(r io.Reader, excludePatterns []string) (io.Reader, error) {
data, err := io.ReadAll(r)
if err != nil {
return nil, err
}

var result bytes.Buffer
for _, section := range splitDiffSections(string(data)) {
name := extractFileName(section)
if name != "" && matchesAny(name, excludePatterns) {
continue
}
result.WriteString(section)
}
return &result, nil
}

// splitDiffSections splits a unified diff string into per-file sections.
// Each section starts with "diff --git" and includes all content up to (but
// not including) the next "diff --git" line.
func splitDiffSections(diff string) []string {
marker := "\ndiff --git "
parts := strings.Split(diff, marker)
if len(parts) == 1 {
return []string{diff}
}
sections := make([]string, 0, len(parts))
for i, p := range parts {
if i == 0 {
if len(p) > 0 {
sections = append(sections, p+"\n")
}
} else {
sections = append(sections, "diff --git "+p)
}
}
return sections
}

func extractFileName(section string) string {
m := diffHeaderRegexp.FindStringSubmatch(section)
if m == nil {
return ""
}
return strings.TrimSpace(m[1] + m[2])
}

func matchesAny(name string, excludePatterns []string) bool {
for _, p := range excludePatterns {
if matched, _ := path.Match(p, name); matched {
return true
}
// Also match against the basename so "*.yml" matches "dir/file.yml"
if matched, _ := path.Match(p, path.Base(name)); matched {
return true
}
}
return false
}
173 changes: 173 additions & 0 deletions pkg/cmd/pr/diff/diff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,26 @@ func Test_NewCmdDiff(t *testing.T) {
isTTY: true,
wantErr: "argument required when using the `--repo` flag",
},
{
name: "exclude single pattern",
args: "--exclude '*.yml'",
isTTY: true,
want: DiffOptions{
SelectorArg: "",
UseColor: true,
Exclude: []string{"*.yml"},
},
},
{
name: "exclude multiple patterns",
args: "--exclude '*.yml' --exclude Makefile",
isTTY: true,
want: DiffOptions{
SelectorArg: "",
UseColor: true,
Exclude: []string{"*.yml", "Makefile"},
},
},
{
name: "invalid --color argument",
args: "--color doublerainbow",
Expand Down Expand Up @@ -142,6 +162,7 @@ func Test_NewCmdDiff(t *testing.T) {
assert.Equal(t, tt.want.SelectorArg, opts.SelectorArg)
assert.Equal(t, tt.want.UseColor, opts.UseColor)
assert.Equal(t, tt.want.BrowserMode, opts.BrowserMode)
assert.Equal(t, tt.want.Exclude, opts.Exclude)
})
}
}
Expand Down Expand Up @@ -211,6 +232,48 @@ func Test_diffRun(t *testing.T) {
stubDiffRequest(reg, "application/vnd.github.v3.diff", fmt.Sprintf(testDiff, "", "", "", ""))
},
},
{
name: "exclude yml files",
opts: DiffOptions{
SelectorArg: "123",
UseColor: false,
Exclude: []string{"*.yml"},
},
wantFields: []string{"number"},
wantStdout: `diff --git a/Makefile b/Makefile
index f2b4805c..3d7bd0f9 100644
--- a/Makefile
+++ b/Makefile
@@ -22,8 +22,8 @@ test:
go test ./...
.PHONY: test

-site:
- git clone https://github.com/github/cli.github.com.git "$@"
+site: bin/gh
+ bin/gh repo clone github/cli.github.com "$@"

site-docs: site
git -C site pull
`,
httpStubs: func(reg *httpmock.Registry) {
stubDiffRequest(reg, "application/vnd.github.v3.diff", fmt.Sprintf(testDiff, "", "", "", ""))
},
},
{
name: "name only with exclude",
opts: DiffOptions{
SelectorArg: "123",
UseColor: false,
NameOnly: true,
Exclude: []string{"*.yml"},
},
wantFields: []string{"number"},
wantStdout: "Makefile\n",
httpStubs: func(reg *httpmock.Registry) {
stubDiffRequest(reg, "application/vnd.github.v3.diff", fmt.Sprintf(testDiff, "", "", "", ""))
},
},
{
name: "web mode",
opts: DiffOptions{
Expand Down Expand Up @@ -394,6 +457,116 @@ func stubDiffRequest(reg *httpmock.Registry, accept, diff string) {
})
}

func Test_filterDiff(t *testing.T) {
rawDiff := fmt.Sprintf(testDiff, "", "", "", "")

tests := []struct {
name string
patterns []string
want string
}{
{
name: "exclude yml files",
patterns: []string{"*.yml"},
want: `diff --git a/Makefile b/Makefile
index f2b4805c..3d7bd0f9 100644
--- a/Makefile
+++ b/Makefile
@@ -22,8 +22,8 @@ test:
go test ./...
.PHONY: test

-site:
- git clone https://github.com/github/cli.github.com.git "$@"
+site: bin/gh
+ bin/gh repo clone github/cli.github.com "$@"

site-docs: site
git -C site pull
`,
},
{
name: "exclude Makefile",
patterns: []string{"Makefile"},
want: `diff --git a/.github/workflows/releases.yml b/.github/workflows/releases.yml
index 73974448..b7fc0154 100644
--- a/.github/workflows/releases.yml
+++ b/.github/workflows/releases.yml
@@ -44,6 +44,11 @@ jobs:
token: ${{secrets.SITE_GITHUB_TOKEN}}
- name: Publish documentation site
if: "!contains(github.ref, '-')" # skip prereleases
+ env:
+ GIT_COMMITTER_NAME: cli automation
+ GIT_AUTHOR_NAME: cli automation
+ GIT_COMMITTER_EMAIL: noreply@github.com
+ GIT_AUTHOR_EMAIL: noreply@github.com
run: make site-publish
- name: Move project cards
if: "!contains(github.ref, '-')" # skip prereleases
`,
},
{
name: "exclude all files",
patterns: []string{"*.yml", "Makefile"},
want: "",
},
{
name: "no matches",
patterns: []string{"*.go"},
want: rawDiff,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
reader, err := filterDiff(strings.NewReader(rawDiff), tt.patterns)
require.NoError(t, err)
got, err := io.ReadAll(reader)
require.NoError(t, err)
assert.Equal(t, tt.want, string(got))
})
}
}

func Test_matchesAny(t *testing.T) {
tests := []struct {
name string
filename string
patterns []string
want bool
}{
{
name: "exact match",
filename: "Makefile",
patterns: []string{"Makefile"},
want: true,
},
{
name: "glob extension",
filename: ".github/workflows/releases.yml",
patterns: []string{"*.yml"},
want: true,
},
{
name: "no match",
filename: "main.go",
patterns: []string{"*.yml"},
want: false,
},
{
name: "directory glob",
filename: ".github/workflows/releases.yml",
patterns: []string{".github/*/*"},
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
assert.Equal(t, tt.want, matchesAny(tt.filename, tt.patterns))
})
}
}

func Test_sanitizedReader(t *testing.T) {
input := strings.NewReader("\t hello \x1B[m world! ăѣ𝔠ծề\r\n")
expected := "\t hello \\u{1b}[m world! ăѣ𝔠ծề\r\n"
Expand Down
Loading