search

AlecAivazis / survey

A golang library for building interactive and accessible prompts with full support for windows and posix terminals.
MIT License
4.08k stars 350 forks source link

Added WithPageSize AskOpt #214

Closed AlecAivazis closed 5 years ago

AlecAivazis commented 5 years ago

This PR is part of the work summarized in #201. It adds the survey.WithPageSize AskOpt that replaces the modification of survey.PageSize as the way to modify the default page size for prompts.

Since we now have to have a place for "global" configuration of prompts, I had to change the Prompt interface to include a reference to the config.

AlecAivazis commented 5 years ago

Yea I thought about that while I was putting this together but it felt like a lot of ceremony for an if statement, especially considering I'd still have to pass the config in. There could be an argument for encapsulating the paginate function into a method. I'll take a look and see if its worth it before merging - thanks for the review!

AlecAivazis commented 5 years ago

So I tried to consolidate the paginate logic into a method on a Pager interface like you suggested. Unfortunately, since you can't set promoted fields, this would force the API to be:

Select{
    Pager{ PageSize: 7 }
}

which is just a little too verbose for me

coryb commented 5 years ago

Okay, yeah, I agree, forgot about that aspect. We could minimize this issue by having a NewSelect etc, with variadic args, ie NewSelect(opts ...func(*Select)) and WithPageSize(s *Select). Not sure if there are other issues that would be resolved or made worse by this direction though.

AlecAivazis commented 5 years ago

Yea that's something to keep in mind. It starts to become very verbose and I think so long as we have sane interpretations of a zero value, we should keep the struct-based API as I think its much simpler for people to grok quickly

AlecAivazis commented 5 years ago

It just occurred to me that it's also very possible that sort of API is what a solution to #105 would look like.