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

[DX] Improve the output of all Symfony commands #11784

Closed
javiereguiluz opened this issue Aug 27, 2014 · 33 comments
Closed

[DX] Improve the output of all Symfony commands #11784

javiereguiluz opened this issue Aug 27, 2014 · 33 comments
Labels
Console DX DX = Developer eXperience (anything that improves the experience of using Symfony)

Comments

@javiereguiluz
Copy link
Member

(Note: I know that this is issue is mostly related to Symfony bundles and not Symfony code itself, but I think that opening the issue in this repo is the best thing to promote a discussion)

The problem

The output of the Symfony commands is completely different from one command to another. This makes more difficult to use them and shows a lack of attention to details:

security_check
yaml_lint
twig_debug
swiftmailer_debug
router_match
generate_bundle
container_debug
config_debug
swiftmailer_spool_send

Moreover, error messages are not designed for humans:

error_yaml_lint
error_translation_debug

The solution

If this DX issue is approved by @symfony/mergers and @symfony/deciders I propose the following:

  • Create a (very small) work group of people that want to work on this.
  • Define a simple (and if possible, visually pleasant) "style guide" for Symfony commands.
  • Make (if necessary) some changes on Console component to allow this work.
  • Adapt each command output to this new style guide using the new features of the component.
@ghost
Copy link

ghost commented Aug 27, 2014

the styleguide should also include how one should name commands. Also, in some places we have something:debug and others debug:something

@stof
Copy link
Member

stof commented Aug 27, 2014

@jrobeson we are using debug:* everywhere in core in Symfony 2.6 (with BC aliases for *:debug)

@pgodel
Copy link
Contributor

pgodel commented Aug 27, 2014

I am a heavy user of commands and I noticed this a few times. Love the idea of the styleguide. I would like to help with this topic.

@wouterj
Copy link
Member

wouterj commented Aug 30, 2014

I like this idea. I propose to take the security:check command output as an example. Resulting in a style guide like this:

  • Start with an empty line;
  • Use the blue forground color for headlines;
  • Visualize the headline levels using reSt formatting;
  • Use blocks to output important information;
  • Use the <info> style for questions;
  • When using enum questions, put the allowed values within square brackets after the question using the <comment> style;
  • Use the borderless table to output tabular data;
  • Only expose exceptions when they are related to the system, use custom error output in all different cases (e.g. in a lint command).

This means we have to add a block style, an example of this can be seen in the SensioGeneratorBundle: https://github.com/sensiolabs/SensioGeneratorBundle/blob/master/Command/Helper/DialogHelper.php#L52-59

@javiereguiluz
Copy link
Member Author

@wouterj thanks for your proposal!

Unfortunately, for now this PR is just an idea. It hasn't been approved by the Symfony Core, so we must wait for their response before starting to work on it.

@wouterj
Copy link
Member

wouterj commented Aug 30, 2014

@javiereguiluz yes, I know. But I wanted to give an example of such a styleguide and I wanted to share what I think is best :)

@fabpot
Copy link
Member

fabpot commented Aug 30, 2014

Big 👍 for the style guide. I have some very opinionated ideas about how things should look like, so whenever we have a draft, I would be more than happy to have a look at it.

@pgodel
Copy link
Contributor

pgodel commented Aug 30, 2014

I am not sure if the following idea applies for this issue, but I think it is related, although it could certainly be a new one.

The help command offers the ability to output xml. This is very useful for consuming the output with other scripts. What about having some kind of native support for xml output for all commands? This could be very useful for catching errors, consuming data printed on tables or lists, etc, when calling commands from other scripts

@ghost
Copy link

ghost commented Aug 30, 2014

i'd like a more directly scriptable/machine readable output like git has vs xml. the most used commands themselves (push,pull, clone, fetch) are just porcelains (git nomenclature) around the internal commands

@hhamon
Copy link
Contributor

hhamon commented Aug 31, 2014

👍 too! Making more standard and unified commands would be better.

@ajanssens
Copy link

👍 for this, i love the unity and the new security:check looks refreshing. I think we could take some parts of it as guide. :)

Nice one @javiereguiluz !

@fabpot
Copy link
Member

fabpot commented Sep 1, 2014

Besides the guide, we should also make sure that the helpers follow our recommendations. The FormatterHelper helper being the one which is the most important as we can add some shortcut to ease developing "compliant" command output.

@javiereguiluz
Copy link
Member Author

I'm preparing a first draft of this style guide to bootstrap discussions with the people that showed interest in this issue.

@sroze
Copy link
Contributor

sroze commented Sep 13, 2014

👍 Really nice idea ! Would be happy to help you on that work.

@webmozart
Copy link
Contributor

Great proposal 👍

@stof stof added Console DX DX = Developer eXperience (anything that improves the experience of using Symfony) labels Sep 20, 2014
@javiereguiluz
Copy link
Member Author

We've prepared a preview of the new Symfony Console Style Guide. Some things are missing, but please download it, review it and provide some feedback:

Download Symfony Console Style Guide (PDF, 107 KB)

UPDATE: the original PDF is no longer available, as we've separated it into two different documents (the style guide and the implementation guide).

@ajanssens
Copy link

That's nice ! I really like this. The only thing that I find not really relevant is the red warning which I do not find enough explicit on the danger of the command from the "php app/console doctrine:database:drop"

I find the old way more explicit.

@cordoval
Copy link
Contributor

nice docu @javiereguiluz maybe to be more concrete we can have the styleguide put into some Factories::getStyles or a class with names on it something that can provide concrete names from the Guideline, that will make the guideline even more easy to implement and follow? or i am missing the point here.. great work JJ

@javiereguiluz
Copy link
Member Author

@cordoval you have exactly described the next step! I'm doing it right now :)

@cordoval
Copy link
Contributor

you rockn', I am looking forward to experiment this new guide with Gush because all are symfony commands 👍 so i will be rather active if it involves command output beautification. Gush and Bldr I believe can greatly benefit from this cc @aequasi

@stof
Copy link
Member

stof commented Sep 24, 2014

I see an issue with this proposal: the fact that it relies on table output makes it hard to stream the output.
For instance, assets:install currently displays the progress by displaying each bundle when its asset get installed. The proposed output requires displaying all names at once, either before or after the process.

The style guide should avoid enforcing interfaces which cannot be streamed, as streming is a common need in CLI.

@javiereguiluz
Copy link
Member Author

@stof good catch! One of the problems of the style guide is that I mixed design and implementation. It should have been an agnostic style guide, but I wanted to show real examples based on the current commands.

In any case, the solution to this problem is very easy: drop the table output because it's inappropriate for this command and output the info with simple debug messages.

@stof
Copy link
Member

stof commented Sep 24, 2014

@javiereguiluz IMO, you should split your work in 2:

  • a proper styleguide defining the rules
  • a separate document showing how existing commands could be updated to match this styleguide.

The first document would then become part of the Symfony documentation, while the second one would become obsolete as soon as the migration is completed (we should keep it around for a while to help bundle to migrate as well)

@javiereguiluz
Copy link
Member Author

@stof good idea! I'll do that once we receive more community feedback and we consider the style guide as finished.

@bruli
Copy link

bruli commented Sep 24, 2014

I like it.

@pyrech
Copy link
Contributor

pyrech commented Sep 24, 2014

Really nice!

  • I find the // symbol for infos not intuitive for a display in shell but maybe only a question of habit :)
  • Agreed with @ajanssens on the php app/console doctrine:database:drop proposed output, I find the warning less readable and less explicit

Anyway, I look forward to see it applied to all commands!

@wouterj
Copy link
Member

wouterj commented Sep 24, 2014

If you put the guide part in the docs now, it would be easier to comment on it

@AndreiIgna
Copy link

hey @javiereguiluz good doc so far! and good idea for this thread. A few ideas:

  • as someone said, dangerous things should stand out more
  • global styles for info/warning/error alerts (as bootstrap css have). Also drop the words OK, ERROR at the beginning

@yahuarkuntur
Copy link
Contributor

Did you mean "pronunciation"? Sorry!!!! The styleguide looks very promising! Keep the awesome work ❤️

Also at page 20 of the document, the subtitle should be "PROPOSAL" instead of "CURRENT".

Did I miss the repo of the document to send minor typos?

@javiereguiluz
Copy link
Member Author

The style guide has been updated. All the discussion has been moved to symfony/symfony-docs#4265 as requested by @wouterj. Please provide as much feedback as you can. Thanks!

@fabpot fabpot closed this as completed Sep 24, 2014
@wouterj
Copy link
Member

wouterj commented Sep 24, 2014

@javiereguiluz I meant as an rst file instead of an PDF, sorry for being unclear

@javiereguiluz
Copy link
Member Author

@wouterj I'm afraid that right now we can't transform the guide to RST because it would take a lot of time. I propose you instead to discuss the PDF contents and once it's finished, let's transform it.

@wouterj
Copy link
Member

wouterj commented Sep 24, 2014

Don't worry, I'm already doing that
Op 24 sep. 2014 19:45 schreef "Javier Eguiluz" notifications@github.com:

@wouterj https://github.com/WouterJ I'm afraid that right now we can't
transform the guide to RST because it would take a lot of time. I propose
you instead to discuss the PDF contents and once it's finished, let's
transform it.


Reply to this email directly or view it on GitHub
#11784 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Console DX DX = Developer eXperience (anything that improves the experience of using Symfony)
Projects
None yet
Development

No branches or pull requests