refactor(label-filters): improve the label filtering

This removes --label and --ignore-label in favor of only using --labels
and --ignore-labels. Having a singular and plural version caused
confusion and created additional code checks that weren't needed.

It also create some new tests to check for validity of the new logic.

Note that it changes the prLabels from undefined []struct{} to []string.
Eventually it might go back to []PR once it's properly defined. But
until then we can use strings only.

Author: nobe4

internal/cmd/inputs.go

@@ -3,42 +3,14 @@ package cmd
 import (
 	"errors"
 	"fmt"
+	"slices"
 )
 
+var errIgnoreLabelsConflict = errors.New("--ignore-labels contains a value which conflicts with --labels")
+
 // validateInputs checks if the provided inputs are valid
 func ValidateInputs(args []string) error {
-	// Check that ignore-label and select-label are not the same
-	if ignoreLabel != "" && selectLabel != "" && ignoreLabel == selectLabel {
-		return errors.New("--ignore-label(s) and --label(s) cannot have the same value")
-	}
-
-	// Check for conflicts between ignoreLabels and selectLabel
-	if selectLabel != "" && len(ignoreLabels) > 0 {
-		for _, ignoreL := range ignoreLabels {
-			if ignoreL == selectLabel {
-				return fmt.Errorf("--ignore-labels contains %q which conflicts with --label %q", ignoreL, selectLabel)
-			}
-		}
-	}
-
-	// Check for conflicts between ignoreLabel and selectLabels
-	if ignoreLabel != "" && len(selectLabels) > 0 {
-		for _, selectL := range selectLabels {
-			if selectL == ignoreLabel {
-				return fmt.Errorf("--label(s) contains %q which conflicts with --ignore-label %q", selectL, ignoreLabel)
-			}
-		}
-	}
-
-	// Check for conflicts between ignoreLabels and selectLabels
-	if len(ignoreLabels) > 0 && len(selectLabels) > 0 {
-		for _, ignoreL := range ignoreLabels {
-			for _, selectL := range selectLabels {
-				if ignoreL == selectL {
-					return fmt.Errorf("--ignore-labels contains %q which conflicts with --labels containing the same value", ignoreL)
-				}
-			}
-		}
+	if err := ValidateLabels(selectLabels, ignoreLabels); err != nil {
 	}
 
 	// If no args and no file, we can't proceed
@@ -48,9 +20,20 @@ func ValidateInputs(args []string) error {
 
 	// Warn if no filtering options are provided at all
 	if branchPrefix == "" && branchSuffix == "" && branchRegex == "" &&
-		ignoreLabel == "" && len(ignoreLabels) == 0 && selectLabel == "" && len(selectLabels) == 0 &&
+		len(ignoreLabels) == 0 && len(selectLabels) == 0 &&
 		!requireCI && !mustBeApproved {
-		Logger.Warn("No filtering options specified. This will attempt to combine ALL open pull requests. Use --label, --labels, --ignore-label, --ignore-labels, --branch-prefix, --branch-suffix, --branch-regex, --dependabot, etc to filter.")
+		Logger.Warn("No filtering options specified. This will attempt to combine ALL open pull requests. Use  --labels, --ignore-labels, --branch-prefix, --branch-suffix, --branch-regex, --dependabot, etc to filter.")
+	}
+
+	return nil
+}
+
+func ValidateLabels(selectLabels []string, ignoreLabels []string) error {
+	// Check for conflicts between ignoreLabels and selectLabels
+	for _, ignoreL := range ignoreLabels {
+		if i := slices.Index(selectLabels, ignoreL); i != -1 {
+			return fmt.Errorf("%w: %q %q", errIgnoreLabelsConflict, ignoreLabels[i], selectLabels)
+		}
 	}
 
 	return nil

internal/cmd/match_criteria.go

@@ -4,21 +4,22 @@ import (
 	"context"
 	"fmt"
 	"regexp"
+	"slices"
 	"strings"
 
 	"github.com/cli/go-gh/v2/pkg/api"
 	graphql "github.com/cli/shurcooL-graphql"
 )
 
 // checks if a PR matches all filtering criteria
-func PrMatchesCriteria(branch string, prLabels []struct{ Name string }) bool {
+func PrMatchesCriteria(branch string, prLabels []string) bool {
 	// Check branch criteria if any are specified
 	if !branchMatchesCriteria(branch) {
 		return false
 	}
 
 	// Check label criteria if any are specified
-	if !labelsMatchCriteria(prLabels) {
+	if !labelsMatch(prLabels, ignoreLabels, selectLabels) {
 		return false
 	}
 
@@ -58,64 +59,25 @@ func branchMatchesCriteria(branch string) bool {
 	return true
 }
 
-// labelsMatchCriteria checks if PR labels match the label filtering criteria
-func labelsMatchCriteria(prLabels []struct{ Name string }) bool {
-	// If no label filters are specified, all PRs pass this check
-	if ignoreLabel == "" && len(ignoreLabels) == 0 &&
-		selectLabel == "" && len(selectLabels) == 0 {
-		return true
-	}
-
-	// Check for ignore label (singular)
-	if ignoreLabel != "" {
-		for _, label := range prLabels {
-			if label.Name == ignoreLabel {
-				return false
-			}
-		}
-	}
-
-	// Check for ignore labels (plural)
-	if len(ignoreLabels) > 0 {
-		for _, ignoreL := range ignoreLabels {
-			for _, prLabel := range prLabels {
-				if prLabel.Name == ignoreL {
-					return false
-				}
-			}
+func labelsMatch(prLabels, ignoreLabels, selectLabels []string) bool {
+	for _, l := range ignoreLabels {
+		if i := slices.Index(prLabels, l); i != -1 {
+			return false
 		}
 	}
 
-	// Check for select label (singular)
-	if selectLabel != "" {
+	for _, l := range selectLabels {
 		found := false
-		for _, label := range prLabels {
-			if label.Name == selectLabel {
-				found = true
-				break
-			}
+		if i := slices.Index(prLabels, l); i != -1 {
+			found = true
+			break
 		}
+
 		if !found {
 			return false
 		}
 	}
 
-	// Check for select labels (plural)
-	if len(selectLabels) > 0 {
-		for _, requiredLabel := range selectLabels {
-			found := false
-			for _, prLabel := range prLabels {
-				if prLabel.Name == requiredLabel {
-					found = true
-					break
-				}
-			}
-			if !found {
-				return false
-			}
-		}
-	}
-
 	return true
 }
 

internal/cmd/match_criteria_test.go

@@ -0,0 +1,59 @@
+package cmd
+
+import "testing"
+
+func TestLabelsMatch(t *testing.T) {
+	t.Parallel()
+
+	tests := []struct {
+		prLabels     []string
+		ignoreLabels []string
+		selectLabels []string
+		want         bool
+	}{
+		{
+			want: true,
+		},
+
+		{
+			prLabels:     []string{"a", "b"},
+			ignoreLabels: []string{"b"},
+			want:         false,
+		},
+		{
+			prLabels:     []string{"a", "b"},
+			ignoreLabels: []string{"b", "c"},
+			want:         false,
+		},
+
+		{
+			prLabels:     []string{"a"},
+			ignoreLabels: []string{"b"},
+			selectLabels: []string{"c"},
+			want:         false,
+		},
+		{
+			prLabels:     []string{"a", "c"},
+			ignoreLabels: []string{"b"},
+			selectLabels: []string{"c"},
+			want:         true,
+		},
+		{
+			prLabels:     []string{"a"},
+			ignoreLabels: []string{"b"},
+			selectLabels: []string{"a", "c"},
+			want:         true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run("", func(t *testing.T) {
+			t.Parallel()
+
+			got := labelsMatch(test.prLabels, test.ignoreLabels, test.selectLabels)
+			if got != test.want {
+				t.Errorf("want %v, got %v", test.want, got)
+			}
+		})
+	}
+}

internal/cmd/root.go

@@ -14,19 +14,20 @@ import (
 )
 
 var (
-	branchPrefix        string
-	branchSuffix        string
-	branchRegex         string
-	selectLabel         string
-	selectLabels        []string
-	addLabels           []string
-	addAssignees        []string
+	branchPrefix string
+	branchSuffix string
+	branchRegex  string
+
+	selectLabels []string
+	ignoreLabels []string
+
+	addLabels    []string
+	addAssignees []string
+
 	requireCI           bool
 	mustBeApproved      bool
 	autoclose           bool
 	updateBranch        bool
-	ignoreLabel         string
-	ignoreLabels        []string
 	reposFile           string
 	minimum             int
 	defaultOwner        string
@@ -46,7 +47,7 @@ func NewRootCmd() *cobra.Command {
 	  # Note: You should use some form of filtering to avoid combining all open PRs in a repository.
 	  # For example, you can filter by branch name, labels, or other criteria.
 	  # Forms of filtering include:
-	  # --label, --labels, --ignore-label, --ignore-labels, --branch-prefix, --branch-suffix, --branch-regex, --dependabot, etc.
+	  # --labels, --ignore-labels, --branch-prefix, --branch-suffix, --branch-regex, --dependabot, etc.
 
       # Basic usage with a single repository to combine all pull requests into one
       gh combine owner/repo
@@ -75,11 +76,11 @@ func NewRootCmd() *cobra.Command {
       gh combine owner/repo --branch-regex "dependabot/.*"
     
       # Filter PRs by labels
-      gh combine owner/repo --label dependencies        # PRs must have this single label
+      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
       
       # Exclude PRs by labels
-      gh combine owner/repo --ignore-label wip          # Ignore PRs with this label
+      gh combine owner/repo --ignore-labels wip         # Ignore PRs with this label
       gh combine owner/repo --ignore-labels wip,draft   # Ignore PRs with ANY of these labels
     
       # Set requirements for PRs to be combined
@@ -105,12 +106,7 @@ func NewRootCmd() *cobra.Command {
 	rootCmd.Flags().StringVar(&branchSuffix, "branch-suffix", "", "Branch suffix to filter PRs")
 	rootCmd.Flags().StringVar(&branchRegex, "branch-regex", "", "Regex pattern to filter PRs by branch name")
 
-	// Label selection flags - singular and plural forms
-	rootCmd.Flags().StringVar(&selectLabel, "label", "", "Only include PRs with this specific label")
 	rootCmd.Flags().StringSliceVar(&selectLabels, "labels", nil, "Only include PRs with ALL these labels (comma-separated)")
-
-	// Label ignoring flags - singular and plural forms
-	rootCmd.Flags().StringVar(&ignoreLabel, "ignore-label", "", "Ignore PRs with this specific label")
 	rootCmd.Flags().StringSliceVar(&ignoreLabels, "ignore-labels", nil, "Ignore PRs with ANY of these labels (comma-separated)")
 
 	// Labels to add to the combined PR
@@ -278,8 +274,16 @@ func processRepository(ctx context.Context, client *api.RESTClient, graphQlClien
 	for _, pull := range pulls {
 		branch := pull.Head.Ref
 
+		// Temporary workaround because passing structures is useless in this
+		// context.
+		// Eventually the []Labels should have better support.
+		labels := []string{}
+		for _, label := range pull.Labels {
+			labels = append(labels, label.Name)
+		}
+
 		// Check if PR matches all filtering criteria
-		if !PrMatchesCriteria(branch, pull.Labels) {
+		if !PrMatchesCriteria(branch, labels) {
 			continue
 		}