From 7bb49acbc4db606b2ac2bd403b9c061bc5ef18f0 Mon Sep 17 00:00:00 2001 From: Prarit Bhargava Date: Wed, 3 Mar 2021 20:41:13 -0500 Subject: [PATCH] lab: Clean up IntVarP usage 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 --- cmd/issue_list.go | 18 ++++++++++-------- cmd/mr_list.go | 13 +++++++------ cmd/project_list.go | 18 ++++++++++-------- cmd/snippet_list.go | 17 ++++++++++------- cmd/util.go | 4 +--- 5 files changed, 38 insertions(+), 32 deletions(-) diff --git a/cmd/issue_list.go b/cmd/issue_list.go index 38495858..e61c190c 100644 --- a/cmd/issue_list.go +++ b/cmd/issue_list.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" "log" + "strconv" "github.com/pkg/errors" "github.com/rsteube/carapace" @@ -17,7 +18,7 @@ var ( issueMilestone string issueState string issueSearch string - issueNumRet int + issueNumRet string issueAll bool issueExactMatch bool ) @@ -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, @@ -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) } @@ -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, diff --git a/cmd/mr_list.go b/cmd/mr_list.go index 77bffd53..5a242598 100644 --- a/cmd/mr_list.go +++ b/cmd/mr_list.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" "log" + "strconv" "github.com/pkg/errors" "github.com/rsteube/carapace" @@ -17,7 +18,7 @@ var ( mrState string mrTargetBranch string mrMilestone string - mrNumRet int + mrNumRet string mrAll bool mrMine bool mrDraft bool @@ -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 } @@ -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, @@ -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", "", diff --git a/cmd/project_list.go b/cmd/project_list.go index aaf14011..7adc6082 100644 --- a/cmd/project_list.go +++ b/cmd/project_list.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" "log" + "strconv" "github.com/spf13/cobra" "github.com/xanzy/go-gitlab" @@ -14,8 +15,7 @@ var projectListConfig struct { Owned bool Membership bool Starred bool - - Number int + Number string } var projectListCmd = &cobra.Command{ @@ -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"), @@ -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) @@ -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 } diff --git a/cmd/snippet_list.go b/cmd/snippet_list.go index 2e52e427..5356bf8e 100644 --- a/cmd/snippet_list.go +++ b/cmd/snippet_list.go @@ -3,6 +3,7 @@ package cmd import ( "fmt" "log" + "strconv" "github.com/rsteube/carapace" "github.com/spf13/cobra" @@ -12,7 +13,7 @@ import ( ) var snippetListConfig struct { - Number int + Number string All bool } @@ -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 == "" { @@ -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) diff --git a/cmd/util.go b/cmd/util.go index b7f9518c..e2ae91df 100644 --- a/cmd/util.go +++ b/cmd/util.go @@ -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 = ""