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

Add info note #36

Merged
merged 2 commits into from
Aug 7, 2023
Merged

Add info note #36

merged 2 commits into from
Aug 7, 2023

Conversation

sebastianpopp
Copy link
Contributor

I'm using prompts outside of Laravel and think it would be helpful to be able to display a simple success message the same way I can display an error message. The name of the function is based on the function in Laravel commands.

@jessarcher
Copy link
Member

Thanks for the PR, @sebastianpopp!

It's cool to hear you're using Prompts in other PHP projects and that you found the note components!

I'm okay with merging this, but I'm not 100% sure what the future holds for the notes (and spinner), which are currently undocumented. I think they're very useful, but they don't completely replace the informational components from Symfony, like tables and progress bars. I haven't had time to think about where the "non-prompting" parts of Prompts fit and the value they can bring. In any case, I don't anticipate us removing them, but they could potentially take a different shape in a later release.

@utsavsomaiya
Copy link

utsavsomaiya commented Aug 4, 2023

In the Laravel application, we have a mix and match situation concerning helper function names. Currently, the helper function info is used to interact with the Log channel. To enhance clarity and avoid conflicts, I suggest renaming this helper function to success. By doing so, we can easily differentiate between logging information and indicating a successful operation.

what do you think @jessarcher?

@jessarcher
Copy link
Member

In the Laravel application, we have a mix and match situation concerning helper function names. Currently, the helper function info is used to interact with the Log channel. To enhance clarity and avoid conflicts, I suggest renaming this helper function to success. By doing so, we can easily differentiate between logging information and indicating a successful operation.

what do you think @jessarcher?

I don't have a strong opinion. The prompt functions are all namespaced, so there is no conflict. The info name matches the corresponding $this->info() method in Artisan console commands, but I understand it's not ideal to share a name as Laravel's global info function and need to alias an import if you want to use both in the same file.

I'll need to think more about the role of these functions and have a chat with Taylor. I probably should have left them out of the package until I had a clearer picture for them.

@utsavsomaiya
Copy link

No worries dear. Thanks.

@taylorotwell taylorotwell merged commit 1b3ab52 into laravel:main Aug 7, 2023
3 checks passed
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.

4 participants