-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Deprecate Annotated Commands in favor of native Symfony Console commands #6135
base: 13.x
Are you sure you want to change the base?
Changes from 26 commits
2f63ffc
ec1f512
47bf610
a201b1e
756d384
000c215
2eeb287
ff65ece
f2f179d
41a8aa6
141fa70
2eea0de
68cad77
b3fbd3a
a194c59
4a9f239
e78b50a
2f42025
ac5ad80
0239f0f
8b16ff4
7d6e8ea
9847b61
b9e11f3
6216ad6
8d7b2e4
1a795af
680da18
12e74f8
74becdc
6c899f4
74b697c
5b55f98
ebd367a
34afdc7
f923a80
2e75912
a2b44fd
b4ed0bf
58c2ddd
c1056ed
8f4f573
fdfaf0f
3712e5d
f0ffbd9
955511e
aa21ad9
8ef8994
b9817b9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,28 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Drush\Attributes; | ||
|
||
use Attribute; | ||
use Consolidation\AnnotatedCommand\Parser\CommandInfo; | ||
use Consolidation\OutputFormatters\Options\FormatterOptions; | ||
|
||
#[Attribute(Attribute::TARGET_METHOD)] | ||
class FieldLabels extends \Consolidation\AnnotatedCommand\Attributes\FieldLabels | ||
#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_CLASS)] | ||
class FieldLabels | ||
{ | ||
const KEY = FormatterOptions::FIELD_LABELS; | ||
|
||
/** | ||
* @param $labels | ||
* An associative array of field names and labels for display. | ||
*/ | ||
public function __construct( | ||
public array $labels | ||
) { | ||
} | ||
|
||
public static function handle(\ReflectionAttribute $attribute, CommandInfo $commandInfo) | ||
{ | ||
$args = $attribute->getArguments(); | ||
$commandInfo->addAnnotation('field-labels', $args['labels']); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,27 @@ | ||
<?php | ||
|
||
declare(strict_types=1); | ||
|
||
namespace Drush\Attributes; | ||
|
||
use Attribute; | ||
use Consolidation\AnnotatedCommand\Parser\CommandInfo; | ||
|
||
#[Attribute(Attribute::TARGET_METHOD)] | ||
class FilterDefaultField extends \Consolidation\AnnotatedCommand\Attributes\FilterDefaultField | ||
#[Attribute(Attribute::TARGET_METHOD | Attribute::TARGET_CLASS)] | ||
class FilterDefaultField | ||
{ | ||
const KEY = 'filter-default-field'; | ||
|
||
/** | ||
* @param $field | ||
* A field name to filter on by default. | ||
*/ | ||
public function __construct( | ||
public string $field | ||
) { | ||
} | ||
|
||
public static function handle(\ReflectionAttribute $attribute, CommandInfo $commandInfo) | ||
{ | ||
$args = $attribute->getArguments(); | ||
$commandInfo->addAnnotation('filter-default-field', $args['field']); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here and elsewhere, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just pushed a fix with a couple tweaks. Depends on another PR in output-formatters consolidation/output-formatters#113 |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<?php | ||
|
||
namespace Drush\Commands; | ||
|
||
use Symfony\Component\Console\Command\Command; | ||
use Symfony\Component\Console\Input\InputOption; | ||
|
||
class OptionSets | ||
{ | ||
public static function sql(Command $command) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe the static method technique of holding option sets would sit better if it lived in some "sql" namespace, rather than putting all option set domains in the same class. (This would be more obvious if there were more than one here.) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. True. The benefit of putting multiple methods on a single OptionSet class is that the optionsets are more discoverable. The class analogous to our existing https://github.com/drush-ops/drush/blob/13.x/src/Commands/OptionsCommands.php |
||
{ | ||
$command->addOption('database', '', InputOption::VALUE_REQUIRED, 'The DB connection key if using multiple connections in settings.php.', 'default'); | ||
$command->addOption('db-url', '', InputOption::VALUE_REQUIRED, 'A Drupal 6 style database URL. For example <info>mysql://root:pass@localhost:port/dbname</info>'); | ||
$command->addOption('target', '', InputOption::VALUE_REQUIRED, 'The name of a target within the specified database connection.', 'default'); | ||
$command->addOption('show-passwords', '', InputOption::VALUE_NONE, 'Show password on the CLI. Useful for debugging.'); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
<?php | ||
|
||
namespace Drush\Commands; | ||
|
||
class Validators | ||
{ | ||
public static function entityLoad(array $ids, string $entityType): void | ||
{ | ||
// @todo It is a burden for the caller to inject the entityTypeManager. | ||
$loaded = \Drupal::entityTypeManager()->getStorage($entityType)->loadMultiple($ids); | ||
if ($missing = array_diff($ids, array_keys($loaded))) { | ||
$msg = dt('Unable to load the !type: !str', ['!type' => $entityType, '!str' => implode(', ', $missing)]); | ||
throw new \Exception($msg); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not super excited to do this outside of tests. Is this for some sort of "alter" hook? If we want to do this, maybe we should, during preflight, add commands to some other list and provide a specific hook to alter that before adding to the application. That might be complicated, since today we add in two steps (commands from modules being added later).
Alternately, instead of removing a command, maybe we could change its name, remove its aliases and hide it?
I'm not sure why I think that setAccissible is worse than that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think thats more more easily done once we use a container to store commands and maybe use a CommandLoader. Yes, this is just used by Woot today - we can remove it if it offends the eyes.