Skip to content

feat: Label Filter Case-Sensitivity Improvements #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 14, 2025
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
12 changes: 10 additions & 2 deletions internal/cmd/match_criteria.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (

"github.com/cli/go-gh/v2/pkg/api"
graphql "github.com/cli/shurcooL-graphql"
"github.com/github/gh-combine/internal/common"
)

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

// Check label criteria if any are specified
if !labelsMatch(prLabels, ignoreLabels, selectLabels) {
if !labelsMatch(prLabels, ignoreLabels, selectLabels, caseSensitiveLabels) {
return false
}

Expand Down Expand Up @@ -71,12 +72,19 @@ func branchMatchesCriteria(branch string) bool {
return true
}

func labelsMatch(prLabels, ignoreLabels, selectLabels []string) bool {
func labelsMatch(prLabels, ignoreLabels, selectLabels []string, caseSensitive bool) bool {
// If no ignoreLabels or selectLabels are specified, all labels pass this check
if len(ignoreLabels) == 0 && len(selectLabels) == 0 {
return true
}

// Normalize labels for case-insensitive matching if caseSensitive is false
if !caseSensitive {
prLabels = common.NormalizeArray(prLabels)
ignoreLabels = common.NormalizeArray(ignoreLabels)
selectLabels = common.NormalizeArray(selectLabels)
}

// If the pull request contains any of the ignore labels, it doesn't match
for _, l := range ignoreLabels {
if slices.Contains(prLabels, l) {
Expand Down
176 changes: 115 additions & 61 deletions internal/cmd/match_criteria_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,107 +6,161 @@ func TestLabelsMatch(t *testing.T) {
t.Parallel()

tests := []struct {
prLabels []string
ignoreLabels []string
selectLabels []string
want bool
name string
prLabels []string
ignoreLabels []string
selectLabels []string
caseSensitive bool
want bool
}{
{
want: true,
},

{
prLabels: []string{"a", "b"},
ignoreLabels: []string{"b"},
want: false,
name: "--ignore-labels match",
prLabels: []string{"a", "b"},
ignoreLabels: []string{"b"},
want: false,
caseSensitive: false,
},
{
prLabels: []string{"a", "b"},
ignoreLabels: []string{"b", "c"},
want: false,
name: "--ignore-labels match (with one out of two)",
prLabels: []string{"a", "b"},
ignoreLabels: []string{"b", "c"},
want: false,
caseSensitive: false,
},

{
prLabels: []string{"a"},
ignoreLabels: []string{"b"},
selectLabels: []string{"c"},
want: false,
name: "no labels match (select or ignore)",
prLabels: []string{"a"},
ignoreLabels: []string{"b"},
selectLabels: []string{"c"},
want: false,
caseSensitive: false,
},
{
prLabels: []string{"a", "c"},
ignoreLabels: []string{"b"},
selectLabels: []string{"c"},
want: true,
name: "--select-labels match",
prLabels: []string{"a", "c"},
ignoreLabels: []string{"b"},
selectLabels: []string{"c"},
want: true,
caseSensitive: false,
},
{
prLabels: []string{"a"},
ignoreLabels: []string{"b"},
selectLabels: []string{"a", "c"},
want: true,
name: "--select-labels match (with one out of two) and ignore labels don't match",
prLabels: []string{"a"},
ignoreLabels: []string{"b"},
selectLabels: []string{"a", "c"},
want: true,
caseSensitive: false,
},
{
name: "the pull request has no labels",
prLabels: []string{},
ignoreLabels: []string{"b"},
selectLabels: []string{"a", "c"},
want: false,
caseSensitive: false,
},
{
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",
prLabels: []string{},
ignoreLabels: []string{"b"},
selectLabels: []string{},
want: true,
caseSensitive: false,
},
{
prLabels: []string{},
ignoreLabels: []string{"b"},
selectLabels: []string{"a", "c"},
want: false,
name: "the pull request has no labels but we want to match the a label",
prLabels: []string{},
ignoreLabels: []string{},
selectLabels: []string{"a"},
want: false,
caseSensitive: false,
},
{
prLabels: []string{},
ignoreLabels: []string{"b"},
selectLabels: []string{},
want: true,
name: "no label match criteria, so it matches",
prLabels: []string{},
ignoreLabels: []string{},
selectLabels: []string{},
want: true,
caseSensitive: false,
},
{
prLabels: []string{},
ignoreLabels: []string{},
selectLabels: []string{"a"},
want: false,
name: "with one matching label and no matching ignore labels so it matches",
prLabels: []string{"a"},
selectLabels: []string{"a"},
ignoreLabels: []string{"b"},
want: true,
caseSensitive: false,
},
{
prLabels: []string{},
ignoreLabels: []string{},
selectLabels: []string{},
want: true,
name: "the pr labels match the select and ignore labels so it doesn't match",
prLabels: []string{"a"},
selectLabels: []string{"a"},
ignoreLabels: []string{"a"},
want: false,
caseSensitive: false,
},
{
prLabels: []string{"a"},
selectLabels: []string{"a"},
ignoreLabels: []string{"b"},
want: true,
name: "the pr has one label but no defined ignore or select labels so it matches",
prLabels: []string{"a"},
selectLabels: []string{},
ignoreLabels: []string{},
want: true,
caseSensitive: false,
},
{
prLabels: []string{"a"},
selectLabels: []string{"a"},
ignoreLabels: []string{"a"},
want: false,
name: "the pr has one label and it is the select label so it matches",
prLabels: []string{"a"},
selectLabels: []string{"a"},
ignoreLabels: []string{},
want: true,
caseSensitive: false,
},
{
prLabels: []string{"a"},
selectLabels: []string{},
ignoreLabels: []string{},
want: true,
name: "the pr has labels and matching select labels but it matches an ignore label so it doesn't match",
prLabels: []string{"a", "b", "c"},
selectLabels: []string{"a", "b"},
ignoreLabels: []string{"c"},
want: false,
caseSensitive: false,
},
{
prLabels: []string{"a"},
selectLabels: []string{"a"},
ignoreLabels: []string{},
want: true,
name: "the pr has uppercase labels and we are using case insensitive labels so it matches",
prLabels: []string{"Dependencies", "rUby", "ready-for-Review"},
selectLabels: []string{"dependencies", "ready-for-review"},
ignoreLabels: []string{"blocked"},
want: true,
caseSensitive: false,
},
{
prLabels: []string{"a", "b", "c"},
selectLabels: []string{"a", "b"},
ignoreLabels: []string{"c"},
want: false,
name: "the pr has uppercase labels and we are using case sensitive labels so it doesn't match",
prLabels: []string{"Dependencies", "rUby", "ready-for-Review"},
selectLabels: []string{"dependencies", "ready-for-review"},
ignoreLabels: []string{"blocked"},
want: false,
caseSensitive: true,
},
}

for _, test := range tests {
t.Run("", func(t *testing.T) {
t.Run(test.name, func(t *testing.T) {
t.Parallel()

got := labelsMatch(test.prLabels, test.ignoreLabels, test.selectLabels)
// Save the original value of caseSensitiveLabels
originalCaseSensitive := caseSensitiveLabels
defer func() { caseSensitiveLabels = originalCaseSensitive }() // Restore after test

// Set caseSensitiveLabels for this test
caseSensitiveLabels = test.caseSensitive

// Run the function
got := labelsMatch(test.prLabels, test.ignoreLabels, test.selectLabels, test.caseSensitive)
if got != test.want {
t.Errorf("want %v, got %v", test.want, got)
t.Errorf("Test %q failed: want %v, got %v", test.name, test.want, got)
}
})
}
Expand Down
3 changes: 3 additions & 0 deletions internal/cmd/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ var (
combineBranchName string
workingBranchSuffix string
dependabot bool
caseSensitiveLabels bool
)

// NewRootCmd creates the root command for the gh-combine CLI
Expand Down Expand Up @@ -77,6 +78,7 @@ func NewRootCmd() *cobra.Command {
# Filter PRs by labels
gh combine owner/repo --labels dependencies # PRs must have this single label
gh combine owner/repo --labels security,dependencies # PRs must have ALL these labels
gh combine owner/repo --labels Dependencies --case-sensitive-labels # PRs must have this label, case-sensitive

# Exclude PRs by labels
gh combine owner/repo --ignore-labels wip # Ignore PRs with this label
Expand Down Expand Up @@ -124,6 +126,7 @@ func NewRootCmd() *cobra.Command {
rootCmd.Flags().StringVar(&reposFile, "file", "", "File containing repository names, one per line")
rootCmd.Flags().IntVar(&minimum, "minimum", 2, "Minimum number of PRs to combine")
rootCmd.Flags().StringVar(&defaultOwner, "owner", "", "Default owner for repositories (if not specified in repo name or missing from file inputs)")
rootCmd.Flags().BoolVar(&caseSensitiveLabels, "case-sensitive-labels", false, "Use case-sensitive label matching")

// Add deprecated flags for backward compatibility
// rootCmd.Flags().IntVar(&minimum, "min-combine", 2, "Minimum number of PRs to combine (deprecated, use --minimum)")
Expand Down
12 changes: 12 additions & 0 deletions internal/common/common.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
package common

import "strings"

// Normalize an array of strings to lowercase
func NormalizeArray(array []string) []string {
normalized := make([]string, len(array))
for i, label := range array {
normalized[i] = strings.ToLower(label)
}
return normalized
}
Loading