Support for display name in Select prompts

[ghost]

No description provided.

[AlecAivazis]

Hey @frodgesh! Thanks for opening this issue. Do you mind adding some more information as to why you think the API should be changed this way? since it's a breaking change, id like to think through this throughly before changing anything.

Currently, select and multi select return the value the user chose so that options can themselves be ints. Also, if the selects return ints, we'll have to add of logic to prevent the casting of the return value since it doesn't make a lot of sense to convert the index into a string.

[ghost]

@AlecAivazis, thanks for the feedback! Of course, user can convert string answer to int, but that's not the point. If we send []string to the quiz, certainly we do have the array, so why do we have to get string as a return? Would be much likely to get an index of this string to find it in the array, I guess.

In example, we got a struct array of food with two fields: name and calories. We'll send to the quiz a string[] copy of this array, but how do we know, how much calories in chosen food if we get a string? Probably, indexing the whole array would not be the safer idea.

[AlecAivazis]

@frodgesh So I thought about this last night, I think the right approach would be something very similar to the settable interface which could allow for you to pass your food structs as questions. Then, when a user selects the option, the struct associated with their selection would be written to the field. What do you think?

[ghost]

@AlecAivazis It is even more acceptable based on the idea that a function should return the answer, not a reference. I hope, it would be possible to choose a field as an option.

[thadeetrompetter]

Hi! In my case, i'm feeding a slice of string 'questions' into the select, which may contain duplicates. In this situation, the returned string does not really help to get a reference to the right question metadata that i'm also storing. A returned int, as well as being able to pass structs as options, would be of great help.

[gregoriokusowski]

Hey, I made a few changes to make it work with the Stringer interface.
It's still WIP and is not working with multiselect yet.

I can open a proper PR if you guys think that this is a good approach.
https://github.com/AlecAivazis/survey/compare/master...gregoriokusowski:wip-select?expand=1

The idea basically is to keep the current API, creating the option Stringer for old string options.
Instead of returning the index, we can return (or copy) your object back.
WDYT?

[AlecAivazis]

Thanks for working on this @gregoriokusowski! I have been meaning to start implementing this but have gotten completely swamped with getting some work out for a client as quickly as possible.

I think this is a great start. I really like the use of the Stringer interface. I think it is a very natural solution instead of defining our own for use internally. The diff seems a bit weird given that you have only actually changed a few things but I can look more closely when you have something that works with MultiSelect. Also, please make sure that the type-casting logic also handles writing a Stringer to other types. That logic lives here.

Please feel free to reach out to me here or on the gophers slack channel if you need any guidance.

[gregoriokusowski]

I updated that branch, so you can check it.
It worked with examples/longlist.go, but the code still needs a huge cleanup.

I didn't have to touch the writer (yet?!), but I'll play more with other types.

Since it's doing some meta-programming, I'm not sure if it worth to have a single implementation that works with both, or have 2 implementations (one for string, another for fmt.Stringer).

[aajtodd]

Hey, first just wanted to say great library!

We also find this to be a bit of a usability issue with select/multi-select. Slice of indices would suffice, getting the actual of struct(s) directly would be fine as well.

Just thinking out loud:

It seems for select/multi-select you want to be able to control the output string for each item.

What about instead of requiring the user pass strings, let them pass the items directly and provide the function for formatting the list of options

survey.Select{
Items []interface{}
FormatFunc func(value interface{}) string
}

[AlecAivazis]

@aajtodd - thanks for the kind words.

It sounds like what you are suggesting is exactly what @gregoriokusowski has been working on. Instead of defining our own interface, he is using fmt.Stringer which requires the String() method.

[aajtodd]

@AlecAivazis I haven't taken the time to review the work he has been doing but yes the end result is close to what I am thinking.

There are some potential downsides with an interface approach though (fmt.Stringer or custom).

1. You may have already defined the String() method for a type and want to keep that behavior for other use cases
2. The types you are displaying in select/multi-select may not belong to the same package thus you can't extend them (not strictly true, see below).
   • This likely won't be uncommon as the CLI interface to for an executable will often be a different package

Both of these result in the same solution, the user would be forced to alias the type and override the String() method

package "two";

type MyAlias one.SomeType
func(a *MyAlias) String() {...}

Thoughts?

[dominicbarnes]

I'd really like to see a solution to this as well. It seems to me a very common practice to have a human-readable label that is distinct and different from the underlying value. (think ID vs Name)

In my case, the labels I want to present to my users are actually a composite possibly several bits of metadata that add more context for a human. The actual value I want to get at is the unique ID that corresponds to that value.

Choose a value:
[ ] Production Database (redshift)
[ ] Staging Database (postgres)

What I ultimately want to return here is the string ID. (same would apply to MultiSelect, obviously with []string instead of string)

[AlecAivazis]

I looked at this recently and tried to find the right API for the user to specify the result. I wanted to keep only one field for setting either a list of strings, or a list of the interfaces, but it would cause us to lose a lot of type safety, since i would have to change select.Options to be any interface{} and check at runtime that the user has passed in a value of the right types. Is compile-time safety worth the complication in the API? I also have a branch where I changed the top-level API to support the interface instead of strings, and then provided a struct that the user can use to wrap a string in something that supports this interface, here's a link for anyone interested.

I will take another stab at this this weekend and just add a field that takes in a list of the specific interface for structs to act as options. So now the question is - does anyone have an opinion on using fmt.Stringer vs an interface defined inside of survey (say survey.SelectOption)? If I went with the latter, i would probably rely on a method like FormatOption() which would return a string but I wanted to check with people in case they were fine with using fmt.Stringer which seems appropriate for this usecase.

tl;dr1 - I hear you. I hope to have a solution for doing this very soon.
tl;dr2 - is a loss of compile time safety for passing options in (ie supporting raw strings, and a special interface), worth keeping the top level API the same?

cc @coryb

[coryb]

Thanks for the ping @AlecAivazis. I spent some time looking into this and testing out various options and I don't see any feasible way that will not break the API.

The StringerOptions will at least not break the API, but it does feel "dirty" to me having two fields in the struct for the same purpose.

We could change Options to interface{}, but as @AlecAivazis mentions, this will be a lot of runtime checking and I think be a fairly non-intuitive API. In general I hate giving up the type safety, and I am not yet convinced it is absolutely necessary for survey.

Maybe it is time to think about adding a new prompt for this purpose? We could deprecate Select (remove it in v3) and replace it with something like SelectOne? This new prompt would use some SelectOptions interface that users can implement. Then we could special case it to have SelectOneString for the common case that allows []string to be passed in. I am thinking something like how the sort package works.

Anyway, I am wondering if we should just take a step back and play around with a few different abstractions that might more coherently address these issues.

[gaptoothclan]

Have you considered a function that wraps the existing select and select multi. Submit a slice of interface{} to the function, use a template to format the fields you require to the existing select and multi select, on completion return the current selected option id so you can then select your item out of your own slice.

[gregoriokusowski]

I like your approach, @AlecAivazis. The spike I implemented is very similar to the idea of using the interface, but dirtier because of the backward compatibility.

Quick review, IMHO the SelectOption could be removed in favor of Stringer.

I would say we have enough available solutions to make a choice. The two dilemmas are Stringer vs Renderer and backward compatible vs new version.

[AlecAivazis]

I just wanted to let this thread know that we're about to merge a PR as part of the v2 release that changes the way Select and MultiSelect work to interact better with indices. @thadeetrompetter, your issue was nagging at me so I tried to fix it the most harmless way I could - by tweaking the runtime to copy the index instead of the value of the target if you are writing to an int.

I think this is as far as I'm going to take this issue/feature in survey. Supporting an index-based API and a value-based on allows for people to create their own utilities that wrap survey to fit their needs. If someone is interested in publishing a public repo with such a utility, i'd be happy to link to it in the readme.

[thadeetrompetter]

@AlecAivazis Thank you for taking the time to address this

[mathieubellon]

Hi,
I have been reading extensively all related discussions and tests but couldn't figure out how to achieve this (passing a list of of custom struct to select options and catch selected struct back)
Can someone point me to an implementation ?