diff --git a/go.mod b/go.mod index 5ad5bf5ab00..a3a55c3cd68 100644 --- a/go.mod +++ b/go.mod @@ -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 diff --git a/go.sum b/go.sum index e1ff3129b49..af66f223f17 100644 --- a/go.sum +++ b/go.sum @@ -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= diff --git a/pkg/cmd/pr/diff/diff.go b/pkg/cmd/pr/diff/diff.go index 6d37bf1e5b8..4d0ee80a39f 100644 --- a/pkg/cmd/pr/diff/diff.go +++ b/pkg/cmd/pr/diff/diff.go @@ -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 +} diff --git a/pkg/cmd/pr/diff/diff_test.go b/pkg/cmd/pr/diff/diff_test.go index ceb93a18790..6a91ba9ca96 100644 --- a/pkg/cmd/pr/diff/diff_test.go +++ b/pkg/cmd/pr/diff/diff_test.go @@ -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"