Skip to content

Commit 2b73a35

Browse files
GrantBirkiGrantBirki
authored
Merge pull request #12 from github/case-sensitive-label-matching-or-not
feat: Label Filter Case-Sensitivity Improvements
2 parents 5b312fd + 5be5e66 commit 2b73a35

File tree

4 files changed

+140
-63
lines changed

4 files changed

+140
-63
lines changed

‎internal/cmd/match_criteria.go

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"github.com/cli/go-gh/v2/pkg/api"
1111
graphql "github.com/cli/shurcooL-graphql"
12+
"github.com/github/gh-combine/internal/common"
1213
)
1314

1415
// checks if a PR matches all filtering criteria
@@ -19,7 +20,7 @@ func PrMatchesCriteria(branch string, prLabels []string) bool {
1920
}
2021

2122
// Check label criteria if any are specified
22-
if !labelsMatch(prLabels, ignoreLabels, selectLabels) {
23+
if !labelsMatch(prLabels, ignoreLabels, selectLabels, caseSensitiveLabels) {
2324
return false
2425
}
2526

@@ -71,12 +72,19 @@ func branchMatchesCriteria(branch string) bool {
7172
return true
7273
}
7374

74-
func labelsMatch(prLabels, ignoreLabels, selectLabels []string) bool {
75+
func labelsMatch(prLabels, ignoreLabels, selectLabels []string, caseSensitive bool) bool {
7576
// If no ignoreLabels or selectLabels are specified, all labels pass this check
7677
if len(ignoreLabels) == 0 && len(selectLabels) == 0 {
7778
return true
7879
}
7980

81+
// Normalize labels for case-insensitive matching if caseSensitive is false
82+
if !caseSensitive {
83+
prLabels = common.NormalizeArray(prLabels)
84+
ignoreLabels = common.NormalizeArray(ignoreLabels)
85+
selectLabels = common.NormalizeArray(selectLabels)
86+
}
87+
8088
// If the pull request contains any of the ignore labels, it doesn't match
8189
for _, l := range ignoreLabels {
8290
if slices.Contains(prLabels, l) {

‎internal/cmd/match_criteria_test.go

Lines changed: 115 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -6,107 +6,161 @@ func TestLabelsMatch(t *testing.T) {
66
t.Parallel()
77

88
tests := []struct {
9-
prLabels []string
10-
ignoreLabels []string
11-
selectLabels []string
12-
want bool
9+
name string
10+
prLabels []string
11+
ignoreLabels []string
12+
selectLabels []string
13+
caseSensitive bool
14+
want bool
1315
}{
1416
{
1517
want: true,
1618
},
1719

1820
{
19-
prLabels: []string{"a", "b"},
20-
ignoreLabels: []string{"b"},
21-
want: false,
21+
name: "--ignore-labels match",
22+
prLabels: []string{"a", "b"},
23+
ignoreLabels: []string{"b"},
24+
want: false,
25+
caseSensitive: false,
2226
},
2327
{
24-
prLabels: []string{"a", "b"},
25-
ignoreLabels: []string{"b", "c"},
26-
want: false,
28+
name: "--ignore-labels match (with one out of two)",
29+
prLabels: []string{"a", "b"},
30+
ignoreLabels: []string{"b", "c"},
31+
want: false,
32+
caseSensitive: false,
2733
},
2834

2935
{
30-
prLabels: []string{"a"},
31-
ignoreLabels: []string{"b"},
32-
selectLabels: []string{"c"},
33-
want: false,
36+
name: "no labels match (select or ignore)",
37+
prLabels: []string{"a"},
38+
ignoreLabels: []string{"b"},
39+
selectLabels: []string{"c"},
40+
want: false,
41+
caseSensitive: false,
3442
},
3543
{
36-
prLabels: []string{"a", "c"},
37-
ignoreLabels: []string{"b"},
38-
selectLabels: []string{"c"},
39-
want: true,
44+
name: "--select-labels match",
45+
prLabels: []string{"a", "c"},
46+
ignoreLabels: []string{"b"},
47+
selectLabels: []string{"c"},
48+
want: true,
49+
caseSensitive: false,
4050
},
4151
{
42-
prLabels: []string{"a"},
43-
ignoreLabels: []string{"b"},
44-
selectLabels: []string{"a", "c"},
45-
want: true,
52+
name: "--select-labels match (with one out of two) and ignore labels don't match",
53+
prLabels: []string{"a"},
54+
ignoreLabels: []string{"b"},
55+
selectLabels: []string{"a", "c"},
56+
want: true,
57+
caseSensitive: false,
58+
},
59+
{
60+
name: "the pull request has no labels",
61+
prLabels: []string{},
62+
ignoreLabels: []string{"b"},
63+
selectLabels: []string{"a", "c"},
64+
want: false,
65+
caseSensitive: false,
66+
},
67+
{
68+
name: "the pull request has no labels and ignore labels don't match so it matches - but select labels is empty so it means all labels or even no labels match",
69+
prLabels: []string{},
70+
ignoreLabels: []string{"b"},
71+
selectLabels: []string{},
72+
want: true,
73+
caseSensitive: false,
4674
},
4775
{
48-
prLabels: []string{},
49-
ignoreLabels: []string{"b"},
50-
selectLabels: []string{"a", "c"},
51-
want: false,
76+
name: "the pull request has no labels but we want to match the a label",
77+
prLabels: []string{},
78+
ignoreLabels: []string{},
79+
selectLabels: []string{"a"},
80+
want: false,
81+
caseSensitive: false,
5282
},
5383
{
54-
prLabels: []string{},
55-
ignoreLabels: []string{"b"},
56-
selectLabels: []string{},
57-
want: true,
84+
name: "no label match criteria, so it matches",
85+
prLabels: []string{},
86+
ignoreLabels: []string{},
87+
selectLabels: []string{},
88+
want: true,
89+
caseSensitive: false,
5890
},
5991
{
60-
prLabels: []string{},
61-
ignoreLabels: []string{},
62-
selectLabels: []string{"a"},
63-
want: false,
92+
name: "with one matching label and no matching ignore labels so it matches",
93+
prLabels: []string{"a"},
94+
selectLabels: []string{"a"},
95+
ignoreLabels: []string{"b"},
96+
want: true,
97+
caseSensitive: false,
6498
},
6599
{
66-
prLabels: []string{},
67-
ignoreLabels: []string{},
68-
selectLabels: []string{},
69-
want: true,
100+
name: "the pr labels match the select and ignore labels so it doesn't match",
101+
prLabels: []string{"a"},
102+
selectLabels: []string{"a"},
103+
ignoreLabels: []string{"a"},
104+
want: false,
105+
caseSensitive: false,
70106
},
71107
{
72-
prLabels: []string{"a"},
73-
selectLabels: []string{"a"},
74-
ignoreLabels: []string{"b"},
75-
want: true,
108+
name: "the pr has one label but no defined ignore or select labels so it matches",
109+
prLabels: []string{"a"},
110+
selectLabels: []string{},
111+
ignoreLabels: []string{},
112+
want: true,
113+
caseSensitive: false,
76114
},
77115
{
78-
prLabels: []string{"a"},
79-
selectLabels: []string{"a"},
80-
ignoreLabels: []string{"a"},
81-
want: false,
116+
name: "the pr has one label and it is the select label so it matches",
117+
prLabels: []string{"a"},
118+
selectLabels: []string{"a"},
119+
ignoreLabels: []string{},
120+
want: true,
121+
caseSensitive: false,
82122
},
83123
{
84-
prLabels: []string{"a"},
85-
selectLabels: []string{},
86-
ignoreLabels: []string{},
87-
want: true,
124+
name: "the pr has labels and matching select labels but it matches an ignore label so it doesn't match",
125+
prLabels: []string{"a", "b", "c"},
126+
selectLabels: []string{"a", "b"},
127+
ignoreLabels: []string{"c"},
128+
want: false,
129+
caseSensitive: false,
88130
},
89131
{
90-
prLabels: []string{"a"},
91-
selectLabels: []string{"a"},
92-
ignoreLabels: []string{},
93-
want: true,
132+
name: "the pr has uppercase labels and we are using case insensitive labels so it matches",
133+
prLabels: []string{"Dependencies", "rUby", "ready-for-Review"},
134+
selectLabels: []string{"dependencies", "ready-for-review"},
135+
ignoreLabels: []string{"blocked"},
136+
want: true,
137+
caseSensitive: false,
94138
},
95139
{
96-
prLabels: []string{"a", "b", "c"},
97-
selectLabels: []string{"a", "b"},
98-
ignoreLabels: []string{"c"},
99-
want: false,
140+
name: "the pr has uppercase labels and we are using case sensitive labels so it doesn't match",
141+
prLabels: []string{"Dependencies", "rUby", "ready-for-Review"},
142+
selectLabels: []string{"dependencies", "ready-for-review"},
143+
ignoreLabels: []string{"blocked"},
144+
want: false,
145+
caseSensitive: true,
100146
},
101147
}
102148

103149
for _, test := range tests {
104-
t.Run("", func(t *testing.T) {
150+
t.Run(test.name, func(t *testing.T) {
105151
t.Parallel()
106152

107-
got := labelsMatch(test.prLabels, test.ignoreLabels, test.selectLabels)
153+
// Save the original value of caseSensitiveLabels
154+
originalCaseSensitive := caseSensitiveLabels
155+
defer func() { caseSensitiveLabels = originalCaseSensitive }() // Restore after test
156+
157+
// Set caseSensitiveLabels for this test
158+
caseSensitiveLabels = test.caseSensitive
159+
160+
// Run the function
161+
got := labelsMatch(test.prLabels, test.ignoreLabels, test.selectLabels, test.caseSensitive)
108162
if got != test.want {
109-
t.Errorf("want %v, got %v", test.want, got)
163+
t.Errorf("Test %q failed: want %v, got %v", test.name, test.want, got)
110164
}
111165
})
112166
}

‎internal/cmd/root.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ var (
3434
combineBranchName string
3535
workingBranchSuffix string
3636
dependabot bool
37+
caseSensitiveLabels bool
3738
)
3839

3940
// NewRootCmd creates the root command for the gh-combine CLI
@@ -77,6 +78,7 @@ func NewRootCmd() *cobra.Command {
7778
# Filter PRs by labels
7879
gh combine owner/repo --labels dependencies # PRs must have this single label
7980
gh combine owner/repo --labels security,dependencies # PRs must have ALL these labels
81+
gh combine owner/repo --labels Dependencies --case-sensitive-labels # PRs must have this label, case-sensitive
8082
8183
# Exclude PRs by labels
8284
gh combine owner/repo --ignore-labels wip # Ignore PRs with this label
@@ -124,6 +126,7 @@ func NewRootCmd() *cobra.Command {
124126
rootCmd.Flags().StringVar(&reposFile, "file", "", "File containing repository names, one per line")
125127
rootCmd.Flags().IntVar(&minimum, "minimum", 2, "Minimum number of PRs to combine")
126128
rootCmd.Flags().StringVar(&defaultOwner, "owner", "", "Default owner for repositories (if not specified in repo name or missing from file inputs)")
129+
rootCmd.Flags().BoolVar(&caseSensitiveLabels, "case-sensitive-labels", false, "Use case-sensitive label matching")
127130

128131
// Add deprecated flags for backward compatibility
129132
// rootCmd.Flags().IntVar(&minimum, "min-combine", 2, "Minimum number of PRs to combine (deprecated, use --minimum)")

‎internal/common/common.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
package common
2+
3+
import "strings"
4+
5+
// Normalize an array of strings to lowercase
6+
func NormalizeArray(array []string) []string {
7+
normalized := make([]string, len(array))
8+
for i, label := range array {
9+
normalized[i] = strings.ToLower(label)
10+
}
11+
return normalized
12+
}

0 commit comments

Comments
 (0)