Missing removeAllCommandsForTarget


#1

there is ApplicationCommand::registerAllCommandsForTarget but not the opposite removeAllCommandsForTarget, so i actually have to clear all commands and register again my subset everytime a target is destroyed.
 

thanx !


#2

Interesting, we will look into this! May be a good idea to add it.


#3

Hi there,

after a second look, I am not sure how useful such a hypothetical ApplicationCommandManager::removeAllCommandsForTarget would be?

The problem is (as far as I can tell right now):  At any time there can be several targets that can handle the same command. Which one gets invoked can depend on focus etc. There are functions like getFirstTargetForCommand and getNextTargetForCommand to iterate over the set of targets that may be interested.

For example: the JUCEApplication object by default handles the command StandardApplicationCommandIDs::quit. Your own target could possibly also handle the same command. However you certainly don't want to remove this command when your own target gets destroyed, do you?


#4

ok, so it's possible to only add commands and not remove them (unless you want to remove them all) ? btw should be up to the developer if call this or not...

i have a window (which is a target) that can be opened and closed independently of the main application windows, so the commands handled by that window should be cleaned up every time i close it: the pity is i have to remember which other targets are open at the time of the window close, and readd them back cause the only way to remove a command is to clear it...

i can do a for loop like this:

Array<CommandID> commands;
MyWindow.getAllCommands(commands);

for (int i = 0; i < commands.size(); i++)
{
    applicationCommandManager.removeCommand(commands[i]);
}

but i think this handy should be done in the command manager itself. it is just to improved the library and maintain code cleaniness, i have several places like that where i can "just do an external function" by my own instead of having juce helps where it can...

 

 


#5

Hi,

maybe I don't quite get it -- what exactly goes wrong if the command does *not* get removed? If there is no target that can handle it, then simply nothing will happen, right?


#6

...and if you do what you do in your example (the for loop), then this implies that you know there are no other targets interested in those commands. Please correct me if I am wrong, but as far as I understand this cannot be assumed in general (= the command manager does not know that this is the case).


#7

ok. if there is no need to remove commands then i'll keep adding them.

what's the purpose of ApplicationCommandManager::clearCommands then ? if it's not needed then it should be removed. i may be removing the JuceApplicationQuit this way-


#8

Good question.

Indeed, as far as I can tell, clearCommands clears *all* commands and key mappings.

Jules, are we missing something here?

 

(btw, "quit" seems to be a special case, because even if you remove the command and key mapping, for example on OSX, ⌘Q will still work  because this will cause the OS to call applicationShouldTerminate directly. so let's forget about "quit", it was a bad example)


#9

i'm sorry to bother, but if i don't remove a previously added command i'm getting this assert, and i'm not updating flags, i just try to call
registerAllCommandsForTarget again for a previously registered Target that is not calling removeAllCommandsForTarget (which doesn't exists yet):

void ApplicationCommandManager::registerCommand (const ApplicationCommandInfo& newCommand)
{
    ....

    // Trying to re-register the same command ID with different parameters can often indicate a typo.
    // This assertion is here because I've found it useful catching some mistakes, but it may also cause
    // false alarms if you're deliberately updating some flags for a command.
    jassert (newCommand.shortName == getCommandForID (newCommand.commandID)->shortName
              && newCommand.categoryName == getCommandForID (newCommand.commandID)->categoryName
              && newCommand.defaultKeypresses == getCommandForID (newCommand.commandID)->defaultKeypresses
              && (newCommand.flags & (ApplicationCommandInfo::wantsKeyUpDownCallbacks | ApplicationCommandInfo::hiddenFromKeyEditor | ApplicationCommandInfo::readOnlyInKeyEditor))
                   == (getCommandForID (newCommand.commandID)->flags & (ApplicationCommandInfo::wantsKeyUpDownCallbacks | ApplicationCommandInfo::hiddenFromKeyEditor | ApplicationCommandInfo::readOnlyInKeyEditor)));

Could this be sorted out somehow ?

EDIT: after a look i'm actually changing the command name in the second call to registerAllCommandForTarget:


void MyFabulousContentComponent::getCommandInfo (CommandID commandID, ApplicationCommandInfo& result)
{
    const String editingCategory ("Editing");

    switch (commandID)
    {
    case editUndo:
        result.setInfo ("Undo " + undoManager.getUndoDescription(), "", editingCategory, 0);
        result.addDefaultKeypress ('z', ModifierKeys::commandModifier);
        result.setActive(undoManager.canUndo());
        break;

    case editRedo:
        result.setInfo ("Redo " + undoManager.getRedoDescription(), "", editingCategory, 0);
        result.addDefaultKeypress ('z', ModifierKeys::commandModifier | ModifierKeys::shiftModifier);
        result.setActive(undoManager.canRedo());
        break;