@samcoe
I mainly used --archived & --no-archived flags from gh repo list as a reference, but I don't have any hard opinions here and let me know if we need to refactor it to have different UX.

I like --draft={true,false} and would consider updating that similar archive flag to match (in another future PR)

--draft={true,false}

@samcoe @vilmibm
Folks, do we have examples of such boolean-like flag somewhere? Or it should be usual string argument, but I should parse it as boolean(taking into account 0/1/yes/no/true/false options) afterwards on my own?

@SiarheiFedartsou Our commander library, Cobra, offers a boolean flag option. An example in our code base can be found here

@samcoe
well, IIUC BoolP always stores either true or false in argument. In our case we actually have 3 states: we either want to show drafts only, or non-drafts only or all PRs. So, maybe proposal with --draft-status={unpublished|published|all} would work better here. WDYT?

@SiarheiFedartsou I agree, which is why I put forth that alternative suggestion, but I do think the UX is better with --draft=true|false just because remembering the options of published, unpublished, and all adds cognitive overhead for the use of the flag when boolean is simpler. We can accomplish the same things though by doing something like this

type ListOptions struct {
  Draft bool
  DraftChanged bool
}

func NewCmdList ... {
  opts := &ListOptions{}
  cmd := &cobra.Command{
    RunE: func(cmd *cobra.Command, args []string) error {
      if cmd.Flags().Changed("draft") {
         opts.DraftChanged = true
      }
   }
  cmd.Flags().BoolP(&opts.Draft, "draft", ...)
}

Now the ListOptions know if the flag was explicitly set and what value it was set to using two fields. Does that make sense?

@samcoe yeah, I was thinking about something like this, but since I am not very familiar with cobra I didn't know how to accomplish it. Thanks, will give it a try!

@samcoe I applied changes in UX proposed by you, please take a look one more time.

@SiarheiFedartsou Thanks for the changes! I made one change which was to convert the draft boolean flag into to a string early on so that it is easier to propagate through to the search code.

@SiarheiFedartsou Thanks for your work! Any chance you could edit the PR body and title to document the final expected behavior just so when people look back on the PR, they know where we landed with the flag structure and what is intended to happen in each case?

I'm late to this party, but I just wanted to state for the record that I would have actually preferred separate --draft and --non-draft flags in this case, and that using a single boolean flag for this filter can be confusing both in the code (as evident by the implementation that saves a boolean flag to a string variable) and to the user.

For example, gh pr create --draft is a boolean flag. Every boolean flag has only 2 states by default, with false being the default state. That means that passing gh pr create --draft=false explicitly is identical to not passing the flag at all.

However, this new pr list --draft flag doesn't work like that. It has three states:

1. gh pr list - draft filter is inactive
2. gh pr list --draft or gh pr list --draft=true - list only draft PRs
3. gh pr list --draft=false - list non-draft PRs

Furthermore, it might not be clear to the user how to use this new flag. Its current documentation is:

  -d, --draft             Filter by draft state

The --draft={true|false} syntax might not be obvious to the user because it's not mentioned in the documentation for this flag.

To summarize, I see two potential problems here: that this boolean flag has 3 states (whereas other boolean flags in gh have only 2 states) and that using this filter to list non-draft PRs requires knowledge of non-obvious flag syntax.

If the goal was to avoid introducing two new flags (--draft & --non-draft) to support a single filter, which I understand, then maybe we could consider other approaches? For example, we can piggyback on the --state filter:

1. gh pr list --state=draft (implies --state=open)
2. gh pr list --state=ready (implies --state=open)