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.

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

bin/console ckeditor:install --quiet doesnt work after updating Symfony to v3.4.3 #335

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

Dukecz
Copy link

@Dukecz Dukecz commented Jan 18, 2018

After I have updated symfony from 3.4.2 to 3.4.3 the console command bin/console ckeditor:install --quiet stopped working, exitting with error code 1:

In QuestionHelper.php line 53:

Notice: Undefined index: Drop the directory & reinstall CKEditor

ckeditor:install [--release [RELEASE]] [--tag [TAG]] [--clear [CLEAR]] [--exclude [EXCLUDE]] [-h|--help] [-q|--quiet] [-v|vv|vvv|--verbose] [-V|--version] [--ansi] [--no-ansi] [-n|--no-interaction] [-e|--env ENV] [--no-debug] [--] []

Script bin/console ckeditor:install --quiet handling the symfony-scripts event returned with error code 1

@robertfausk
Copy link

Same with update symfony/symfony from v3.3.14 to v3.3.15 and composer script Ivory\\CKEditorBundle\\Composer\\CKEditorScriptHandler::install

@faffyman
Copy link

faffyman commented Jan 17, 2018

Looks like it's related to $question in the install command::createNotifier

it's calling the choice method passing an array as the $question, but the symfony ChoiceQuestion can only accept a string as the first parameter, not an array.

 switch ($type) {
                case CKEditorInstaller::NOTIFY_CLEAR:
                    $result = $this->choice(
                        [
                            sprintf('CKEditor is already installed in "%s"...', $data),
                            '',
                            'What do you want to do?',
                        ],

The choice method would need to be sure to always transform that array into a string

 /**
     * @param string|string[] $question  <--  this is an array
     * @param string[]        $choices
     * @param string          $default
     * @param InputInterface  $input
     * @param OutputInterface $output
     *
     * @return string|null
     */
    private function choice($question, array $choices, $default, InputInterface $input, OutputInterface $output)
    {
        $helper = new QuestionHelper();
        $question = is_array($question) ? implode(PHP_EOL,  $question) : $question; // SUGGESTION
        $result = $helper->ask($input, $output, new ChoiceQuestion(
            $question,
            $choices,
            $choices[$default]
        ));
        $output->writeln('');
        return $result;
    }

Actual Error

PHP Fatal error:  Uncaught Symfony\Component\Debug\Exception\FatalThrowableError: Type error: Argument 1 passed to Symfony\Component\Console\Question\ChoiceQuestion::__construct() must be of the type string, array given, 
called in /vendor/egeloen/ckeditor-bundle/Command/CKEditorInstallerCommand.php on line 326 

@faffyman
Copy link

faffyman commented Jan 17, 2018

Actually, looking again if this change was made to the CKEditorInstallerCommand::choice() method - it installs ok for 3.4.3+

FROM

$result = $helper->ask($input, $output, new ChoiceQuestion(
            $question,
            $choices,
            $choices[$default]
        ));

TO

$result = $helper->ask($input, $output, new ChoiceQuestion(
            $question,
            $choices,
            $default // <-- just the default don't refernce the array of choices anymore
        ));

Since the changes to the Symfony Helper\QuestionHelper return the value and not the index see symfony/console@d6f3e07#diff-48f20b7ff8c43c4083a7ee0458c45f45

@qdequippe
Copy link

Any updates on this PR? my project is blocked by this, is there a workaround?

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.

4 participants