-
-
Notifications
You must be signed in to change notification settings - Fork 9.6k
Add ReStructuredText descriptor for console component. #45718
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
Conversation
Not sure why the Psalm test isn't passing. It doesn't seem to have anything to do with the changes in this PR. I find the output of that job hard to parse.
I think this is ready for review. |
Hey! I think @boesing has recently worked with this code. Maybe they can help review this? Cheers! Carsonbot |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
protected function describeInputArgument(InputArgument $argument, array $options = []) | ||
{ | ||
$this->write( | ||
'``'.($argument->getName() ?: '<none>')."``\n".str_repeat($this->argumentTitleUnderlineChar, Helper::width($argument->getName()) + 4)."\n\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
indentation looks wrong
} | ||
|
||
$this->write( | ||
'``'.$name.'``'."\n".str_repeat($this->argumentTitleUnderlineChar, Helper::width($name) + 4)."\n\n" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
here as well
use Symfony\Component\Console\Output\OutputInterface; | ||
|
||
/** | ||
* ReStructuredTextDescriptor descriptor. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can be removed
*/ | ||
protected function write(string $content, bool $decorated = true) | ||
{ | ||
parent::write($content, $decorated); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why overriding the method to do nothing else than calling the parent ?
|
||
The output format (txt, xml, json, or md) | ||
|
||
- Accept value: yes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be Accepts value
to be consistent with the usage of Is
below
*/ | ||
class ReStructuredTextDescriptor extends Descriptor | ||
{ | ||
private $applicationTitleUnderlineChar = '#'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some context in case anyone needs it: RST format doesn't define which characters to use for <h1>
, <h2>
, <h3>
, etc. RST defines a series of characters that you can use (###
, ===
, ^^^
, ~~~
, etc.) and the first one found is considered <h1>
, the second set of characters found is considered <h2>
, etc.
I'm not saying that we need to do the same here, but, for your consideration, the standard notation that we follow in Symfony Docs is:
h1 =====
h2 -----
h3 ~~~~~
h4 .....
Here we would be using:
h1 #####
h2 *****
h3 =====
h4 -----
Static installation | ||
------------------- | ||
|
||
Dump the script to a global completion file and restart your shell: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I haven't checked if this works too, but I think that "literal blocks" require:
- Contents to be indented by 4 spaces (this works perfectly in your code)
- The line before the literal block must end with two colons
::
(<-- this is the missing piece here)
See https://www.sphinx-doc.org/en/master/usage/restructuredtext/basics.html#literal-blocks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, the line before the literal block must end with ::
. See also the official reStructuredText markup specification: https://docutils.sourceforge.io/docs/ref/rst/restructuredtext.html#literal-blocks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice feature @grasmash! Thanks for contributing it.
@grasmash Are you still interested in finishing this PR? |
Closing as stalled. Don't hesitate to reopen when you're ready. |
I've cleaned this up, addressed feedback, and re-opened as #48981 |
This PR was squashed before being merged into the 6.3 branch. Discussion ---------- [Console] Add ReStructuredText descriptor | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | yes | Deprecations? | no | Tickets | Fix #45717 | License | MIT | Doc PR | n/a, descriptors are internal and not documented. ### Description The console component currently has descriptors for Markdown, Json, etc. This feature would add Restructured Text descriptor (rst) as an option. This replaces and addresses all feedback from #45718 ### Example ``` $helper = new DescriptorHelper(); $helper->describe($output, $this->getApplication(), [ 'format' => 'rst', ]); ``` Commits ------- f3ab66e [Console] Add ReStructuredText descriptor
Description
The console component currently has descriptors for Markdown, Json, etc. This feature would add Restructured Text descriptor (rst) as an option.
Example