feat(pr diff): add --exclude flag to filter files from diff output
Add a new --exclude (-e) flag to gh pr diff that allows users to exclude files matching glob patterns from the diff output. This is useful for filtering out auto-generated files, vendor directories, or other noise when reviewing pull requests. Supports standard glob patterns and can be specified multiple times. Patterns match against both the full path and basename. Closes #8739
This commit is contained in:
parent
fa43288852
commit
18536dc2b1
2 changed files with 246 additions and 0 deletions
|
|
@ -2,10 +2,12 @@ package diff
|
|||
|
||||
import (
|
||||
"bufio"
|
||||
"bytes"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"path/filepath"
|
||||
"regexp"
|
||||
"strings"
|
||||
"unicode"
|
||||
|
|
@ -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 {
|
||||
|
|
@ -92,6 +95,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
|
||||
}
|
||||
|
|
@ -135,6 +139,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)
|
||||
}
|
||||
|
|
@ -357,3 +368,65 @@ 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(`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, patterns []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([]byte(section))
|
||||
if name != "" && matchesAny(name, patterns) {
|
||||
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 "
|
||||
var sections []string
|
||||
for {
|
||||
idx := strings.Index(diff, marker)
|
||||
if idx == -1 {
|
||||
if len(diff) > 0 {
|
||||
sections = append(sections, diff)
|
||||
}
|
||||
break
|
||||
}
|
||||
sections = append(sections, diff[:idx+1]) // include the trailing \n
|
||||
diff = diff[idx+1:] // next section starts at "diff --git"
|
||||
}
|
||||
return sections
|
||||
}
|
||||
|
||||
func extractFileName(section []byte) string {
|
||||
m := diffHeaderRegexp.FindSubmatch(section)
|
||||
if m == nil {
|
||||
return ""
|
||||
}
|
||||
return strings.TrimSpace(string(m[1]) + string(m[2]))
|
||||
}
|
||||
|
||||
func matchesAny(name string, patterns []string) bool {
|
||||
for _, p := range patterns {
|
||||
if matched, _ := filepath.Match(p, name); matched {
|
||||
return true
|
||||
}
|
||||
// Also match against the basename so "*.yml" matches "dir/file.yml"
|
||||
if matched, _ := filepath.Match(p, filepath.Base(name)); matched {
|
||||
return true
|
||||
}
|
||||
}
|
||||
return false
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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",
|
||||
|
|
@ -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)
|
||||
})
|
||||
}
|
||||
}
|
||||
|
|
@ -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{
|
||||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue