using c.test in E.showMenu

@skm I'm just moving this here - as posting on the new firmware announcement thread probably isn't that useful.

As your reported:

This code works in 2v09 (and earlier) but not in 2v10:

var c = {};
c.test = false;
var main_menu = {
  "Test" : { value : c.test, format : v => v?"On":"Off", onchange : v => { c.test = v; }, },
};
E.showMenu(main_menu);

When I press the middle button, I get this error:

Uncaught Error: Field or method "test¿ñ" does not already exist, and can't create it on Number
 at line 1 col 10
c.test = v;
         ^
in function "onchange" called from line 1 col 223
...onchange)b.onchange(b.value);d.draw()­}
                              ^
in function "select" called from line 1 col 22
b?d.move(b):d.select()
                     ^
in function "cb" called from line 1 col 4
cb();
   ^
in function called from system

I had the same results from the right-hand window of the IDE until I uninstalled RAM Widget(!). Now running the code from RAM is fine, but running it as an app still occasionally gives me the error messages, and the odd characters are different every time:

Uncaught Error: Field or method "testê¡à" does not already exist, and can't create it on Number
Uncaught Error: Field or method "testÚ¡Ù" does not already exist, and can't create it on Number
Uncaught Error: Field or method "testuQå" does not already exist, and can't create it on Number

The code always works fine if I replace every instance of "c.test" with "ctest"

I've just dug a little deeper and I can reproduce even on a desktop build with:

g = Graphics.createArrayBuffer(240,240,1);
g.flip = x=>x;
Modules.addCached("locale",function() {
  exports.translate = x=>x
});

E.showMenu = (function(n){g.clear(1);Bangle.setLCDPow­er(1);Bangle.drawWidgets();if(n){var q=g.getWidth()-9;g.getHeight();var f=Object.keys(n),a=n[""];a&&f.splice(f.i­ndexOf(""),1);a instanceof Object||(a={});a.fontHeight=16;a.x=0;a.x­2=q-2;a.y=24;a.y2=220;void 0===a.selected&&(a.selected=0);a.fontHei­ght||(a.fontHeight=6);var r=0|a.x,p=a.x2||g.getWidth()-1,m=0|a.y,u­=a.y2||g.getHeight()-1;a.title&&(m+=a.fo­ntHeight+2);var t=require("locale"),d={lastIdx:0,draw:fu­nction(b,c){var l=0|Math.min((u-
m)/a.fontHeight,f.length),e=E.clip(a.sel­ected-(l>>1),0,f.length-l);e!=d.lastIdx&­&(b=void 0);d.lastIdx=e;var h=m;g.reset().setFont("6x8",2).setFontAl­ign(0,-1,0);void 0===b&&a.title&&(g.drawString(a.title,(r­+p)/2,m-a.fontHeight-2),g.drawLine(r,m-2­,p,m-2));void 0!==b&&(e<b&&(h+=a.fontHeight*(b-e),e=b)­,e+l>c&&(l=1+c-b));for(;l--;){c=f[e];b=n­[c];var k=e==a.selected&&!d.selectEdit;g.setColo­r(k?g.theme.bgH:g.theme.bg);g.fillRect(r­,h,p,h+a.fontHeight-1);g.setColor(k?g.th­eme.fgH:g.theme.fg);g.setFontAlign(-1,
-1);g.drawString(t.translate(c),r,h);"ob­ject"==typeof b&&(c=p,k=b.value,b.format&&(k=b.format(­k)),k=t.translate(""+k),d.selectEdit&&e=­=a.selected&&(c-=25,g.setColor(g.theme.b­gH).fillRect(c-(g.stringWidth(k)+4),h,p,­h+a.fontHeight-1),g.setColor(g.theme.fgH­).drawImage("\f\u0005\u0081\x00 \u0007\x00\u00f9\u00f0\u000e\x00@",c,h+(­a.fontHeight-10)/2,{scale:2})),g.setFont­Align(1,-1),g.drawString(k,c-2,h));g.set­Color(g.theme.fg);h+=a.fontHeight;e++}g.­setFontAlign(-1,-1);l=e<f.length;g.drawI­mage("\b\b\u0001\u00108|\u00fe\u0010\u00­10\u0010\u0010",
q,40);g.drawImage("\b\b\u0001\u0010\u001­0\u0010\u0010\u00fe|8\u0010",q,194);g.dr­awImage("\b\b\u0001\x00\b\f\u000e\u00ff\­u000e\f\b",q,116);g.setColor(l?g.theme.f­g:g.theme.bg).fillPoly([104,220,136,220,­120,228]);g.flip()},select:function(b){b­=n[f[a.selected]];if("function"==typeof b)b(d);else if("object"==typeof b){if("number"==typeof b.value)d.selectEdit=d.selectEdit?void 0:b;else if("boolean"==typeof b.value&&(b.value=!b.value),b.onchange)b­.onchange(b.value);d.draw()}},move:funct­ion(b){if(d.selectEdit){var c=
d.selectEdit;c.value-=(b||1)*(c.step||1)­;void 0!==c.min&&c.value<c.min&&(c.value=c.min­);void 0!==c.max&&c.value>c.max&&(c.value=c.max­);if(c.onchange)c.onchange(c.value);d.dr­aw(a.selected,a.selected)}else c=a.selected,a.selected=(b+a.selected)%f­.length,0>a.selected&&(a.selected+=f.len­gth),d.draw(Math.min(c,a.selected),Math.­max(c,a.selected))}};d.draw();Bangle.set­UI("updown",b=>{b?d.move(b):d.select()})­;return d}Bangle.setUI()});

var Bangle = {
  setUI : ()=>{},
  setLCDPower : ()=>{},
  drawWidgets : ()=>{}
};

var c = {};
c.test = false;
var main_menu = {
  "Test" : { value : c.test, format : v => v?"On":"Off", onchange : v => { c.test = v; }, },
};
var l = E.showMenu(main_menu);
l.move(1);
print(c);
l.select();
print(c);

Turns out it's not a memory corruption issue or anything like that (although the Field or method "testuQå" error does seem ominous). It's to do with how the code for menu.move was minified.

The function (after minification, and with some semblance of formatting added) is:

move = function(b){
  if(d.selectEdit){
    var c=d.selectEdit;
    c.value-=(b||1)*(c.step||1);
    void 0!==c.min&&c.value<c.min&&(c.value=c.min­);
    void 0!==c.max&&c.value>c.max&&(c.value=c.max­);
    if(c.onchange)c.onchange(c.value);
    d.draw(a.selected,a.selected)
  } else c=a.selected,a.selected=(b+a.selected)%f­.length,0>a.selected&&(a.selected+=f.len­gth),d.draw(Math.min(c,a.selected),Math.­max(c,a.selected))
}

Unminified code is:

move : function(dir) {
      if (l.selectEdit) {
        var item = l.selectEdit;
        item.value -= (dir||1)*(item.step||1);
        if (item.min!==undefined && item.value<item.min) item.value = item.min;
        if (item.max!==undefined && item.value>item.max) item.value = item.max;
        if (item.onchange) item.onchange(item.value);
        l.draw(options.selected,options.selected­);
      } else {
        var a=options.selected;
        options.selected = (dir+options.selected)%menuItems.length;­
        if (options.selected<0) options.selected += menuItems.length;
        l.draw(Math.min(a,options.selected), Math.max(a,options.selected));
      }
    }

I guess it's valid JS, but what the minifier has done is change all variable names to c, and then it has just a single var c in the first if statement, and nowhere else. So if that if doesn't get run in Espruino, no local var is created and it assumes that the var is global, overwriting c.

So in short:

• Looks like global variables named c are a no-go in Espruino 2v10
  
• I'll get a fix in so the minifier doesn't do this and it's fixed for 2v11
  

... also just fixed Field or method "testuQå" does not already exist - it was a string formatting bug in the error message

Wed 2021.10.27

'Looks like global variables named c are a no-go in Espruino 2v10'

I believe there are several/many others as I demonstrated in several code block examples over 18 months ago:

http://forum.espruino.com/conversations/­346667/#comment16244096

Those should provide useful for eliminating the other letters of the alphabet.

@Robin, could you be a bit more specific on

I believe there are several/many others 'what?'

@Gordon,

So if that if doesn't get run in Espruino, no local var is created and it assumes that the var is global, overwriting c

I assume this is because Espruino is interpreting on the source code without compilation into any other format first, vs practically all and especially the browser JS engines do.

Solution of the issue would be to declare the variable as 1st thing in the function body before any use. I'm sure this would also survive the minifier and cost 'nothing':

Code_1:

move : function(dir) {
      var item,a;
      if (l.selectEdit) {
        item = l.selectEdit;
        item.value -= (dir||1)*(item.step||1);
        if (item.min!==undefined && item.value<item.min) item.value = item.min;
        if (item.max!==undefined && item.value>item.max) item.value = item.max;
        if (item.onchange) item.onchange(item.value);
        l.draw(options.selected,options.selected­);
      } else {
        a=options.selected;
        options.selected = (dir+options.selected)%menuItems.length;­
        if (options.selected<0) options.selected += menuItems.length;
        l.draw(Math.min(a,options.selected), Math.max(a,options.selected));
      }
    }

I could go even a step further and actually gain some bytes by doing some funny things:

Code_2:

move : function(dir,item,a) {
      if (item = l.selectEdit) {
        item.value -= (dir||1)*(item.step||1);
        if (item.min!==undefined && item.value<item.min) item.value = item.min;
        if (item.max!==undefined && item.value>item.max) item.value = item.max;
        if (item.onchange) item.onchange(item.value);
        l.draw(options.selected,options.selected­);
      } else {
        a=options.selected;
        options.selected = (dir+( a=options.selected))%menuItems.length;
        if (options.selected<0) options.selected += menuItems.length;
        l.draw(Math.min(a,options.selected), Math.max(a,options.selected));
      }
    }

Note that the Espruino IDE editor's linter complains with a warning about the logical expression in 1st line of function body to be an unexpected assignment. But that is just the linter. Since I do not like warnings hanging around, I use double parens to 'hide' the fact: ((item = l.selectEdit)). Adding the local var definitions to the function's argument list does get them defined for me... (and since there are no optional args in move() nor there is some arg count dependency, it does not hurt).

Looking even closer at what is going on, I notice that actually only exactly one var is needed that is mutually exclusively used in the then and else path to hold on to a value. Therefore, this is my - almost - final solution (legible? ...that's a different ball game).

Code_3:

move : function(dir,v) {
      if (v = l.selectEdit) {
        v.value -= (dir||1)*(v.step||1);
        if (v.min!==undefined && v.value<v.min) v.value = v.min;
        if (v.max!==undefined && v.value>v.max) v.value = v.max;
        if (v.onchange) v.onchange(v.value);
        l.draw(v=options.selected,v);
      } else {
        options.selected = (dir+(v=options.selected))%menuItems.len­gth;
        if (options.selected<0) options.selected += menuItems.length;
        l.draw(Math.min(v,options.selected), Math.max(v,options.selected));
      }
    }

Growing up with <= 16KBytes for serious biz application including utilities, such as ISAM for one key, you end up really at the bottom of things: since item.value and options.selected shows up so many times, getting a second work variable w in will cut back even more on footprint (and legibility/maintainability - :\ ), and(/but) surpasses what ever a minifier can do without knowing that there is no compiler with optimization involved on execution:

Code_4:

move: function(dir,v,w) {
  if (v = l.selectEdit) { // --- value selection
    w = v.value - (dir||1)*(v.step||1);
    v.value = w = (v.min!==undefined && w<v.min) ? v.min
                : (v.max!==undefined && w>v.max) ? v.max
                : w;
    if (v.onchange) v.onchange(w);
    l.draw(w=options.selected,w);
  } else { // --- menu item selection
    w=((dir||1)+(v=options.selected)+(w=menu­Items.length))%w;
    l.draw(Math.min(v,options.selected=w), Math.max(v,w));
  }
}

I'm surprised that item.min and item.max can be undefined; after all it looks like a magnitude thing and not discrete (non-magnitude) values. This should be resolved on item creation... If that can be resolved, things shrink even more. Same can be said of the onchange callback: it could be initialized with empty regular or fat function and called unconditionally, all the times.

Also: why is dir (ection in values and list) once value checked/defaulted and once not? (so I defaulted 2nd one too) ...and why is 1st one not positive 1 defaulted?

Code_4: Minified - 318 byes vs. 460 as fixed Code_1 AND unminified code in post #2... 30% minified savings wow! <----------

(using google closure compiler service all with var l, move = function(.... and manually 'prettied'):

move: function(c,a,b){
  if(a=l.selectEdit){
    b=a.value-(c||1)*(a.step||1);
    a.value=b=void 0!==a.min&&b<a.min?a.min:void 0!==a.max&&b>a.max?a.max:b;
    if(a.onchange)a.onchange(b);
    l.draw(b=options.selected,b) }
  else b=((c||1)+(a=options.selected)+(b=menuIt­ems.length))%b
     , l.draw(Math.min(a,options.selected=b),Ma­th.max(a,b))
};

Note the shrinking of the tertiary min/max statements...

--- I never ran into this issue - var ref not known and trying to overwrite global - even with the most obscure code in the https://espruino.com/UI. Surprisingly related: In the past, I had a manuscript editor for my publications who remembered whether I had defined / introduced a term or not before using it, even though the editor was not a subject matter expert, nor did the editor use the computer, and the publication could be several hundredths of pages long. It was an amazing - and painful - experience... to yield and get the things in sequence | order | defined... and that was just in natural language / English. Now, I see the same phenomenon in a formal language, JS, where the reader is not a human but a machine. Obvious conclusion: both need a proper setup of context to operate correctly!

Lessons learned for Espruino: Smart intro, placement and use of intermediate 'work' variables vs. Fowler'sh demanded paradigm of referencing and some familiarity w/ (ESPRUINO's) interpreter's workings buys you A LOT OF FREE ESPRUINO VARS (bytes)!

PS1: M. Fowler context: C/C++, Java, JS w/ compiler... - to be context-correct! - Clean Code and Refactoring are excellent readings.

PS2: interested in feedback if above code works... (have no immediate 'machine' setup to validate).

Btw, I stated in previous post - #6 - that Code_1 survives the minifier... I just checked and it does not! - It fails with the same issue, because - again - the single variable definition of a moves into the then path and if the else path is executed, overwrite of global a still happens... ouch:

Code_1 minified:

move: function(b){
  if(l.selectEdit){
    var a=l.selectEdit;
    a.value-=(b||1)*(a.step||1);
    void 0!==a.min&&a.value<a.min&&(a.value=a.min­);
    void 0!==a.max&&a.value>a.max&&(a.value=a.max­);
    if(a.onchange)a.onchange(a.value);
    l.draw(options.selected,options.selected­)}
  else a=options.selected,options.selected=(b+o­ptions.selected)%menuItems.length
     , 0>options.selected&&(options.selected+=m­enuItems.length)
     , l.draw(Math.min(a,options.selected),Math­.max(a,options.selected))
};

A small change though makes a big difference and makes the variable definition to survive the minifier and also already safes 11 bytes....

Code_1.1:

move: function(dir) {
      var item = l.selectEdit,a;
      if (item) {
        item.value -= (dir||1)*(item.step||1);
        if (item.min!==undefined && item.value<item.min) item.value = item.min;
        if (item.max!==undefined && item.value>item.max) item.value = item.max;
        if (item.onchange) item.onchange(item.value);
        l.draw(options.selected,options.selected­);
      } else {
        a=options.selected;
        options.selected = (dir+options.selected)%menuItems.length;­
        if (options.selected<0) options.selected += menuItems.length;
        l.draw(Math.min(a,options.selected), Math.max(a,options.selected));
      }
    };

Code_1.1 minified:

move: function(b){
  var a=l.selectEdit;
  if(a){
    a.value-=(b||1)*(a.step||1);
    void 0!==a.min&&a.value<a.min&&(a.value=a.min­);
    void 0!==a.max&&a.value>a.max(a.value=a.max);­
    if(a.onchange)a.onchange(a.value);
    l.draw(options.selected,options.selected­)}
  else a=options.selected
     , options.selected(b+options.selected)%men­uItems.length
     , 0>options.selected&&(options.selected+=m­enuItems.length)
     , l.draw(Math.min(a,options.selected),Math­.max(a,options.selected))
};

Very interesting.

The whole thing comes form JS var behavior introduced by the compile into byte code / the way byte code is generated: Any encountered var in a var-scoped block (function) moves the declaration IN THE BYTE CODE 'up' to be processed before the first execution statement... See Variable Hoisting - https://www.sitepoint.com/demystifying-j­avascript-variable-scope-hoisting/. The regular minifier is based on that behavior, takes advantage of it, and sees no issue with it. After all minification is the goal... compiler/byte code generation will take care of proper sequencing.

Espruino does no hoisting... it sticks to the sequence by its very own and unique nature... (see https://www.espruino.com/Features#genera­l).

Thanks! I'm not sure if you'd spotted, but this is pretty much exactly the fix I'd put in: https://github.com/espruino/Espruino/com­mit/3814bedff6a4b4457940f1cd41ffbe445c7e­8425#diff-1b2a6245c921816c1acf6e4a95da40­4e0eebd9a784b660de19fde5e44f0bc36d

Yes, the lack of hoisting is a bit tricky. It's something I could conceivably add I guess, but it's all extra memory used to store the list of hoisted variables.

@Robin your issues aren't related at all. I've tried to explain in response to your most recent post

@Gordon, thanks for pointing it out. It can really be tricky with minification. It was fun to think thru this small piece of code...

Btw, these two lines

  options.selected = (dir+options.selected)%menuItems.length;­
  if (options.selected<0) options.selected += menuItems.length;

can be put into one, since % takes care of the business:

options.selected = (dir+options.selected+menuItems.length)%­menuItems.length;

Ahh, that's a nice trick - thanks! While % doesn't like negative numbers, adding the length solves it nicely :)