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

Section method only in ConsoleOutputInterface#14550

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
wouterj merged 1 commit intosymfony:4.4fromybenhssaien:patch-13
Nov 22, 2020

Conversation

ybenhssaien
Copy link
Contributor

Sinceexcecute() accepts an instanceofOutputInterface which doesn't contain the methodsection we have to add a check onConsoleOutputInterface

Copy link

@MattsMatts left a comment

Choose a reason for hiding this comment

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

Hey@ybenhssaien

After this change LGTM 👍

ybenhssaien reacted with thumbs up emoji
@wouterj
Copy link
Member

Hi@ybenhssaien! Thanks for your PR :)

However, I do not agree completely with the changes. Personally, I don't think it's possible to ever get a non-ConsoleOutputInterface output as argument toexecute(), unless you're calling it yourself. In this case, I expect people to have a deeper understanding than the normal documentation audience.

That aside, I think the code you've posted does not work. Later on in the example,$section1->overwrite()/$section1->clear() is used, which only exists on theConsoleSectionOutput class and not theOutputInterface. If you think it's needed to add something here, I would go for something likeif (!$output instanceof ConsoleOutputInterface) { throw new \LogicException('This code should not be reached, only console output is supported.') } at the start of the command.

ybenhssaien and Matts reacted with thumbs up emoji

@ybenhssaien
Copy link
ContributorAuthor

ybenhssaien commentedNov 17, 2020•
edited
Loading

Thanks@wouterj for your feedback, I agree with you about throwing exception rather than my suggestion, and clearly people should receive an instance ofConsoleOutputInterface when creating commands inside a Symfony app, may some enterprises internal frameworks which usingConsole components outside Symfony package depend only on this doc to customize their commands, so I think it is better to mention it (I like the idea of throwing exception) => I will update my PR

This is a test withBufferedOutput (usingStreamOutput too since both of them extendOutput)
image

@wouterjwouterj changed the base branch from5.1 to4.4November 22, 2020 11:48
wouterj added a commit that referenced this pull requestNov 22, 2020
@wouterjwouterj merged commit6f13815 intosymfony:4.4Nov 22, 2020
@wouterj
Copy link
Member

Thank you@ybenhssaien! I've merged this one in 4.4 (the lowest still maintained version) and made the nr of lines a bit smaller:73f02f1 (the focus of this example is the usage of console sections, so I kept the exception as small as possible to avoid too much distraction)

wouterj added a commit that referenced this pull requestNov 23, 2020
* 4.4:  Enhancement: Streamline workflow  GitHub Actions: use docker container for CI build  Bus restricting with Interfaces  Security: add example code which Maker Bundle generated  [#14550] Reduce nr of lines used for exception  Section method only in ConsoleOutputInterface
wouterj added a commit that referenced this pull requestNov 23, 2020
* 5.1:  Enhancement: Streamline workflow  GitHub Actions: use docker container for CI build  Bus restricting with Interfaces  Security: add example code which Maker Bundle generated  [#14550] Reduce nr of lines used for exception  Section method only in ConsoleOutputInterface
wouterj added a commit that referenced this pull requestNov 23, 2020
* 5.2:  Enhancement: Streamline workflow  GitHub Actions: use docker container for CI build  Bus restricting with Interfaces  Security: add example code which Maker Bundle generated  [#14550] Reduce nr of lines used for exception  Section method only in ConsoleOutputInterface
OskarStark added a commit to OskarStark/symfony-docs that referenced this pull requestNov 25, 2020
* 5.1: (31 commits)  Fix package name for OvhCloud notifier  Fix wrong key name  Enhancement: Streamline workflow  GitHub Actions: use docker container for CI build  Bus restricting with Interfaces  Security: add example code which Maker Bundle generated  [symfony#14550] Reduce nr of lines used for exception  Section method only in ConsoleOutputInterface  Added a new troubleshooting section in the maintainer guide  Removed duplicated line  Complete documentation about mailer integration  [symfony#14083] Revert mailer.headers option in 4.4 - 5.1  Removed 4.3 versionadded directive  [symfony#14565] Minor formatting fixes  Updated getParent() description with the why/when (thanks javier!)  [Form] Added explicit `getParent()` call in types inheritance mechanism  Added PHP types to the DI related articles  Added PHP types to the Form related articles  Added PHP types to the Doctrine related articles  Tweak  ...
This was referencedOct 27, 2022
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@MattsMattsMatts requested changes

@HeahDudeHeahDudeAwaiting requested review from HeahDude

@xabbuhxabbuhAwaiting requested review from xabbuh

Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

4 participants
@ybenhssaien@wouterj@Matts@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp