Skip to content

Commit

Permalink
lab: Clean up IntVarP usage
Browse files Browse the repository at this point in the history
A user noticed an issue in which 'lab mr --list' and 'lab mr list'
returned lists of 10 and 20 entries respectively.  Both effectively call
the same code in the mr_list file and the only difference is that
LabPersistentPreRun() is run on 'lab mr' and 'lab mr list' respectively.

It appears that IntVarP is not handled properly by viper (there are a few
reports of other issues with IntVarP online) and the suggestion is to
use StringVarP in place of IntVarP.

The IntVarP value is used as the number of entries returned per page
for the list commands of MRs, Issues, Projects, and Snippets.  In all of
these cases the user modified value was not being honoured in the
code so fixes for these are also included.

Signed-off-by: Prarit Bhargava <prarit@redhat.com>
  • Loading branch information
prarit committed Mar 4, 2021
1 parent 2b21ae6 commit 7bb49ac
Show file tree
Hide file tree
Showing 5 changed files with 38 additions and 32 deletions.
18 changes: 10 additions & 8 deletions cmd/issue_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cmd
import (
"fmt"
"log"
"strconv"

"github.com/pkg/errors"
"github.com/rsteube/carapace"
Expand All @@ -17,7 +18,7 @@ var (
issueMilestone string
issueState string
issueSearch string
issueNumRet int
issueNumRet string
issueAll bool
issueExactMatch bool
)
Expand Down Expand Up @@ -66,9 +67,14 @@ func issueList(args []string) ([]*gitlab.Issue, error) {
issueMilestone = milestone.Title
}

num, err := strconv.Atoi(issueNumRet)
if issueAll || (err != nil) {
num = -1
}

opts := gitlab.ListProjectIssuesOptions{
ListOptions: gitlab.ListOptions{
PerPage: issueNumRet,
PerPage: num,
},
Labels: labels,
Milestone: &issueMilestone,
Expand All @@ -87,10 +93,6 @@ func issueList(args []string) ([]*gitlab.Issue, error) {
opts.Search = &issueSearch
}

num := issueNumRet
if issueAll {
num = -1
}
return lab.IssueList(rn, opts, num)
}

Expand All @@ -101,8 +103,8 @@ func init() {
issueListCmd.Flags().StringVarP(
&issueState, "state", "s", "opened",
"filter issues by state (all/opened/closed)")
issueListCmd.Flags().IntVarP(
&issueNumRet, "number", "n", 10,
issueListCmd.Flags().StringVarP(
&issueNumRet, "number", "n", "10",
"number of issues to return")
issueListCmd.Flags().BoolVarP(
&issueAll, "all", "a", false,
Expand Down
13 changes: 7 additions & 6 deletions cmd/mr_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cmd
import (
"fmt"
"log"
"strconv"

"github.com/pkg/errors"
"github.com/rsteube/carapace"
Expand All @@ -17,7 +18,7 @@ var (
mrState string
mrTargetBranch string
mrMilestone string
mrNumRet int
mrNumRet string
mrAll bool
mrMine bool
mrDraft bool
Expand Down Expand Up @@ -69,8 +70,8 @@ func mrList(args []string) ([]*gitlab.MergeRequest, error) {
return nil, err
}

num := mrNumRet
if mrAll {
num, err := strconv.Atoi(mrNumRet)
if mrAll || (err != nil) {
num = -1
}

Expand Down Expand Up @@ -105,7 +106,7 @@ func mrList(args []string) ([]*gitlab.MergeRequest, error) {

opts := gitlab.ListProjectMergeRequestsOptions{
ListOptions: gitlab.ListOptions{
PerPage: mrNumRet,
PerPage: num,
},
Labels: labels,
State: &mrState,
Expand Down Expand Up @@ -161,8 +162,8 @@ func init() {
listCmd.Flags().StringVarP(
&mrState, "state", "s", "opened",
"filter merge requests by state (all/opened/closed/merged)")
listCmd.Flags().IntVarP(
&mrNumRet, "number", "n", 10,
listCmd.Flags().StringVarP(
&mrNumRet, "number", "n", "10",
"number of merge requests to return")
listCmd.Flags().StringVarP(
&mrTargetBranch, "target-branch", "t", "",
Expand Down
18 changes: 10 additions & 8 deletions cmd/project_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cmd
import (
"fmt"
"log"
"strconv"

"github.com/spf13/cobra"
"github.com/xanzy/go-gitlab"
Expand All @@ -14,8 +15,7 @@ var projectListConfig struct {
Owned bool
Membership bool
Starred bool

Number int
Number string
}

var projectListCmd = &cobra.Command{
Expand All @@ -28,9 +28,15 @@ var projectListCmd = &cobra.Command{
if err != nil {
log.Fatal(err)
}

num, err := strconv.Atoi(projectListConfig.Number)
if projectListConfig.All || (err != nil) {
num = -1
}

opt := gitlab.ListProjectsOptions{
ListOptions: gitlab.ListOptions{
PerPage: projectListConfig.Number,
PerPage: num,
},
Simple: gitlab.Bool(true),
OrderBy: gitlab.String("id"),
Expand All @@ -40,10 +46,6 @@ var projectListCmd = &cobra.Command{
Starred: gitlab.Bool(projectListConfig.Starred),
Search: gitlab.String(search),
}
num := projectListConfig.Number
if projectListConfig.All {
num = -1
}
projects, err := lab.ProjectList(opt, num)
if err != nil {
log.Fatal(err)
Expand All @@ -64,6 +66,6 @@ func init() {
projectListCmd.Flags().BoolVarP(&projectListConfig.Owned, "mine", "m", false, "limit by your projects")
projectListCmd.Flags().BoolVar(&projectListConfig.Membership, "member", false, "limit by projects which you are a member")
projectListCmd.Flags().BoolVar(&projectListConfig.Starred, "starred", false, "limit by your starred projects")
projectListCmd.Flags().IntVarP(&projectListConfig.Number, "number", "n", 100, "number of projects to return")
projectListCmd.Flags().StringVarP(&projectListConfig.Number, "number", "n", "100", "Number of projects to return")
projectListCmd.Flags().SortFlags = false
}
17 changes: 10 additions & 7 deletions cmd/snippet_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package cmd
import (
"fmt"
"log"
"strconv"

"github.com/rsteube/carapace"
"github.com/spf13/cobra"
Expand All @@ -12,7 +13,7 @@ import (
)

var snippetListConfig struct {
Number int
Number string
All bool
}

Expand Down Expand Up @@ -41,14 +42,16 @@ func snippetList(args []string) ([]*gitlab.Snippet, error) {
if err != nil {
return nil, err
}
listOpts := gitlab.ListOptions{
PerPage: snippetListConfig.Number,
}

num := snippetListConfig.Number
if snippetListConfig.All {
num, err := strconv.Atoi(snippetListConfig.Number)
if snippetListConfig.All || (err != nil) {
num = -1
}

listOpts := gitlab.ListOptions{
PerPage: num,
}

// See if we're in a git repo or if global is set to determine
// if this should be a personal snippet
if global || rn == "" {
Expand All @@ -65,7 +68,7 @@ func snippetList(args []string) ([]*gitlab.Snippet, error) {
}

func init() {
snippetListCmd.Flags().IntVarP(&snippetListConfig.Number, "number", "n", 10, "number of snippets to return")
snippetListCmd.Flags().StringVarP(&snippetListConfig.Number, "number", "n", "10", "Number of snippets to return")
snippetListCmd.Flags().BoolVarP(&snippetListConfig.All, "all", "a", false, "list all snippets")

snippetCmd.AddCommand(snippetListCmd)
Expand Down
4 changes: 1 addition & 3 deletions cmd/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,8 @@ func flagConfig(fs *flag.FlagSet) {
case "stringSlice":
configValue = getMainConfig().GetStringSlice(CommandPrefix + f.Name)
configString = strings.Join(configValue.([]string), " ")

case "int":
configValue = getMainConfig().GetInt64(CommandPrefix + f.Name)
configString = strconv.FormatInt(configValue.(int64), 10)
log.Fatal("ERROR: found int flag, use string instead: ", f.Value.Type(), f)
case "stringArray":
// viper does not have support for stringArray
configString = ""
Expand Down

0 comments on commit 7bb49ac

Please sign in to comment.