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

[5.7] Fixes broken `setHidden` method #27005

Merged
merged 2 commits into from Dec 31, 2018

Conversation

Projects
None yet
4 participants
@nunomaduro
Copy link
Member

nunomaduro commented Dec 30, 2018

Not sure if is related to #25342 or #26781, but the setHidden method no longer works as expected.

Issue: When the developer calls the method setHidden that exists the on the Symfony\Command::class, the property $hidden that gets changed it's the private one on the Symfony\Command::class instead of the protected one on the Laravel\Command::class:

$command->setHidden(true);
$command->isHidden(); // returns false

Solution: Add the method setHidden on the Laravel\Command::class to make sure that both attributes have the same value:

    /**
     * {@inheritdoc}
     */
    public function setHidden($hidden)
    {
        parent::setHidden($this->hidden = $hidden);

        return $this;
    }

Result:

$command->setHidden(true);
$command->isHidden(); // returns true

Note: We should consider add some tests on this new hidden related methods. Please close the Pull Request if you want me to add tests on this.

@TBlindaruk

This comment has been minimized.

Copy link
Member

TBlindaruk commented Dec 30, 2018

@nunomaduro , Could you create tests for covered this case?

@taylorotwell taylorotwell merged commit 2106db9 into laravel:5.7 Dec 31, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/styleci/pr The analysis has passed
Details

@nunomaduro nunomaduro deleted the nunomaduro:patch-2 branch Dec 31, 2018

@Broutard

This comment has been minimized.

Copy link
Contributor

Broutard commented Jan 2, 2019

Not work for me.
Commands like "schedule:run" still be shown.

It seems it's due to this PR : #26781
If I comment the isHidden() method in Illuminate\Console\Command it works.

@nunomaduro

This comment has been minimized.

Copy link
Member

nunomaduro commented Jan 2, 2019

@Broutard schedule:run is not hidden on Laravel.

@Broutard

This comment has been minimized.

Copy link
Contributor

Broutard commented Jan 2, 2019

But it is hidden in my config

'hidden' => [
        NunoMaduro\LaravelConsoleSummary\SummaryCommand::class,
        Symfony\Component\Console\Command\HelpCommand::class,
        Illuminate\Console\Scheduling\ScheduleRunCommand::class,
        Illuminate\Console\Scheduling\ScheduleFinishCommand::class,
        Illuminate\Foundation\Console\VendorPublishCommand::class,
    ],

In fact, the isHidden() method added in #26781 reference the Illuminate\Console\Command protected $hidden variable but when you use setHidden() this is the Symfony\Component\Console\Command\Command method which reference his private $hidden.

@nunomaduro

This comment has been minimized.

Copy link
Member

nunomaduro commented Jan 2, 2019

@Broutard Seems that you are talking about Laravel Zero. The issue will be fixed when @taylorotwell release the version v5.7.20 of the illuminate/console component.

@Broutard

This comment has been minimized.

Copy link
Contributor

Broutard commented Jan 2, 2019

Oh, ok sorry I thought it was already done.

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