Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Provide a usefull error when ask*() is not used wihtin a task() #1083

Merged
merged 3 commits into from
Mar 11, 2017
Merged

Provide a usefull error when ask*() is not used wihtin a task() #1083

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e802bff
Provide a usefull error when ask*() is not used wihtin a task()
staabm Mar 10, 2017
f31a84a
Update CHANGELOG.md
staabm Mar 10, 2017
4b9eb3b
Moved require'd() handling into Context.
staabm Mar 10, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
- Add a way to retrieve a defined task [#1008](https://github.com/deployphp/deployer/pull/1008)
- Add support for configFile in the NativeSsh implementation [#979](https://github.com/deployphp/deployer/pull/979)
- Add `--no-hooks` option for running commands without `before()` and `after()` [#1061](https://github.com/deployphp/deployer/pull/1061)
- Added a usefull error when ask*() is not used wihtin a task() [#1083](https://github.com/deployphp/deployer/pull/1083)

### Changed
- Parse hyphens in environment setting names [#1073](https://github.com/deployphp/deployer/pull/1074)
Expand Down
24 changes: 23 additions & 1 deletion src/functions.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@
use Deployer\Task\Context;
use Deployer\Task\GroupTask;
use Deployer\Type\Result;
use Deployer\Cluster\ClusterFactory;
use Deployer\Exception\Exception;
use Monolog\Logger;
use Symfony\Component\Console\Question\ChoiceQuestion;
use Symfony\Component\Console\Question\ConfirmationQuestion;
use Symfony\Component\Console\Question\Question;
use Symfony\Component\Finder\Finder;
use Symfony\Component\Process\Exception\ProcessFailedException;
use Symfony\Component\Process\Process;
use Deployer\Cluster\ClusterFactory;
use Symfony\Component\Console\Input\InputArgument;
use Symfony\Component\Console\Input\InputInterface;
use Symfony\Component\Console\Input\InputOption;
Expand Down Expand Up @@ -560,6 +561,8 @@ function has($name)
*/
function ask($message, $default = null, $suggestedChoices = null)
{
requiresTaskContext(__FUNCTION__);

if (($suggestedChoices !== null) && (empty($suggestedChoices))) {
throw new \InvalidArgumentException('Suggested choices should not be empty');
}
Expand Down Expand Up @@ -591,6 +594,8 @@ function ask($message, $default = null, $suggestedChoices = null)
*/
function askChoice($message, array $availableChoices, $default = null, $multiselect = false)
{
requiresTaskContext(__FUNCTION__);

if (empty($availableChoices)) {
throw new \InvalidArgumentException('Available choices should not be empty');
}
Expand Down Expand Up @@ -624,6 +629,8 @@ function askChoice($message, array $availableChoices, $default = null, $multisel
*/
function askConfirmation($message, $default = false)
{
requiresTaskContext(__FUNCTION__);

if (isQuiet()) {
return $default;
}
Expand All @@ -645,6 +652,8 @@ function askConfirmation($message, $default = false)
*/
function askHiddenResponse($message)
{
requiresTaskContext(__FUNCTION__);

if (isQuiet()) {
return '';
}
Expand Down Expand Up @@ -742,3 +751,16 @@ function parse($value)
{
return Context::get()->getEnvironment()->parse($value);
}

/**
* Throws a Exception when not called within a task-context.
*
* This method provides a usefull error to the end-user to make him/her aware
* to use a function in the required task-context.
*/
function requiresTaskContext($callerName)
Copy link
Member

Choose a reason for hiding this comment

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

I think better to create method in Context:

Context::required();

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also thought about that option. will change it.

Copy link
Contributor Author

@staabm staabm Mar 10, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

now I know again why I decided against a method in Context ;-):

the message and logic here is rather task specific and Context class atm is rather generic.

    /**
     * Throws a Exception when not called within a task-context.
     *
     * This method provides a usefull error to the end-user to make him/her aware
     * to use a function in the required task-context.
     *
     * @throws Exception
     * @return Context
     */
    public static function required($callerName)
    {
        $ctx = self::get();
        if (!$ctx) {
            throw new Exception("'$callerName' can only be used within a 'task()'-function!");
        }
        return $ctx;
    }

do you still think we should move it to Context, I am fine with any solution you will accept.

Copy link
Member

Choose a reason for hiding this comment

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

I think its better.

{
if (!Context::get()) {
throw new Exception("'$callerName' can only be used within a 'task()'-function!");
}
}