Merge pull request #12655 from yuvrajangadsingh/feature/pr-diff-exclude
feat(pr diff): add --exclude flag to filter files from diff output
This commit is contained in:
Babak K. Shandiz
GitHub
commit
4ae2743bf1
2 changed files with 266 additions and 2 deletions
|
|
@ -2,10 +2,12 @@ package diff
|
|||
|
||||
import (
|
||||
"bufio"
|
||||
"bytes"
|
||||
"errors"
|
||||
"fmt"
|
||||
"io"
|
||||
"net/http"
|
||||
"path"
|
||||
"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 {
|
||||
|
|
@ -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)
|
||||
|
|
@ -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
|
||||
}
|
||||
|
|
@ -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)
|
||||
}
|
||||
|
|
@ -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])
|
||||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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