Unindented use of global variables in TiddlyWiki 2.0.3 core (missing "var")

[Udo Borkowski]

Jeremy:

In TiddlyWiki 2.0.3 there are some locations that use a global variable where a local variable is probably intended (i.e. missing "var"). Here is what I found:

1+2) function detectPlugin() // Lifted from http://developer.apple.com/internet/webcontent/detectplugins.html

        for (pluginsArrayCounter=0; pluginsArrayCounter < pluginsArrayLength; pluginsArrayCounter++ )
...
            for(namesCounter=0; namesCounter < daPlugins.length; namesCounter++)



3) Story.prototype.createTiddler = function(place,before,title,template)
{
    theTiddler = createTiddlyElement(null,"div",this.idPrefix + title,"tiddler",null);




4) TiddlyWiki.prototype.createTiddler = function(title)
{
    tiddler = this.fetchTiddler(title);




5) Number.prototype.clamp = function(min,max)
{
    c = this;



6) config.macros.list.handler = function(place,macroName,params)
{
    var type = params[0] ? params[0] : "all";
...
    if(this[type].handler)
        results = this[type].handler(params);
    for (t = 0; t < results.length; t++)



7) config.macros.allTags.handler = function(place,macroName,params)
{
    var tags = store.getTags();
...
    for (t=0; t<tags.length; t++)



8) config.macros.timeline.handler = function(place,macroName,params)
{
....
    var c = 0;

    for (t=tiddlers.length-1; t>=0; t--)



9) config.macros.gradient.handler = function(place,macroName,params,wikifier)
{
...
    if(wikifier)
        {
        var styles = config.formatterHelpers.inlineCssHelper(wikifier);
        var t; // scope?
        for(t=0; t<styles.length; t++)
            panel.style[styles[t].style] = styles[t].value;
        }
    var colours = [];
    for(t=1; t<params.length; t++)



10) function applyPageTemplate(title)
...
    if(display)
        {
        nodes = display.childNodes;
        for(var t=nodes.length-1; t>=0; t--) //scope?
            stash.appendChild(nodes[t]);
        }
...
    nodes = stash.childNodes;
    for(t=nodes.length-1; t>=0; t--)
        display.appendChild(nodes[t]);



11) Wikifier.prototype.subWikify = function(output,terminator)
            this.matchText = formatterMatch[0];
            this.nextMatch = this.formatter.formatterRegExp.lastIndex;
            // Figure out which formatter matched
            var matchingformatter = -1; // Typo! lower ...formatter vs. ...Formatter
            for(var t=1; t<formatterMatch.length; t++)
                if(formatterMatch[t])
                    matchingFormatter = t-1;
            // Call the formatter



The fixes should be easy.

(I hope you read this email with HTML formatting, since I marked the locations bold).


Udo

[Jeremy Ruston]

Udo, that's terrific, many thanks!

Cheers

Jeremy

--
Jeremy Ruston
mailto:jer...@osmosoft.com
http://www.tiddlywiki.com

[Udo Borkowski]

Notice that these may not be all places, just the ones I got when playing around with my TW installation.

So there may be code branches that I haven't reached during my tests and that define global variables. You may want to "rerun" my "List global Variables" tiddler (just posted in other group) from time to time...


Udo

[Jeremy Ruston]

> So there may be code branches that I haven't reached during my tests and
> that define global variables. You may want to "rerun" my "List global
> Variables" tiddler (just posted in other group) from time to time...

It would be terrific if there was a decent 'lint' type utility for JavaScript...

By the way, numbers 9 and 10 in your list are actually OK; bizarrely
for those of us used to C-type languages, in JavaScript the scope of a
var is a function, not a statement block. I've corrected them anyway
because it's clearly better style.

Many thanks again,

Cheers

Jeremy

[Udo Borkowski]

It would be terrific if there was a decent 'lint' type utility for JavaScript...

I absolutely agree. But I guess for such loosely-typed, prototype-based languages like JavaScript this will be a little bit harder than C etc.. Or does anyone know about such a tool?

By the way, numbers 9 and 10 in your list are actually OK; bizarrely
  

Are you sure about 9 and 10? In both cases the variable t is only created as "var" in a conditional block (either "if(wikifier)" or "if(display)"). So if no wikifier is specified or or "display" is null the first time the "t" is encountered is in the "non-var" assignment statement.

But if you move the var to the outside everthing is fine and we can just ignore these subtelities....


Udo

[Florian Cauvin]

>> It would be terrific if there was a decent 'lint' type utility for JavaScript...
> I absolutely agree. But I guess for such loosely-typed, prototype-based
> languages like JavaScript this will be a little bit harder than C etc..
> Or does anyone know about such a tool?

There is one free tool called jslint (which is itself a javascript
app):
http://www.crockford.com/jslint/lint.html

[Udo Borkowski]

Thanks for the reference to jslint.

That's quite a nice tool, and in fact it also helps to track down the "FireFox 1.5.0.1" issue, in that it reports all "globals" used in a program!

You can specify a list of globals you expect to use (e.g the DOM globals like "document, window, alert" or the globals provided by TiddlyWiki) and it lists all "other globals". In that it is very similar to the output provided by the "List all globals" tiddler I posted recently. But the jslint approach has the big advantage that it processes the JavaScript source and does a static analysis, vs. my ("dynamic") approach that only found the globals used during running the tests. So jslint is the one to prefer.

The only downside is that the jslint tool enforces some coding style that may not be everyone's favorite.

E.g. all blocks in "if", "while" etc. must be "real blocks" with "{...}" i.e. you cannot write something like

    while(e && !hasClass(e,"tiddler"))
        e = e.parentNode;

but you need to write:

    while(e && !hasClass(e,"tiddler")) {
        e = e.parentNode;
    }

Also it is quite strict in the use of ";" to end statements and some other .

As a consequence you will probably need to edit you JavaScript source quite a bit when you run it against jslint the first time. You also have the option to disable some checks. In addition, since the tool is written in JavaScript and open source you also have the option to adapt it to your style (see below).

I think jslint is definitely a tool a JavaScript developer should have a look at. It already found two bugs in my YourSearchPlugin. I will probably check all my plugin code against that tool, step by step.


Udo


BTW: If you would like to use single statements in you "if", "while" etc. and don't want to add the "{...}" you may change the jslint function block and set jslint.allowSingleStatementAsBlock = true.


    function block() {
        var t = token;
        if (!jslint.allowSingleStatementAsBlock || token.id == '{') {
            advance('{');
            statements();
            advance('}', t);
        } else {
            statement();
        }
        verb = null;
    }