8000 Allow custom scripts to be defined directly by its `Command`'s props/methods by igorsantos07 · Pull Request #12403 · composer/composer · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Allow custom scripts to be defined directly by its Command's props/methods #12403

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

igorsantos07
Copy li 8000 nk
Contributor

Builds on top of #11151; originally mentioned at #11134 (comment)

Long story short, this allows Command subclasses to have complete definitions inside them, instead of splitting the command and description between composer.json and the class contents.

I included this because, otherwise, someone may try to use setName() or setDescription() and get confused - while setHelp() can still only be defined inside the class.
It's also useful when defining a family of script classes, for writing dynamic descriptions, or pairing the description with the long help contents.


I'm just not sure about the change in Command\RunScriptCommand - was there a specific reason for it to filter only for ScriptAliasCommand, or could it be simply because there were no other subclasses of Command at that time?

Also: I guess we should now rename $dummy at Console\Application? In fact, when the name is defined inside the Command, the dummy value ends up being the array key 😂

I also added some commas, mostly suggested by PHPStorm - I do think they make sense, though. Lastly, I changed a sentence that had some unneeded text repetition.
Changing the `RunScriptCommand`'s `instanceof` to `Command` was redundant, so I removed it. Still not sure why it was there in the first place, so this might be a bad removal.
@igorsantos07
Copy link
Contributor Author

PHPStan is failing for a file I didn't edit, so I'm not fixing it with fear of conflicts 😅

@Seldaek Seldaek added this to the 2.9 milestone May 12, 2025
@Seldaek
Copy link
Member
Seldaek commented May 12, 2025

You can rebase if you want to fix the build. I'll need some more time to review the rest sorry.

@igorsantos07
Copy link
Contributor Author

No sorries. This is open source software! Take your time, sir haha
And thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0