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

Custom and auto mappings can't override default #246

Closed InterLinked1 closed 3 years ago

InterLinked1 commented 3 years ago

The default control mappings for keys includes "M" and "L" in the list, which prevents their being used for other purposes.

For instance, neither of the following works:

$menu->addCustomControlMapping('M', $myCallback);

->addItem('[M]ember List', $itemCallable2)

Rather than throwing an exception, this should allow the default behavior of that key to be overridden (or removed). The L/R functionality of the L and M keys is already redundant and for a menu item like "Member List", there is no obvious choice of character that will work for the menu, particularly one that is large.

AydinHassan commented 3 years ago

hi @InterLinked1 you can use disableDefaultControlMappings or setDefaultControlMappings on CliMenu to remove the defaults and/or set your own.

InterLinked1 commented 3 years ago

hi @InterLinked1 you can use disableDefaultControlMappings or setDefaultControlMappings on CliMenu to remove the defaults and/or set your own.

Okay, thanks, sorry, I could not find that in the documentation before.

It does seem though that per 244, it is not possible to create a menu that builds at runtime rather than all at once? https://github.com/php-school/cli-menu/issues/244

For instance, if trying to set something up with lots of SQL to interact with a database, all the SQL tries to execute immediately when the menu is built, rather than once a menu item is accessed. Just want to make sure I'm understanding this right? Thanks!

AydinHassan commented 3 years ago

No problem, it doesn't appear to be in the docs, but I kinda remembered something like it existed.

No that's not correct, menu item callables are only executed when the menu item is selected. That issue appears to be about performing an action when a sub menu (which is a specific type of menu item) is selected which is not possible.

InterLinked1 commented 3 years ago

No problem, it doesn't appear to be in the docs, but I kinda remembered something like it existed.

No that's not correct, menu item callables are only executed when the menu item is selected. That issue appears to be about performing an action when a sub menu (which is a specific type of menu item) is selected which is not possible.

I see, but certain things would require a sub-menu as opposed to the original menu, right, in which case it would not be possible to evaluate those later?

The examples I see use addItem to do something that modifies that menu rather than going into an entirely new (sub) menu, but it doesn't seem like a menu item could do what the sub menu does.

AydinHassan commented 3 years ago

Sorry I'm not following. Can you maybe show some example code?

InterLinked1 commented 3 years ago

Sorry I'm not following. Can you maybe show some example code?

Like, say I wanted to generate a sub-menu with all members:

   $stmt = $mysqli->prepare("SELECT id, full_name, city, state FROM users");
    $stmt->execute();
    $stmt->bind_result($id, $fn, $city, $state);
    while ($stmt->fetch()) {
        $b->addItem($fn, $itemCallable2);
    }
    $b->addItem('[G]o back', new GoBackAction);
    $b->enableAutoShortcuts();

If I put that in a sub-menu, then this gets executed at build time, rather than when the submenu is accessed (if ever). If I had 20 of these, as soon as the menu is opened, all 20 queries fire at once. These DB queries are going across the Internet, so I don't want SQL queries being fired unless they are being used immediately.

However, I want to actually build a menu where one could choose the right row from the result and then, say, edit the data in that row, and so forth. So that seems to require addSubMenu, not just addItem.

AydinHassan commented 3 years ago

Thanks for the example. Yes unfortunately that is not supported.

InterLinked1 commented 3 years ago

To follow up on this, is there any way to use the menu to retrieve the value when an option is selected? Been trying to play around with that and struggling to find a way to do that. My thinking is perhaps I can just return to the parent PHP code and then spawn another menu and maybe work around this that way.

For instance, with the basic example: $itemCallable = function (CliMenu $menu) { echo $menu->getSelectedItem()->getText(); };

$menu = (new CliMenuBuilder) ->setTitle('Basic CLI Menu') ->addItem('First Item', $itemCallable) ->addItem('Second Item', $itemCallable) ->addItem('Third Item', $itemCallable) ->addLineBreak('-') ->build();

$menu->open(); echo "continue";

Selecting a menu option just echos that forever. I'm trying to see if possibly selecting that could be captured into a PHP variable (the text of the menu item), and then "continue" would execute immediately, that is selecting an option closes the menu.

Or is this basically once you launch the menu, you can't return from it without explicitly exiting? Essentially, it seems php-ncurses is dead and I need to make a CLI terminal "GUI" of sorts, so need to be creative ;)

AydinHassan commented 3 years ago

You can do this a bunch of ways:

AydinHassan commented 3 years ago

eg

<?php

use PhpSchool\CliMenu\CliMenu;
use PhpSchool\CliMenu\Builder\CliMenuBuilder;

require_once(__DIR__ . '/../vendor/autoload.php');

$itemCallable = function (CliMenu $menu) {
    //echo $menu->getSelectedItem()->getText();
    $menu->close();
};

$menu = (new CliMenuBuilder)
    ->setTitle('Basic CLI Menu')
    ->addItem('First Item', $itemCallable)
    ->addItem('Second Item', $itemCallable)
    ->addItem('Third Item', $itemCallable)
    ->addLineBreak('-')
    ->build();

$menu->open();
echo $menu->getSelectedItem()->getText() . "\n";
InterLinked1 commented 3 years ago

Great, thank you!

This works well, though I've noticed some interesting behavior with the keyboard mappings when doing this. It seems that it will return whatever menu item was "selected" (like literally which one was highlighted), NOT which menu item was chosen to exit the menu. As an example:

$selectedItem = '';
function menuFlash($title) {
    $accessDenied = function (CliMenu $menu) {
        $flash = $menu->flash($title);
        $flash->getStyle()->setBg('88', 'green');
        $flash->display();
    };
}
function menu($title = '', array $items) {
    $exitItem = function (CliMenu $menu) {
        $menu->close();
    };
    $itemCallable = function(CliMenu $menu) {
        global $selectedItem;
        $selectedItem = $menu->getSelectedItem()->getText();
        $menu->close();
    };
    $menuItems = array();
    foreach ($items as $item) {
        $menuItems[] = [$item, $itemCallable];
    }
    $builder = new CliMenuBuilder;
    $builder->setTitle($title);
    $builder->enableAutoShortcuts();
    $builder->addItems($menuItems);
    $menu = $builder->build();
    $menu->addCustomControlMapping('x', $exitItem);
    $menu->open();
    return $menu->getSelectedItem()->getText();
}
echo menu('test', array('[T]est1', 'Se[c]ond'));
echo $selectedItem;

The return value of menu is always what's highlighted - usually incorrect. The value of $selectedItem is always correct. Is this by design? Would there be a "native" way to get the actual menu item chosen (rather than highlighted at the time an option was chosen), or should I just use global variables like this as a workaround?

AydinHassan commented 3 years ago

Yeah you're right that is not what you want, it is by design and is how the internals work.

You don't need to use a global though, you can just use a reference in your callable.

AydinHassan commented 3 years ago
$selected = null;
$itemCallable = function(CliMenu $menu) use (&$selected){
    $selected = $menu->getSelectedItem()->getText();
    $menu->close();
};
AydinHassan commented 3 years ago

or even like this if you wish:

<?php

use PhpSchool\CliMenu\CliMenu;
use PhpSchool\CliMenu\Builder\CliMenuBuilder;

require_once(__DIR__ . '/../vendor/autoload.php');

class Selected {
    public string $text;
}

$selected = new Selected;

$itemCallable = function(CliMenu $menu) use ($selected){
    $selected->text = $menu->getSelectedItem()->getText();
    $menu->close();
};

$menu = (new CliMenuBuilder)
    ->setTitle('Basic CLI Menu')
    ->addItem('First Item', $itemCallable)
    ->addItem('Second Item', $itemCallable)
    ->addItem('Third Item', $itemCallable)
    ->addLineBreak('-')
    ->build();

$menu->open();

var_dump($selected);

Bonus example:

use PhpSchool\CliMenu\CliMenu;
use PhpSchool\CliMenu\Builder\CliMenuBuilder;

require_once(__DIR__ . '/../vendor/autoload.php');

class ItemCallable {
    public string $text;

    public function __invoke(CliMenu $menu) {
        $this->text = $menu->getSelectedItem()->getText();
        $menu->close();
    }
}

$itemCallable = new ItemCallable;

$menu = (new CliMenuBuilder)
    ->setTitle('Basic CLI Menu')
    ->addItem('First Item', $itemCallable)
    ->addItem('Second Item', $itemCallable)
    ->addItem('Third Item', $itemCallable)
    ->addLineBreak('-')
    ->build();

$menu->open();

var_dump($itemCallable);

We are not providing these utils because it's just PHP code, you can do this however you want and it's not really necessary for us to impose restrictions

InterLinked1 commented 3 years ago

Got it, yeah, saving it internally and returning it works well. I've played around with this method and it works really well! I can now build dynamic menus by simply calling a new menu for each "thing", and intersperse with other PHP as needed.

Ran into a slight issue with the auto shortcuts, though and I tried following the steps at the top of this post but there seems to be an issue with ordering:

enableAutoShortcuts acts on a CliMenuBuilder object. disableDefaultControlMappings acts on a CliMenu object.

Problem is, it seems that calling enableAutoShortcuts and then disableDefaultControlMappings doesn't work, because enableAutoShortcuts tries to map shortcuts that already exist, so PHP crashes.

The logical thing to do seems to be calling disableDefaultControlMappings before enableAutoShortcuts. Except, you can only call disableDefaultControlMappings on a built menu, and enableAutoShortcuts can only be invoked on a menu builder... so chicken and egg problem? Or have I missed something again?

I know, in theory, I could bind all the custom shortcuts myself, but that's not really feasible when the order can vary and the class already has all the functions to do this, but they're all private, so I can't use them directly.

AydinHassan commented 3 years ago

Sorry I don't have a solution for you that doesn't require internal changes. Feel free to send a PR, maybe moving the control mappings up in to the builder.

InterLinked1 commented 3 years ago

Sorry I don't have a solution for you that doesn't require internal changes. Feel free to send a PR, maybe moving the control mappings up in to the builder.

Yeah, it seemed like it would require code changes one way or another. I think an easy fix might be a way to empty this array of non-arrow options in the builder, before calling enableAutoShortcuts:

protected $defaultControlMappings = [
        '^P' => InputCharacter::UP,
        'k'  => InputCharacter::UP,
        '^K' => InputCharacter::DOWN,
        'j'  => InputCharacter::DOWN,
        "\r" => InputCharacter::ENTER,
        ' '  => InputCharacter::ENTER,
        'l'  => InputCharacter::LEFT,
        'm'  => InputCharacter::RIGHT,
    ];

Or alternately, an option to enableAutoShortcuts to have it replace rather than throw a fatal error. This might actually be better.

At this point, I think I will simply delete everything in this array that isn't an arrow key in the code source, the quickest and easiest fix for me, since I have no use for any of the letter bindings. However, I agree an option would be nice. Thanks for the help!

InterLinked1 commented 3 years ago

All right, back with another question! This has all been working really well for me, thank you again for this project!

An issue I've encountered is on unsupported terminal types, this class simply throws an exception and crashes. What that means in my environment is the whole program just disappears. What I would like to do is before trying to create a menu, use the isInteractive() public function to test if indeed we are supported, and if not, gracefully fall back to something that would be universally supported - e.g. just a simple list with prompting for a character input, something like that.

The way the function is set up though, it seems tied to creating a terminal and I am not seeing how I would easily do this before a menu is created. What would be the best way to do this? I also tried finding what isInteractive() is doing behind the scenes, but it I just see "return true;" which doesn't seem like that could be it.