Re: [Wikiwyg-dev] Re: BUG: JavaScript strict warnings (Dave Howorth)

[Alex Vincent <alex.vincent@corp.manyone.net>]

Ingy dot Net wrote:
> Alex,
>
> I have a lot of questions on this patch. Some of these questions might be
> addressed in earlier posts that I have not read yet (grrrrr), but here I go
> anyway:
>   
No, I just posted the patch this morning.  :-)  Actually, you're the 
first person to reply.
> As far as I know, these 3 function headers mean the same thing (in the global
> scope):
>
>     onload = function() { ...
>     window.onload = function() { ...
>     function onload() { ...
>
> and even this:
>
>     this.onload = function() { ...
>
> So this:
>
>     window.onload = function onload() { ...
>
> seems redundant at best
They may have the same global scope, but Mozilla will tell you there are 
strict warnings with it.  What this means is that
it's making its "best guess" at what your intent is, and it may have 
guessed wrong.  It's really better to fix the warnings and be
absolutely clear to the JS interpreter what you want.  This helps you in 
turn by forcing you to be clearer in your code, so that
other developers have a lower chance of accidentally breaking your code.

Explicitly declaring "window.onload" removes the confusion.

As for function() versus function onload(), that's more of a stylistic 
difference.  The key there, though, is that Venkman, Mozilla's 
JavaScript debugger, reads the function declaration for function names.  
Then when it generates a JavaScript function stack for debugging, you 
can see clearly what function it is.

> , and this:
>
>     proto.insert_text_at_cursor = function insert_text_at_cursor(text) {
>
> just seems wrong.
>   
Again, this is more for the debugger's benefit.
> ---
>
> Next, variables defined at the global scope are global whether or not you use
> `var`. So the Wikiwyg style is to not use `var` there:
>
>     -klass = Wikiwyg.Wikitext;
>     +var klass = Wikiwyg.Wikitext;
>   
Which causes more strict warnings.
> ---
>
> Also I'm not sure why you /do/ return null sometimes and don't others:
>
>     +        return null; // Silence strict warning.
>
> vs:
>
>     -            return false
>     +            return;
>
> ---
>   
Mainly I was trying to keep with the convention in the function in 
question, and check the caller to see if they're checking a return 
value.  If they do check a return value, then generally I want to return 
something.  :-)

On the return after die(), return null is just to silence the warning, 
since die() throws an exception.
> If you could address these for me I surely appreciate it.
>
> A final note... this patch contains quite a few different types of
> changes. It is better for me to have more smaller patches than one big
> patch. Of course that's a little bit harder for you since you don't have
> commit access yet.
>
> Cheers, Ingy
>   
I'll work on that, but I figured this patch was small enough :-)

Alex