php-school / cli-menu

🖥 Build beautiful PHP CLI menus. Simple yet Powerful. Expressive DSL.
http://www.phpschool.io
MIT License
1.94k stars 106 forks source link

Don't call $callback->bindTo() #191

Closed jtreminio closed 4 years ago

jtreminio commented 4 years ago

Both CliMenuBuilder::addSplitItem() and SplitItemBuilder::addSubMenu() call $callback = $callback->bindTo($builder);

This overrides $this within the context of the callee, forcing the use of $self = $this or similar:

    protected function execute(InputInterface $input, OutputInterface $output): int
    {
        $self = $this;

        $menu = (new CliMenuBuilder)
            ->setBackgroundColour('black')
            ->setForegroundColour('green')
            ->setTitle('Create a New Project')
            ->addLineBreak()
            ->addStaticItem('Select Language')
            ->addSubMenu('PHP', function (CliMenuBuilder $b) use ($self) {
                $self->newPhpProject($b);
            })
            ->addItem('Javascript', function (CliMenu $menu) {})
            ->addItem('Ruby', function (CliMenu $menu) {})
            ->addItem('Python', function (CliMenu $menu) {})
            ->addItem('Go', function (CliMenu $menu) {})
            ->addLineBreak()
            ->build();

        $menu->open();

        return 0;
    }

    protected function newPhpProject(CliMenuBuilder $builder)
    {
        $symfonySubMenu = function (CliMenuBuilder $b) {
            $symfony = new Project\Symfony($b);
            $symfony->init();
        };

        $builder->setTitle('Choose PHP Project Type')
            ->addSubMenu('Symfony', $symfonySubMenu)
            ->addItem('Laravel', function (CliMenu $menu) {})
            ->addItem('Wordpress', function (CliMenu $menu) {})
            ->addItem('Drupal', function (CliMenu $menu) {})
            ->addItem('Pure', function (CliMenu $menu) {})
            ->build()
        ;
    }

Within the anonymous function, $this refers to the same $b:

            ->addSubMenu('PHP', function (CliMenuBuilder $b) use ($self) {
                $self->newPhpProject($b);
            })

But CliMenuBuilder::addSplitItem() and SplitItemBuilder::addSubMenu() already pass the builder object to the callback, $callback($builder); (also seen above in function (CliMenuBuilder $b)).

Removing $callback = $callback->bindTo($builder); lets us use $this as expected:

            ->addSubMenu('PHP', function (CliMenuBuilder $b) {
                $this->newPhpProject($b);
            })
AydinHassan commented 4 years ago

Yes this does seem rather counterintuitive in that sense. It was specifically requested and implemented in #155 though so I'd like to hear @Lynesth opinion on this. It makes sense from my point of view to make this change though.