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

Cookbook about Command in Application with AnsiToHtml #5062

Closed
wants to merge 1 commit into from

Conversation

rvanlaak
Copy link
Contributor

@rvanlaak rvanlaak commented Mar 6, 2015

Q A
Doc fix? no
New docs? yes
Applies to >= 2.3
Fixed tickets symfony/symfony#11392

single: Console; Use commands in your Controller

How to make use of a Command in your application
===============================
Copy link
Member

Choose a reason for hiding this comment

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

Minor syntax issue: this line should be as long as its associated headline.

@javiereguiluz
Copy link
Member

@rvanlaak thanks for submitting this nice new cookbook!

@rvanlaak
Copy link
Contributor Author

rvanlaak commented Mar 6, 2015

Was thinking about adding more information about the Theming and a code sample about the Twig extension, but seems like that is too much info for the cookbook article. What do you think @javiereguiluz ?

@javiereguiluz
Copy link
Member

@rvanlaak I agree with you: it looks like too much information for this cookbook. I personally love short and straight-to-the-point cookbooks, so I really like your tutorial as it is now. Thanks.

@rvanlaak
Copy link
Contributor Author

rvanlaak commented Mar 6, 2015

Another thing, where can I find new decisions the such as the one you mentioned about not using acme examples? Could not find anything on http://symfony.com/doc/current/contributing/documentation/index.html and probably such decisions are made in the Google Groups, but we can not follow that the entire day 👍

@javiereguiluz
Copy link
Member

I don't really know where we decided this, because I talk with doc maintainers via email, IRC and GitHub :( In any case, the #4740 PR is the one which made the biggest recent change to remove acme, demo, foo and nonsenses like those.

I'm going to check if we can add something about this in the formal document that you linked. Thanks for your comments.

@wouterj
Copy link
Member

wouterj commented Mar 6, 2015

@rvanlaak we don't use the Google Groups and you sure don't have to watch them in order to contribute something (these reviews are just to help you adding amazing content to the docs, it doesn't matter if your PR isn't 100% correct at first).

Decisions like this aren't really strict standards or decisions. As an important rule, it is always better using real-world examples than foobaring your examples. While being more clear, it also works as an advertisement for the framework ("wow, can I also create X with this?").

Since the best practices, we are updating the docs to follow them. One of the most important changes is to no longer recommend using multiple bundles, but just one bundle: AppBundle. We are still in the process of it, as it's a very hard task to do nicely.

.. index::
single: Console; Use commands in your Controller

How to make use of a Command in your application
Copy link
Member

Choose a reason for hiding this comment

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

I think something like "How to Call a Command from a Controller" better matches the contents.

@xabbuh
Copy link
Member

xabbuh commented Mar 7, 2015

Thank you for creating the pull request, @rvanlaak. It's nice work. However, I think we should start the text with a short introduction to show use cases when you might want to call a command from a controller.

I was thinking about something like:

You may have the need to execute some function that is only available in a console command. Usually, you should refactor the command and move some logic into a service that can be reused in the controller. However, when the command is part of a third-party library, you wouldn't want to modify or duplicate their code, but want to directly execute the command instead.

What do you think?

@rvanlaak
Copy link
Contributor Author

rvanlaak commented Mar 8, 2015

Agree with you @xabbuh , so I've updated the example to a real world use case, eg. sending the SwiftMailer spool directly from the controller. Let me know if it's fine now, so I can squash the commits into one.

@@ -6,6 +6,7 @@ Console

console_command
usage
command_in_controller
Copy link
Member

Choose a reason for hiding this comment

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

You need to also add a reference to the document in /cookbook/map.rst.inc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

instead.

An example of this is sending the emails that Swift Mailer spooled earlier
:doc:`using the ``swiftmailer:spool:send`` command </cookbook/email/spool>`. Symfony
Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately, you cannot use literals here.

@rvanlaak
Copy link
Contributor Author

Rebased the latest changes @javiereguiluz

@javiereguiluz
Copy link
Member

@rvanlaak awesome! Thanks for working on this and for your infinite patience with our comments and suggestions.

@xabbuh this nice PR is now ready to be labeled as finished, don't you think?

@rvanlaak
Copy link
Contributor Author

No problem, great way of getting known with the community guidelines 👍


The :doc:`Console component documentation </components/console/introduction>` covers
how to create a console command. This cookbook article covers how to use a console command
directly in your application.
Copy link
Member

Choose a reason for hiding this comment

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

"[...] from a controller."?

Copy link
Contributor

Choose a reason for hiding this comment

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

this also works in any service? so i am fine with this statement.

Copy link
Member

Choose a reason for hiding this comment

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

the article is calld "from a controller". So I think we should change this.

@rvanlaak
Copy link
Contributor Author

rvanlaak commented Apr 9, 2015

Fixed the outstanding issues. Let me know if the commits are ready for a rebase 👍

@wouterj
Copy link
Member

wouterj commented Apr 9, 2015

👍

@xabbuh
Copy link
Member

xabbuh commented Apr 12, 2015

👍 Great work @rvanlaak!

@rvanlaak
Copy link
Contributor Author

Completed the rebase, squased all commits into rvanlaak@62ae7c3

$content = stream_get_contents($output->getStream());
fclose($output->getStream());

return $content;
Copy link
Contributor

Choose a reason for hiding this comment

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

i think this is wrong no? why are you not returning a valid HttpResponse instead?

@rvanlaak
Copy link
Contributor Author

rvanlaak commented May 5, 2015

@xabbuh anything I've got to change for this PR?

use Symfony\Component\Console\Input\ArrayInput;
use Symfony\Component\Console\Output\StreamOutput;

class SpoolController extend Controller
Copy link
Member

Choose a reason for hiding this comment

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

extends

@xabbuh
Copy link
Member

xabbuh commented May 6, 2015

@rvanlaak I left two minor comments. After that I think it's finished.

... and color coding Ansi output using SensioLabs AnsiToHtml

Documentation about using Command in Controller

... and color coding Ansi output using SensioLabs AnsiToHtml

Line syntax

Change component link

Update command_in_controller.rst

with real world example about Swift Mailer.

Update command_in_controller.rst

Added command_in_controller to map.rst.inc

Update map.rst.inc

Documentation about using Command in Controller

Update command_in_controller.rst

Documentation about using Command in Controller

Documentation about using Command in Controller

Update command_in_controller.rst

Update command_in_controller.rst

Documentation about using Command in Controller
@rvanlaak
Copy link
Contributor Author

rvanlaak commented May 6, 2015

@xabbuh nice pick, fixed and rebased it!

@xabbuh
Copy link
Member

xabbuh commented May 6, 2015

👍

weaverryan added a commit that referenced this pull request May 23, 2015
…Rvanlaak)

This PR was submitted for the 2.7 branch but it was merged into the 2.3 branch instead (closes #5062).

Discussion
----------

Cookbook about Command in Application with AnsiToHtml

| Q             | A
| ------------- | ---
| Doc fix?      | no
| New docs?     | yes
| Applies to    | >= 2.3
| Fixed tickets | symfony/symfony#11392

Commits
-------

0799366 Documentation about using Command in Controller
@weaverryan
Copy link
Member

Thanks so much @rvanlaak! This is exactly the small, targeted article that we love having. And welcome to the process and thanks for sticking with us through all the reviews.

I have now merged this into the 2.3 branch, and opened a new PR - #5299 - with a few further tweaks. If you can review that, it would be awesome.

Cheers!

@weaverryan weaverryan closed this May 23, 2015
weaverryan added a commit that referenced this pull request May 23, 2015
This PR was merged into the 2.3 branch.

Discussion
----------

Command controller tweaks to #5062

| Q             | A
| ------------- | ---
| Doc fix?      | yes #5062
| New docs?     | no
| Applies to    | 2.3+
| Fixed tickets | #5062

Hi guys!

This is a proofread and bug fix after merging #5062. Notably, I changed to use the `BufferedOutput` and returned a Response from the controller instead of the string.

Please let me know if you see any errors - I was coding this right inside the docs 😇.

Thanks!

Commits
-------

aad7277 Fixing things thanks to Wouter
59f8c95 Tweaks to the new using commands in a controller article
@rvanlaak rvanlaak deleted the 2.7 branch June 11, 2015 08:43
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.

9 participants