Incorrect menu documentation

[Richienb]

Expected Behavior

The extension menu documentation in https://github.com/LLK/scratch-vm/blob/develop/docs/extensions.md#defining-a-menu should match with the jsdocs in https://github.com/LLK/scratch-vm/blob/develop/src/extension-support/extension-metadata.js#L39

Actual Behavior

Both sources describe different behaviour.

Steps to Reproduce

The first source outlined this:

{
    menus: {
        fooMenu: {
            items: ['a', 'b', 'c']
        }
    }
}

However, the second source suggests each value in menus should be a string, array of strings or array of {value: *, text: string}.

[ericrosenbaum]

As far as I can tell the documentation here is correct and complete. It describes all the different ways you can specify menus, with examples. It appears to match the jsdoc information here.

Maybe I misunderstood your question- if you can clarify, or ask a more specific question, please re-open this issue or create a new one. Thanks!

[Richienb]

@ericrosenbaum
The jsdocs do not document the expected behaviour.

ExtensionMenuMetadata can be either ExtensionDynamicMenu or ExtensionMenuItems:
https://github.com/LLK/scratch-vm/blob/7c652246dfe7f54a40db681eed65a6e7a0fc68d6/src/extension-support/extension-metadata.js#L39

ExtensionDynamicMenu must be a string:
https://github.com/LLK/scratch-vm/blob/7c652246dfe7f54a40db681eed65a6e7a0fc68d6/src/extension-support/extension-metadata.js#L44

and ExtensionMenuItems must be an array of ExtensionMenuItemSimple (a string) or ExtensionMenuItemComplex ({value: *, text: string}):
https://github.com/LLK/scratch-vm/blob/7c652246dfe7f54a40db681eed65a6e7a0fc68d6/src/extension-support/extension-metadata.js#L50

In no way do these types describe the items property outlined in the examples.

[cwillisf]

I took another look and I do see one or two places where the documentation in extensions.md and in extensions-metadata.js diverge. We should definitely get these back in sync, but I don't think the priority will be especially high for us until we start working toward third-party extensions again.

It looks like the main problem is that extension-metadata.js only documents the "shorthand" cases where the menu's value is either a function name or an array of items. We need to add the case where the menu's value is an object which contains items and (optionally) other flags, like acceptReporters.

We should also triple-check this stuff against the actual menu parsing code to make sure both sources of documentation match the actual behavior.

[Richienb]

@cwillisf terminal or isTerminal?

https://github.com/LLK/scratch-vm/search?q=terminal&unscoped_q=terminal