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 Mojo::Promise handling to the eval command #1306

Merged
merged 2 commits into from
Dec 31, 2018
Merged

Add Mojo::Promise handling to the eval command #1306

merged 2 commits into from
Dec 31, 2018

Conversation

jberger
Copy link
Member

@jberger jberger commented Dec 29, 2018

I find the eval command to be invaluable for one-off usage and testing, especially while developing. As I've been writing more promise apis and now as I'm writing fewer blocking apis due to the existence of Mojo::AsyncAwait, I'm finding the usage of promises in the eval command to be cumbersome to the point of not using it. With this change, one that I doubt would affect any usage in the wild (who wants to dump the stringified promise?), we can make a very comfortable interface for one-off testing of promise apis and make for easy examples and demos for newcomers or answer-seekers on irc.

Copy link
Member

@jhthorsen jhthorsen left a comment

Choose a reason for hiding this comment

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

I’m in favor of this change 👍

lib/Mojolicious/Command/eval.pm Outdated Show resolved Hide resolved
@kraih
Copy link
Member

kraih commented Dec 30, 2018

I never had a need for this, so not sure how to vote.

@jberger
Copy link
Member Author

jberger commented Dec 30, 2018

I never had a need for this, so not sure how to vote.

I had only rarely until a new project at $work. Since async functions necessarily return promises and since they nearly obviate the need for blocking apis I now have an app for which the model is entirely promises. I think once aa becomes more common this will be the case more and more often, it feels very natural

@jberger jberger force-pushed the eval_promise branch 2 times, most recently from b67fb23 to 9190b16 Compare December 30, 2018 21:59
@jberger
Copy link
Member Author

jberger commented Dec 31, 2018

@marcusramberg I know you're on vacation, but if you get a second, I'd appreciate a review/vote

@jberger
Copy link
Member Author

jberger commented Dec 31, 2018

Per the new PR policy, I'm calling a vote on this PR. @mojolicious/core

@kraih
Copy link
Member

kraih commented Dec 31, 2018

Neutral vote from me.

@marcusramberg
Copy link
Member

I'm also in favor of this change 👍

@kraih kraih merged commit 4d64043 into master Dec 31, 2018
@s1037989
Copy link
Contributor

What do you think about adding a Usage example?

Usage: APPLICATION eval [OPTIONS] CODE

  ./myapp.pl eval 'say app->ua->get("/")->result->body'
  ./myapp.pl eval 'say for sort keys %{app->renderer->helpers}'
  ./myapp.pl eval -v 'app->home'
  ./myapp.pl eval -V 'app->renderer->paths'
  ./myapp.pl eval -V '

I don't know if this is a good example, but something like:

  ./myapp.pl eval -v 'app->ua->get_p("https://mojolicious.org")->then(sub{shift->result->dom->at("title")->text})'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants