-
-
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
TableEmptyMessage attribute #5641
base: 12.x
Are you sure you want to change the base?
Conversation
use Attribute; | ||
|
||
#[Attribute(Attribute::TARGET_METHOD)] | ||
class TableEmptyMessage extends \Consolidation\AnnotatedCommand\Attributes\TableEmptyMessage |
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.
It seems really unfortunate that we need to duplicate all of the attributes in the \Consolidation\AnnotatedCommand\Attributes namespace here.
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've accepted it in favor of having one namespace to autocomplete attribute names in an IDE. Custom code is welcome to use AC attributes directly as https://github.com/drush-ops/drush/blob/12.x/tests/fixtures/lib/Drush/Commands/ExampleAttributesDrushCommands.php#L80-L95 does.
be3914b
to
b1f3c31
Compare
@@ -235,6 +235,7 @@ public function validateUninstall(CommandData $commandData): void | |||
])] | |||
#[CLI\DefaultTableFields(fields: ['package', 'display_name', 'status', 'version'])] | |||
#[CLI\FilterDefaultField(field: 'display_name')] | |||
#[CLI\TableEmptyMessage(message: 'There are no items to list')] |
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.
Run drush pm:list --filter=package=Core
to see a list of core modules; run drush pm:list --filter=package=Foo
to see the empty message.
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.
LGTM. Thanks.
use Attribute; | ||
|
||
#[Attribute(Attribute::TARGET_METHOD)] | ||
class TableEmptyMessage extends \Consolidation\AnnotatedCommand\Attributes\TableEmptyMessage |
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've accepted it in favor of having one namespace to autocomplete attribute names in an IDE. Custom code is welcome to use AC attributes directly as https://github.com/drush-ops/drush/blob/12.x/tests/fixtures/lib/Drush/Commands/ExampleAttributesDrushCommands.php#L80-L95 does.
Looks like consolidation/annotated-command#303 needs to get merged still @greg-1-anderson |
If this looks okay, then we can merge consolidation/annotated-command#303 and consolidation/output-formatters#108 and make releases.
Feel fee to comment on those PRs if you have any comments on naming things, or any other detail.