Skip to content

[Console] HelpCommand strips arguments with same names as output styles #24225

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

Closed
pbull opened this issue Sep 15, 2017 · 4 comments
Closed

[Console] HelpCommand strips arguments with same names as output styles #24225

pbull opened this issue Sep 15, 2017 · 4 comments

Comments

@pbull
Copy link

pbull commented Sep 15, 2017

Q A
Bug report? yes
Feature request? no
BC Break report? no
RFC? no
Symfony version 2.7.x, 3.3.x

When a console command defines one or more input arguments that have the same name(s) as defined OutputFormatterStyles, the HelpCommand output strips those arguments from the Usage section of the output and applies that style to the content that follows.

This is occurring because in Symfony\Component\Console\Input\InputDefinition::getSynopsis() (https://github.com/symfony/symfony/blob/master/src/Symfony/Component/Console/Input/InputDefinition.php#L398) those argument names are wrapped in < and > and so the argument is treated as the opening tag of a style wrapper:

        foreach ($this->getArguments() as $argument) {
            $element = '<'.$argument->getName().'>';
[...]
        }

An example command that defines the argument names "info", "comment", and "foo" as follows:

    protected function configure() {
        $this->addArgument('info', InputArgument::REQUIRED, 'The first argument.');
        $this->addArgument('comment', InputArgument::OPTIONAL, 'The second argument.');
        $this->addArgument('foo', InputArgument::OPTIONAL, 'The third argument.');
    }

... would appear as follows when running help for that command. Note that the yellow foreground color of the default comment style is being applied to the following text in the output:

console1

... when the expected output would be more like:

console2

@pbull
Copy link
Author

pbull commented Sep 15, 2017

While a simple fix is to prepend the first < with a backslash (and this fixes both the text and markdown output of the help command), this will require updating a number of tests to account for that backslash, and that character is also inserted into the JSON and XML formatted output from help. I don't know if that is acceptable, or if the JsonDescriptor and XmlDescriptor should account for that somehow (e.g. passing through stripslashes()?):

--- a/src/Symfony/Component/Console/Input/InputDefinition.php
+++ b/src/Symfony/Component/Console/Input/InputDefinition.php
@@ -395,7 +395,7 @@ class InputDefinition
         }

         foreach ($this->getArguments() as $argument) {
-            $element = '<'.$argument->getName().'>';
+            $element = '\<'.$argument->getName().'>';
             if (!$argument->isRequired()) {
                 $element = '['.$element.']';
             } elseif ($argument->isArray()) {

@ro0NL
Copy link
Contributor

ro0NL commented Sep 15, 2017

using \ here looks like a leaked implementation detail, cant we simply escape on the rendering side? See OutputFormatter::escape()

edit: nice catch btw 👍

@pbull
Copy link
Author

pbull commented Sep 15, 2017

Yes, looks like this could be handled more gracefully in the TextDescriptor. I'll give that a try.

@sroze
Copy link
Contributor

sroze commented Oct 6, 2017

PRed a fix in #24455

chalasr pushed a commit that referenced this issue Oct 8, 2017
This PR was merged into the 2.7 branch.

Discussion
----------

[Console] Escape command usage

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #24225
| License       | MIT
| Doc PR        | ø

Escape the console usage to prevent arguments named `info` or similar to be formatted.

Commits
-------

2141056 Escape command usage when displaying it in the text descriptor
@chalasr chalasr closed this as completed Nov 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants