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] Add broader support for command "help" definition#59473

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
chalasr merged 1 commit intosymfony:7.3fromyceruto:invokable_command_help
Jan 20, 2025

Conversation

yceruto
Copy link
Member

@ycerutoyceruto commentedJan 10, 2025•
edited
Loading

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?no
Issues-
LicenseMIT

Follow up#59340

Invokable and regular commands can now define the commandhelp content via the#[AsCommand] attribute.

This is particularly useful for invokable commands, as it avoids the need to extend theCommand class.

#[AsCommand(    name:'user:create',    description:'Create a new user',    help:<<<TXTThe <info>%command.name%</info> command generates a new user class for securityand updates your security.yaml file for it. It will also generate a user providerclass if your situation needs a custom class.<info>php %command.full_name% email</info>If the argument is missing, the command will ask for the class name interactively.TXT)]class CreateUserCommand{publicfunction__invoke(SymfonyStyle$io, #[Argument]string$email):int    {// ...    }}

Cheers!

valtzu, smnandre, and tacman reacted with thumbs up emoji
@ycerutoyceruto requested a review fromkbondJanuary 10, 2025 21:20
Copy link
Member

@GromNaNGromNaN left a comment

Choose a reason for hiding this comment

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

I'm worried that these texts will be unnecessarily duplicated when the container is compiled.

@ycerutoycerutoforce-pushed theinvokable_command_help branch 4 times, most recently frombb48d97 to92d279bCompareJanuary 10, 2025 22:33
@yceruto
Copy link
MemberAuthor

I'm worried that these texts will be unnecessarily duplicated when the container is compiled.

It's already the same case forname anddescription when configured via tag; I don't see a problem here.

Copy link
Member

@chalasrchalasr left a comment•
edited
Loading

Choose a reason for hiding this comment

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

That was on my list, thanks!

yceruto and OskarStark reacted with rocket emoji
@yceruto
Copy link
MemberAuthor

Just rebased and conflicts solved.

@kbond
Copy link
Member

This wasn't onmy radar but good call!

OskarStark and yceruto reacted with thumbs up emoji

Copy link
Contributor

@OskarStarkOskarStark left a comment

Choose a reason for hiding this comment

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

Like it 🖖

yceruto reacted with heart emoji
Copy link
Member

@GromNaNGromNaN left a comment

Choose a reason for hiding this comment

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

I'm worried that these texts will be unnecessarily duplicated when the container is compiled.

It's already the same case for name and description when configured via tag; I don't see a problem here.

The help text can be large, the duplication in the dumped container is only for commands using the attribute to declare it (twice in the XML and once in the PHP).

@@ -100,6 +100,10 @@ public function __construct(?string $name = null)
$this->setDescription(static::getDefaultDescription() ?? '');
}

if ('' === $this->help && $attributes = (new \ReflectionClass(static::class))->getAttributes(AsCommand::class)) {
$this->setHelp($attributes[0]->newInstance()->help ?? '');
}

Choose a reason for hiding this comment

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

we should either add addDefaultHelp or deprecate getDefaultName/Description to be consistent

Copy link
MemberAuthor

@ycerutoycerutoJan 13, 2025•
edited
Loading

Choose a reason for hiding this comment

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

The name and description static methods remain useful for lazy commands, but this approach is not necessary forhelp. Additionally, introducing a newgetDefaultHelp() method seems redundant since it's already possible to overridegetHelp() when customization is needed.

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

would you prefer adding a private static method forgetDefaultHelp() purely for the sake of consistency?

Copy link
Member

Choose a reason for hiding this comment

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

The name and description static methods remain useful for lazy commands

Are they really? I miss a case where it cannot be found in the tag and#[AsCommand]

Copy link
MemberAuthor

Choose a reason for hiding this comment

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

Hmm, I think I missed the fact that theAsCommand attribute is always auto-configured, so yes! they can now be deprecated. I'll make the changes in another PR, as it's not directly related to the feature proposed here.

chalasr and mtarld reacted with thumbs up emoji
@yceruto
Copy link
MemberAuthor

rebased and conflicts solved.

@chalasr
Copy link
Member

PR welcome to solve#59473 (comment) (deprecations) 🙏

yceruto reacted with thumbs up emoji

@chalasr
Copy link
Member

Thank you@yceruto.

@chalasrchalasr merged commite348b70 intosymfony:7.3Jan 20, 2025
8 of 10 checks passed
@ycerutoyceruto deleted the invokable_command_help branchJanuary 20, 2025 13:31
fabpot added a commit that referenced this pull requestJan 25, 2025
…faultDescription methods (yceruto)This PR was merged into the 7.3 branch.Discussion----------[Console] Deprecating Command getDefaultName and getDefaultDescription methods| Q             | A| ------------- | ---| Branch?       | 7.3| Bug fix?      | no| New feature?  | no| Deprecations? | yes| Issues        | -| License       | MITrelated discussion#59473 (comment)Commits-------b529726 Deprecating command getDefault* methods
@fabpotfabpot mentioned this pull requestMay 2, 2025
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@nicolas-grekasnicolas-grekasnicolas-grekas left review comments

@GromNaNGromNaNGromNaN approved these changes

@chalasrchalasrchalasr approved these changes

@kbondkbondkbond approved these changes

@OskarStarkOskarStarkOskarStark approved these changes

@alexandre-dauboisalexandre-dauboisalexandre-daubois approved these changes

@mtarldmtarldmtarld approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

9 participants
@yceruto@kbond@chalasr@nicolas-grekas@GromNaN@OskarStark@alexandre-daubois@mtarld@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp