Add @method tags to the AbstractController #3438

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
5 participants
@micheh
Contributor

micheh commented Jan 15, 2013

This pull request adds the convenience methods for the controller plugins to the AbstractController via the @method tag, which enables full code completion in the IDE.

@Ocramius

View changes

library/Zend/Mvc/Controller/AbstractController.php
@@ -23,10 +24,22 @@
use Zend\Stdlib\DispatchableInterface as Dispatchable;
use Zend\Stdlib\RequestInterface as Request;
use Zend\Stdlib\ResponseInterface as Response;
+use Zend\View\Model\ModelInterface;

This comment has been minimized.

Show comment Hide comment
@Ocramius

Ocramius Jan 16, 2013

Member

Instead of importing this unused class, use FQCN on all the @method usages

@Ocramius

Ocramius Jan 16, 2013

Member

Instead of importing this unused class, use FQCN on all the @method usages

This comment has been minimized.

Show comment Hide comment
@weierophinney

weierophinney Jan 16, 2013

Member

@Ocramius It's actually standard practice to import namespaces/classes only
referenced in docblocks as well; since php will do nothing with them unless
code requests them, they're effectively a no-op. Since phpdoc and most IDEs
expand them based on the current namespace or imports, we gain some
readability.
On Jan 15, 2013 6:43 PM, "Marco Pivetta" notifications@github.com wrote:

In library/Zend/Mvc/Controller/AbstractController.php:

@@ -23,10 +24,22 @@
use Zend\Stdlib\DispatchableInterface as Dispatchable;
use Zend\Stdlib\RequestInterface as Request;
use Zend\Stdlib\ResponseInterface as Response;
+use Zend\View\Model\ModelInterface;

Instead of importing this unused class, use FQCN on all the @method usages


Reply to this email directly or view it on GitHubhttps://github.com/zendframework/zf2/pull/3438/files#r2659471.

@weierophinney

weierophinney Jan 16, 2013

Member

@Ocramius It's actually standard practice to import namespaces/classes only
referenced in docblocks as well; since php will do nothing with them unless
code requests them, they're effectively a no-op. Since phpdoc and most IDEs
expand them based on the current namespace or imports, we gain some
readability.
On Jan 15, 2013 6:43 PM, "Marco Pivetta" notifications@github.com wrote:

In library/Zend/Mvc/Controller/AbstractController.php:

@@ -23,10 +24,22 @@
use Zend\Stdlib\DispatchableInterface as Dispatchable;
use Zend\Stdlib\RequestInterface as Request;
use Zend\Stdlib\ResponseInterface as Response;
+use Zend\View\Model\ModelInterface;

Instead of importing this unused class, use FQCN on all the @method usages


Reply to this email directly or view it on GitHubhttps://github.com/zendframework/zf2/pull/3438/files#r2659471.

This comment has been minimized.

Show comment Hide comment
@micheh

micheh Jan 16, 2013

Contributor

Yes, the imports were added for better readability. For some reason the @method tag does not work with line breaks, therefore the lines would be very long with the FQCN. For even shorter lines (without importing everything) I tried to use the relative namespaces for the controller plugins, but some IDEs don't support that and since this pull request is mainly intended to make the work in the IDEs more comfortable, I used the FQCN instead.

@micheh

micheh Jan 16, 2013

Contributor

Yes, the imports were added for better readability. For some reason the @method tag does not work with line breaks, therefore the lines would be very long with the FQCN. For even shorter lines (without importing everything) I tried to use the relative namespaces for the controller plugins, but some IDEs don't support that and since this pull request is mainly intended to make the work in the IDEs more comfortable, I used the FQCN instead.

This comment has been minimized.

Show comment Hide comment
@Ocramius

Ocramius Jan 16, 2013

Member

Gotcha

@Ocramius

Ocramius Jan 16, 2013

Member

Gotcha

@weierophinney

View changes

library/Zend/Mvc/Controller/AbstractController.php
/**
* Abstract controller
*
+ * Convenience methods for pre-built plugins (@see __call):
+ * @method ModelInterface acceptableViewModelSelector(array $matchAgainst = null, bool $returnDefault = true, AbstractFieldValuePart $resultReference = null)
+ * @method \Zend\Mvc\Controller\Plugin\FlashMessenger flashMessenger()

This comment has been minimized.

Show comment Hide comment
@weierophinney

weierophinney Jan 21, 2013

Member

Couldn't we use "Plugin\FlashMessenger" here, since it resolves relative to the current namespace? (same with other plugins)

Can you test and update indicating whether or not that works?

@weierophinney

weierophinney Jan 21, 2013

Member

Couldn't we use "Plugin\FlashMessenger" here, since it resolves relative to the current namespace? (same with other plugins)

Can you test and update indicating whether or not that works?

This comment has been minimized.

Show comment Hide comment
@micheh

micheh Jan 21, 2013

Contributor

As commented above I tried that. It works with Netbeans, but PHPStorm can't resolve this (seems to be a bug of PHPStorm, as it can resolve relative namespaces in other tags).

@micheh

micheh Jan 21, 2013

Contributor

As commented above I tried that. It works with Netbeans, but PHPStorm can't resolve this (seems to be a bug of PHPStorm, as it can resolve relative namespaces in other tags).

This comment has been minimized.

Show comment Hide comment
@weierophinney

weierophinney Jan 21, 2013

Member

@micheh I'd argue we either use FQCN in all cases, or relative to imports/namespace, but not a mix of both. Since this is primarily for IDE hinting, and we know FQCN works in all cases without issue, let's go with that for now.

@weierophinney

weierophinney Jan 21, 2013

Member

@micheh I'd argue we either use FQCN in all cases, or relative to imports/namespace, but not a mix of both. Since this is primarily for IDE hinting, and we know FQCN works in all cases without issue, let's go with that for now.

@ghost ghost assigned weierophinney Jan 21, 2013

@weierophinney

This comment has been minimized.

Show comment Hide comment
@weierophinney

weierophinney Jan 21, 2013

Member

Also, don't worry about the unit test errors -- those have been corrected on current master and develop branches.

Member

weierophinney commented Jan 21, 2013

Also, don't worry about the unit test errors -- those have been corrected on current master and develop branches.

@micheh

This comment has been minimized.

Show comment Hide comment
@micheh

micheh Jan 22, 2013

Contributor

@weierophinney Ok, updated the commit to use the FQCN in all cases.

Contributor

micheh commented Jan 22, 2013

@weierophinney Ok, updated the commit to use the FQCN in all cases.

@radnan

This comment has been minimized.

Show comment Hide comment
@radnan

radnan Jan 23, 2013

Contributor

You should add fileprg into the mix. Also maybe add a short description after the plugins that implement __invoke to hint what happens when the arguments aren't null, like:

/**
 * @method Params|mixed params(string $param = null, mixed $default = null) Get the params plugin. Grabs from route match if $param is specified.
 */
Contributor

radnan commented Jan 23, 2013

You should add fileprg into the mix. Also maybe add a short description after the plugins that implement __invoke to hint what happens when the arguments aren't null, like:

/**
 * @method Params|mixed params(string $param = null, mixed $default = null) Get the params plugin. Grabs from route match if $param is specified.
 */
@micheh

This comment has been minimized.

Show comment Hide comment
@micheh

micheh Jan 24, 2013

Contributor

This is a good idea, I'll add the plugins introduced in 2.1.
Adding a short description is somewhat problematic, as no line breaks are allowed. One line is already over 210 characters long...

Contributor

micheh commented Jan 24, 2013

This is a good idea, I'll add the plugins introduced in 2.1.
Adding a short description is somewhat problematic, as no line breaks are allowed. One line is already over 210 characters long...

@weierophinney

This comment has been minimized.

Show comment Hide comment
@weierophinney

weierophinney Jan 25, 2013

Member

Merged to develop branch for release with 2.1.0

Member

weierophinney commented Jan 25, 2013

Merged to develop branch for release with 2.1.0

@apoca

This comment has been minimized.

Show comment Hide comment
@apoca

apoca Jul 13, 2015

I'm using phpstorm and code completion don't work for ZF2 framework... How can I solve this!?

apoca commented Jul 13, 2015

I'm using phpstorm and code completion don't work for ZF2 framework... How can I solve this!?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment