Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[Console] Added suggestions for missing packages#29865

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:masterfromprzemyslaw-bogusz:console_packages
Feb 21, 2019

Conversation

@przemyslaw-bogusz
Copy link
Contributor

QA
Branch?master
Bug fix?no
New feature?yes
BC breaks?no
Deprecations?no
Tests pass?yes
LicenseMIT

Currently, when someone runs one of the most common commands, e.g.server:run, but does not have a required package installed, they will get a general'There are no commands defined...' message.

This commit adds a more useful message, informing the user about a package that might be missing and suggesting a command that should be run in order to install it, e.g.composer require symfony/web-server-bundle --dev.

@chalasr
Copy link
Member

I like the idea.
I think this belongs to the framework, not the Console component.
Components should not be aware of bundles, it's the other way around.
FrameworkBundle looks like a perfect fit for this, as it represents the glue for all Symfony components and bundles, and it has its ownconsole Application.

@chalasrchalasr added this to thenext milestoneJan 13, 2019
@ro0NL
Copy link
Contributor

i'd prefer something like we did for twig:https://github.com/symfony/symfony/blob/master/src/Symfony/Bridge/Twig/UndefinedCallableHandler.php

i think its more straightforward to handle it in e.g. a catch-all event listener for unknown commands, opposed to tracking a list on high level (console application).

@przemyslaw-bogusz
Copy link
ContributorAuthor

I had some doubts adding it to the main application, since it is widely used as a standalone component, but I wanted the suggestions to be available for some, who started his project with a symfony/skeleton (so without the FrameworkBundle) and tries to use features, that are not installed.

By the way, can you tell me, why my CI checks failed? Did I forget about something in my PR?

@przemyslaw-bogusz
Copy link
ContributorAuthor

My mistake. I confused FrameworkBundle with SensioFrameworkExtraBundle. I will rework the PR.

But my other question about the CI checks is still valid :)

@przemyslaw-bogusz
Copy link
ContributorAuthor

I went with the suggestion from@ro0NL and created an event subscriber. I can always switch to a try/catch inFrameworkBundle Console Application.

@przemyslaw-boguszprzemyslaw-boguszforce-pushed theconsole_packages branch 2 times, most recently from018f91a to7c80043CompareJanuary 14, 2019 23:49
Copy link
Member

@chalasrchalasr left a comment

Choose a reason for hiding this comment

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

We will need some tests

@chalasrchalasr added the DXDX = Developer eXperience (anything that improves the experience of using Symfony) labelJan 15, 2019
@przemyslaw-boguszprzemyslaw-boguszforce-pushed theconsole_packages branch 2 times, most recently from56f0342 to3bf0806CompareJanuary 17, 2019 09:01
@przemyslaw-bogusz
Copy link
ContributorAuthor

I improved the logic. For example, when someone hasWebServerBundle and runsserver:dump, the message will suggest installingVarDumper (along with alternatives from the originalCommandNotFoundException). But onserver:foo will add nothing - just show the alternatives.

@przemyslaw-bogusz
Copy link
ContributorAuthor

@fabpot Done.

Copy link
Member

@nicolas-grekasnicolas-grekas left a comment

Choose a reason for hiding this comment

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

(with minor comment)

@przemyslaw-boguszprzemyslaw-boguszforce-pushed theconsole_packages branch 2 times, most recently fromf3187ec toba3f404CompareFebruary 14, 2019 13:55
@przemyslaw-bogusz
Copy link
ContributorAuthor

Too many reviews in a short period of time and things got a little messy for a moment. I've implemented all suggestions the code, fixed the tests. The one Travis CI test that fails, failed before and I guess it has nothing to do with my code directly.

The only thing left to decide is whether to putthe back in the exception message by default, or add it to selected messages. I'm in favor ofthe by default.

@fabpot
Copy link
Member

Thank you@przemyslaw-bogusz.

@fabpotfabpot merged commit423a54f intosymfony:masterFeb 21, 2019
fabpot added a commit that referenced this pull requestFeb 21, 2019
…myslaw-bogusz)This PR was squashed before being merged into the 4.3-dev branch (closes#29865).Discussion----------[Console] Added suggestions for missing packages| Q             | A| ------------- | ---| Branch?       | master| Bug fix?      | no| New feature?  | yes| BC breaks?    | no| Deprecations? | no| Tests pass?   | yes| License       | MITCurrently, when someone runs one of the most common commands, e.g. `server:run`, but does not have a required package installed, they will get a general **'There are no commands defined...'** message.This commit adds a more useful message, informing the user about a package that might be missing and suggesting a command that should be run in order to install it, e.g. `composer require symfony/web-server-bundle --dev`.Commits-------423a54f [Console] Added suggestions for missing packages
@nicolas-grekasnicolas-grekas modified the milestones:next,4.3Apr 30, 2019
@fabpotfabpot mentioned this pull requestMay 9, 2019
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas approved these changes

@fabpotfabpotfabpot approved these changes

@chalasrchalasrchalasr approved these changes

+2 more reviewers

@mbabkermbabkermbabker left review comments

@ro0NLro0NLro0NL left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

DXDX = Developer eXperience (anything that improves the experience of using Symfony)FeatureFrameworkBundleStatus: Reviewed

Projects

None yet

Milestone

4.3

Development

Successfully merging this pull request may close these issues.

7 participants

@przemyslaw-bogusz@chalasr@ro0NL@fabpot@nicolas-grekas@mbabker@carsonbot

[8]ページ先頭

©2009-2025 Movatter.jp