[Console][DescriptorHelper] plugs the docu for 2.3, a second PR follows after this is merged #3430
Conversation
| =========== | ||
|
|
||
| .. versionadded:: 2.3 | ||
| Descriptors and Descriptor Helper were introduced in Symfony 2.3. |
There was a problem hiding this comment.
This should be indented by 4 spaces
|
@weaverryan are these adaptations enough or we still think more is needed. I am ready to comply, just that i am posing an opinion as well, but please don't hesitate to tell me if i should override. |
| MarkdownDescriptor | ||
| JsonDescriptor | ||
|
|
||
| Whenever you need to use a custom descriptor |
|
@wouterj not sure why you removed your comment, but you did. I guess you realized that HelpCommand is not the focus here. That helper is a mechanism bus to plug and use the descriptors in other commands where they are actually used. |
|
@cordoval I did not. It's still there, hidden in the oudated diff: #3430 (comment) And we are talking about the help command here. The default Descriptor can only render help of a command, input definition, etc. That'll not be helpfull in other commands than the help command, does it? |
|
@wouterj the only application for descriptors is not the help command. That is why you add only custom descriptors to the helper. Of course it will. |
|
@cordoval but the point is that the example you gave is only talking about the default command descriptor. You should not use the available implementation, only the It looks like you completely missed the second part of both Ryan's and my comments. |
|
no, my paragraphs were not talking about only the default command descriptor. People can be told what they can do, they can extend or implement, is their choice. Even when extending it is more for reuse the good part, if you want to use other parts you extend them or finally implement. so no, even with the input_definition stuff, it can be reused because is common, nobody says that you will keep all the defaults. I think when descriptors was created was keeping in mind this. I don't think i have missed those parts. I just disagree slightly. |
I think this is pretty clear that it's overriding and creating a new Descriptor class for commands and not for other things. You should explain creating descriptors for other things, like a Users object. Then you only need the <?php
use Symfony\Component\Console\Descriptor\DescriptorInterface;
use Acme\User;
use Acme\UserGroup;
abstract class UserDescriptor implements DescriptorInterface
{
protected $output;
public function describe(OutputInterface $output, $subject, array $options = array())
{
$this->output = $output;
if ($subject instanceof User) {
$this->describeUser($subject);
} elseif ($subject instanceof UserGroup) {
$this->describeUserGroup($subject);
}
}
abstract protected function describeUser(User $subject);
abstract protected function describeUserGroup(UserGroup $subject);
}
use Symfony\Component\Console\Helper\TableHelper;
class TextUserDescriptor extends UserDescriptor
{
protected function describeUser(User $subject)
{
$lines = array();
$lines[] = 'Name: '.$subject->getName();
$lines[] = 'Age: '.$subject->getAge();
$lines[] = 'Group: '.$subject->getGroup()->getName();
$this->output->writeln($lines);
}
protected function describeUserGroup(UserGroup $subject)
{
$table = new TableHelper();
$table->setLayout(TableHelper::LAYOUT_COMPACT);
$table->setHeaders(array('Name', 'Age'));
foreach ($subject->getUsers() as $user) {
$table->addRow(array($user->getName(), $user->getAge()));
}
$this->output->writeln(array(
'Group name: '.$subject->getName(),
'',
));
$table->render($this->output);
}
}In that case, this article will be helpfull. Currently, I don't see what this article tries to explain to me. |
|
yes this is a good point @wouterj, i fully agree with showing this example and then that should be enough to get it across. I will work on this if we are agreeing on this. Now it is clear what needs to be done. Thanks for the bearing with me 👶 |
|
|
||
| Descriptors act upon the flags passed to the console app commands:: | ||
|
|
||
| php app.php some:command --format=json --raw=true |
There was a problem hiding this comment.
bash commands should be prefixed with $
|
👶 ~ gsh p:squash 3430 Luiss-MacBook-Pro-3 [21:42:45]
OUT > Fetching origin
OUT > Fetching cordoval
ERR > Already on '2803-document-descriptors-and-descriptorhelper'
OUT > Your branch is up-to-date with 'cordoval/2803-document-descriptors-and-descriptorhelper'.
OUT > [2803-document-descriptors-and-descriptorhelper 121e5bf] 2803-document-descriptors-and-descriptorhelper
4 files changed, 158 insertions(+), 1 deletion(-)
8000
create mode 100644 components/console/helpers/descriptorhelper.rst
ERR > To git@github.com:cordoval/symfony-docs.git
ERR > + 94ac9cf...121e5bf 2803-document-descriptors-and-descriptorhelper -> 2803-document-descriptors-and-descriptorhelper (forced update)
OUT > Branch 2803-document-descriptors-and-descriptorhelper set up to track remote branch 2803-document-descriptors-and-descriptorhelper from cordoval. |
| TextDescriptor | ||
| XmlDescriptor | ||
| MarkdownDescriptor | ||
| JsonDescriptor |
There was a problem hiding this comment.
show me please an example that I may learn this, thanks man
There was a problem hiding this comment.
|
🚢 🇮🇹 🐤 |
| * TextDescriptor | ||
| * XmlDescriptor | ||
| * MarkdownDescriptor | ||
| * JsonDescriptor |
|
done |
|
ping @weaverryan @wouterj |
| you should register it with the helper above adding:: | ||
|
|
||
| $helper->register('no-color', new NoColorDescriptor()); | ||
|
|
There was a problem hiding this comment.
I don't think this section is relevant, because I don't think Symfony ever looks for the descriptor inside the helper set. The descriptors are only ever used directly, for example:
- https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Console/Command/HelpCommand.php#L83
- https://github.com/symfony/symfony/blob/master/src/Symfony/Bundle/FrameworkBundle/Command/ContainerDebugCommand.php#L115
So you don't register it, I think you just simply create one and use it.
|
@weaverryan thanks for the feedback, i will take action next next week if not before |
|
@cordoval Thanks buddy :) |
|
it also needs a rebase |
|
ping @cordoval! |
|
@weaverryan !! 😊 I will try to do it after my work ends tonight |
|
heh i messed it up, let me fix it, or try |
|
my git powers did not fail me. @weaverryan i just hope we don't let 4 more months go. If you or someone else can take it fine with me, else I don't want to get grilled now at least not on this one. 😊 |
|
I removed the Finished label and replaced it with Stalled. I think @cordoval did a nice job explaining things. But to be frank, I'm not sure that this feature is even worth explaining - it's not clear to me why it was even refactored into a helper in 2.3. The way things are setup, it's not possible to add, for example, a Yaml dumper. So being able to dump help information in a different format is not a use-case. Without that, it just seems like we have a few classes that you can choose to use on your own to help output help information if you have that use-case. This seems like a very abstract use-case, and I personally would just output things directly (e.g. Am I missing a real use-case? Is this even worth documenting? Thanks! |
|
I'm going to close this PR, unless someone can tell me what the use-case is to document this. @cordoval Sorry about this - you did a really nice job of working on this and explaining things. And to be honest, I think we needed someone to try documenting this to see that maybe it is not worth documenting. Cheers! |
Sent using Gush