Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.x] Fix prompt fallback return value when using numeric keys #50995

Merged
merged 1 commit into from
Apr 12, 2024

Conversation

jessarcher
Copy link
Member

@jessarcher jessarcher commented Apr 10, 2024

This PR fixes an inconsistency issue with any Prompt fallbacks that rely on Symfony's choice component.

The inconsistency occurs when passing an array of options with numeric keys that are not sequential, starting from 0.

Prompts uses PHP's array_is_list function to determine whether the returned value should be the selected key or the selected value. Symfony's choice component differs by always returning the value for any array with all numeric keys, regardless of whether they are sequential from 0:

// Prompts without fallback

select('Foo', [
    '9999' => 'High', // returns '9999'
    '1' => 'Low', // returns '1'
]);

// Prompts with Symfony fallback

select('Foo', [
    '9999' => 'High', // returns 'High'
    '1' => 'Low', // returns 'Low',
]);

select('Foo', [
    '9999' => 'High', // returns '9999'
    'medium' => 'Medium', // returns 'medium',
    '1' => 'Low', // returns '1'
]);

This is an issue because the return value is inconsistent depending on whether or not the fallback was activated.

This PR solves this by wrapping each option in a new PromptOption class and passing them to Symfony as a list. Symfony can render the string version of the PromptOption class, and because it's passed as a list, it will always return the selected PromptOption class, allowing us to extract the correct value.

A nice side effect of always passing a list is that Symfony will not display the underlying array keys to the user, which makes the fallback more consistent with Prompts. It also simplifies the approach we use to inject the "None" option for the multiselect fallback.

* @param string|int|null $default
* @return string|int
*/
private function selectFallback($label, $options, $default = null)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made this method private because it ends up in the Command class, and I wanted to prevent IDEs from suggesting it.

@jessarcher jessarcher force-pushed the fix-prompts-fallback-with-numeric-keys branch 3 times, most recently from 8c399cb to 79c9f85 Compare April 11, 2024 04:22
@driesvints
Copy link
Member

@jessarcher we fixed the tests on the base branch, can you rebase?

@jessarcher jessarcher force-pushed the fix-prompts-fallback-with-numeric-keys branch from 79c9f85 to af7ad65 Compare April 11, 2024 23:16
@jessarcher jessarcher marked this pull request as ready for review April 12, 2024 07:47
@taylorotwell taylorotwell merged commit f29e0a5 into 11.x Apr 12, 2024
29 checks passed
@taylorotwell taylorotwell deleted the fix-prompts-fallback-with-numeric-keys branch April 12, 2024 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants