selection.New(selection.Choice[T]) doesn't work about promptkit HOT 7 CLOSED

5. selection.New could also accept a func(T) string (probably as var arg functional option), which would probably alleviate the need for Choice in the first place.

from promptkit.

Hi, thanks for your feedback and sorry for the late response. Could you maybe share an example of how you are using the library?

Maybe I'm understanding you wrong but I think you are maybe passing a slice of Choices to selection.New because that was how it worked prior to v0.8.0. As stated in the README.md, the library's API is not yet stable and I changed the API to incorporate generics as soon as they were available (this was announced a long time ago). The release notes for v0.8.0 contain a migration guide. If you look at the example for the selection prompt, the values are passed as []string instead of []*Choice and as a result, selection.RunPrompt() returns a string. The Choice type is now only used for selection.ValueAsChoice() in order to make it possible to find out what index is currently selected. For the same reason, the NewChoice function was removed in v0.8.0.

from promptkit.

Thanks for responding! https://github.com/avamsi/axl/blob/05c2a49d04477c556386f65010eafc8f0d422cec/main.go#L124-L131 is my use case -- basically I want to display a "beautified" version of the value for the user but want to process the "raw" value of user's selection, so simple strings are out of the question.

Choice used to enable this usecase previously and I understand the API is not stable (and I wasn't expecting to be), but looking at the switch case I linked above (which is trying to extract the String value from Choice), it seemed like there was some intention of trying to make it work still, so I figured I'd report it as a bug.

I ended up defining a choice Stringer myself and using that as you can see in the linked code.

from promptkit.

Yeah this code used to be exposed to users but now it is only used internally and I never removed the "unnecessary" cases. However, your example is exactly how I intended the library to be used.

I honestly never thought that someone would use Choice this way. I only introduced this type as a crutch before generics were available, so I never thought about someone setting Choice.String manually as it is normally populated automatically (e.g. via the value's io.Stringer).

Now I totally see where you are coming from. I still think the way you implemented it now in your example should be the primary and documented way to do it. Im also not against changing the function to support your prior Choice use case. Unfortunately it is stretching the current capabilities of generics in Golang a bit and reflection is quite awkward...

from promptkit.

Makes sense, any thoughts on possibility 5 from my earlier comment? In the example I shared, I already have a beautify function defined and if I could just plumb plain strings to selection.New along with this function, that would simplify the code somewhat.

I now see there's very related couple ChoiceStyle fields defined already, but they take *Choice[T] as an input and not T (and 3 different knobs instead of 1).

from promptkit.

I'd rather not change the API because I want it to be used with []io.Stringer, []string, and so on. I would much prefer to make it silently work for your use case and not advertise it too much.

What is the problem with *Choice[T] in ChoiceStyle? The T is just a .Value away. However, I do understand that this is not the right tool for the job because it is intended to be used to customize how the selected choice is styled compared to the unselected choice (and as you said, 3 knobs instead of 1).

Anyway, I researched it a bit and I'm not even sure if it would work with reflection as the compiler probably wouldn't be able to assign it to selection.choices after I made sure it is the correct type via reflection. Maybe we should leave it as-is and you keep the current implementation in your example.

from promptkit.

Fair enough, closing. Thanks for your time!

from promptkit.