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

Extract Looping Mechanisms #154

Conversation

ProjektGopher
Copy link
Contributor

@ProjektGopher ProjektGopher commented Jun 11, 2024

I've successfully implemented an asynchronous, non-blocking prompt, which can simply extend our existing abstract prompt, leaving backwards compatibility unaffected. By using a reactphp event loop, I'm able to trigger renders and do other operations at any time since reading terminal input is no longer a blocking operation. This can almost certainly remove our reliance on forking using the Process Control extension in the Spinner class, and there's a slight chance that it could solve our Windows story (I still have to verify this, so don't quote me). It also happens to enable me to do some really cool stuff involving websockets that I'll have ready soon.

However, none of that is in this PR.

This PR is exclusively to make my new feature compatible with Laravel Prompts irrespective of its ultimate home (I'm totally fine with it living in my community prompts package if that's what has to happen).

  • I've updated any references to self to static when setting or getting the ConsoleOutput object so that the $output static property is set on the child.
  • I've extracted the loop for registering keypress mocks into a method that can be overridden in an extending class.
  • I've extracted the looping mechanism in the prompt method which can now also be overridden in an extending class.
  • I've added an empty Nothing support class.

Since the while loop is now a function, if we were to rely on returning null to continue looping, we could no longer return null from any prompt. Since null is a valid return value, I needed a way to, in effect, check for a void return.

edit: I checked, and non-blocking i/o still wont work on Windows.

@ProjektGopher
Copy link
Contributor Author

Based on the following comment from @jessarcher in #77, I tested making the text prompt async and using it in a new command in a fresh project to confirm that the output buffers will work as expect. Everything appears fine.

When Prompts are used with Laravel commands, we set the output buffer to the existing one we already have in the Laravel command to track newlines between Prompt output and other output in the command. The ConsoleOutput class is only used when using Prompts without Laravel.

@taylorotwell
Copy link
Member

Drafting pending @jessarcher taking a look.

@taylorotwell taylorotwell marked this pull request as draft June 12, 2024 16:25
@taylorotwell
Copy link
Member

After chatting with @jessarcher I think we're going to hold off on this one!

ProjektGopher added a commit to artisan-build/community-prompts that referenced this pull request Jul 13, 2024
This is a temporary solution to laravel/prompts#154 having been closed.

That PR was a 'compatibility' PR to enable easy integration of the AsyncPrompt, PatchedPrompt will act as a 'shim' until I'm able to convince Taylor to approve my PR.

The whole point of this shim is to extract the looping mechanisms into separate functions that can be overridden in implementing classes, enabling the easy use of a ReactPHP event loop.

Also includes a bunch of code styling.
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

2 participants