1 Correct answer
Thank you all so much for your replies, and thank you Harbs for making it finally work!
Just to make sure this can serve as a resource for others in the future, below is a final (working) menu based on Harb's code. I also added 2 submenu items just to be complete.
Thanks again everyone. I'm so glad you could help get this working!
Dan
...#targetengine "HarbsTestMenu"
menuInstaller();
function menuInstaller() {
// SET THE FILES THAT ARE TRIGGERED BY MENU ITEMS
menuItem1Handler = function( /*onInvo
54
Replies
Note to self: Don't respond to the forum before your brain is engaged... 
Ignore what I wrote above. It made no sense on a lot of levels...
The issue at hand is caused by the following:
Submenus do not go away until you remove them. When restart InDesign, the submenu still exists, so you are creating the meni tems below it.
You need to create the submneu first and specify LocationOptions. Something like this:
function menuInstaller() {
var menuItem1T = "My Menu Item 1",
menuItem2T = "My Menu Item 2",
menuT = "MyTestMenu",
subT = "Sub Menu",
subs = app.menus.item("$ID/Main").submenus, sma, mnu;
var refItem = app.menus.item("$ID/Main").submenus.item("$ID/&Layout");
subMenu1 = app.scriptMenuActions.item(menuItem1T);
if( subMenu1 == null ) {
subMenu1 = app.scriptMenuActions.add(menuItem1T);
}
subMenu1.eventListeners.add("onInvoke", menuItem1Handler);
subMenu2 = app.scriptMenuActions.item(menuItem2T);
if( subMenu2 == null ) {
subMenu2 = app.scriptMenuActions.add(menuItem2T);
}
subMenu2.eventListeners.add("onInvoke", menuItem2Handler);
mnu = subs.item(menuT);
if( mnu == null ) {
mnu = subs.add(menuT, LocationOptions.after, refItem);
}
subsSubs = app.menus.item( '$ID/Main' ).submenus.item( menuT ).submenus;
mnuSubMenu = subsSubs.item( subT );
if( mnuSubMenu == null ) {
mnuSubMenu = subsSubs.add( subT);
}
mnu.menuItems.add(subMenu1,LocationOptions.BEFORE,mnuSubMenu);
mnu.menuSeparators.add(LocationOptions.BEFORE,mnuSubMenu);
mnu.menuItems.add(subMenu2,LocationOptions.AFTER,mnuSubMenu);
};
Harbs
Harbs. wrote:
Chances are, my code was probably easier to read...
Well, in this area, I think "all is relative" but I'm not going to argue this specific point.
Now, let's suppose that someone wants to use your menu loader in another project having a different number of menu items and submenus. Say he wants to obtain the following hierarchy:
MyMenu
MyFeature1
MyFeature2
MySubmenuA
MyFeatureA1
MyFeatureA2
MyFeatureA3
MySubmenuB
MyFeatureB1
MyFeatureB2
etc.
Do you see how many changes he needs to do in your code?
In the solution I suggest, he only has to update the FEATURES array this way:
FEATURES = [
{ caption: "MyFeature1", fileName: "File-One.jsx", subName: "" }, { caption: "MyFeature2", fileName: "File-Two.jsx", subName: "" },
{ caption: "MyFeatureA1", fileName: "File-A-One.jsx", subName: "MySubmenuA" }, { caption: "MyFeatureA2", fileName: "File-A-Two.jsx", subName: "MySubmenuA" }, { caption: "MyFeatureA3", fileName: "File-A-Three.jsx", subName: "MySubmenuA" },
{ caption: "MyFeatureB1", fileName: "File-B-One.jsx", subName: "MySubmenuB" }, { caption: "MyFeatureB2", fileName: "File-B-Two.jsx", subName: "MySubmenuB" }
]
Is this less readable?
@+
Marc
Like you said, it's relative. I personally find your code a pleasure to read, but my natural style is a bit different.
It really depends on your background.
If you have a strong OOP background, your code is pure poetry! 
If you don't have an OOP background, it takes a lot of mental overhead to follow how the code is supposed to work.
Harbs
When I answer questions, I try to stick as closely to the coding style of the OP as I can, to help answer the question at hand. (It helps clarify the answer in few words -- and I don't enjoy excessive writing...) 
Chances are I'm not going to change anyone's coding style in a single forum post...
Hey, Marc: I'm going to be sort of blunt. Your code is hard for most people to read because it uses Javascript idioms they are entirely unfamiliar with, which don't make a lot of sense to people not really familiar with the language. And that's what most people here are. Let's take a walk through it:
(function(/*obj|undefined*/HOST)
The comment itself is confusing. Actually, it's even confusing to me.
HOST || (HOST=$.global);
Tbe || idiom is confusing. Most people don't know what it means offhand.
// ---
// Prevent the current (startup) script from being uselessly rerun
// ---
if( HOST[$.engineName] )
{
alert( "This script is automatically executed at startup. You shouldn't run it manually." );
Well, this isn't confusing, but I think it's bad. One of the important things to do is to test what happens when the menus already exist, and that's easiest to do by rerunning the script.
var MENU_NAME = "My Test Menu",
All caps for a function-local variable? Even using all caps for a globally scoped variable (Crockford) is confusing, but for a function-scope variable? That confuses me!
FEATURES = [
{ caption: "My Menu Item 1", fileName: 'File-One.jsx', subName: "" },
Use of Objects is justified here, but they are probably confusing.
FEATURE_LOCATION_PATH = (function()
Anonymous functions...definitely confusing! I'm also not sure what the point of this function heere is. Why not execute the code directly
on the next line?
try{ f=app.activeScript; }
catch(_){ f=File(_.fileName); }
Variables named "_"? Confusing!
})();
And this is why anonymous functions are confusing! Not much to be done about it, but why use one where you don't need to. It's not like we're polluting the global namespace here.
while( i-- )
Use of a while loop is a totally legitimate style choice, but I think it's typically less readable than a for loop. YMMV, of course!
t = FEATURES;
I'm horribly guilty of this, but single-letter variable names are not the most readable.
(t.action = app.scriptMenuActions.add( t.caption )).
addEventListener('onInvoke', f);
More parentheses imply more mental gymnastics to understand. Breaking this into two lines,
with t.action=addEventListener... would help readability.
var s,
n = FEATURES.length,
subs = {},
sub = null;
Wait, a mid-function var block? Gasp! What's up with that? Crockford's right on this. One var statement and all at the top.
I am not sure why we're initializing sub to null -- what's wrong with the good old undefined? This probably doesn't qualify as confusing.
for( i=0 ; i < n ; ++i )
Caching FEATURES.length in n is good for performance, but it probably doesn't matter here, and makes it more confusing to read. Not by much, but let's say half a confusion point. I would also say that "++i" is harder to read than "i++", maybe to the tune of 0.1 confusion-points, but maybe that's just me. YMMV Extremely.
sub = (s=t.subName) ?
( subs|| (subs=mnu.submenus.add( s, LO_END )) ) :
mnu;
The ternary conditional operator, ?:, is pretty much always hard to read. And it's worse with short-circuit or-ing, ||, or three close-parens in rapid succession. Of course ?: tends to promote extra () as it almost always becomes ()?(): so...
}
})();
And then it's worth re-emphasizing, basically nobody understands the anonymous function paradigm .
So there we have it, that's why I think your code is confusing to most readers.
(On the other hand Dan's final code ignores my recommendation and still has those abysmal ==null tests in there. What's up with that? )
AdChoices