(Finally) Fix for "Natives' Syntax-Issues & Implementation Laziness." (Issue #1774)

[19h]

Hey,

this is a fix for Issue #1774 for NodeJS is unable to start when called with latest-build V8 arguments "--harmony_typeof --harmony_weakmaps --harmony_block_scoping". This has been fixed, the JS-compliance has been "re"-implemented.

Cheers,
Kenan.

[aikar]

might be worth git rebasing and squashing all the commits into 1 with a meaningful commit message.

[19h]

Well okay, I'm considering that. Anyway any problem with my commits at all? However, the commit will follow.

[aikar]

don't have the time to fully review the change, I just noticed how many commits were involved and their commit messages, and know Ryan wouldn't like that.

Remember your commits are "copied" into the project when a pull req happens, so all those messages would then be in the projects history.

Ry could fix him self but he prolly doesnt want to have to :P

[bnoordhuis]

Sorry, but I'm not going to merge this. Long lines, doesn't follow the code style, overly clever, change for the sake of change - I could go on...

[tj]

WOAH

[19h]

Excuse me, what is your idea of "code style"? Do you want strict syntax, which, by V8 is accepted - or strict-breaking syntax which "follows the code style"? This is hilarious.. I was thinking of Node.JS being developers not bitching about JS-syntax... -.-"

However, you could probably give me an idea of what your *style looks like.

Thanks & cheers.

[19h]

Should be return !!e?e:null; Committing upon knowledge of the future and given relevance of the changes of this fork.

[aikar]

"This is hilarious.. I was thinking of Node.JS being developers not bitching about JS-syntax... -.-"
So you think node.js developers are all novices who do not work professionally?

If you work on a team in a professional environment, you care about style and coding standards.

[aikar]

also, I merged your changes to get an overall diff.

Stuff like this:

-   if (isNaN(value) || value === Infinity) {
-     m = isNaN(value) ? 1 : 0;
-     e = eMax;
-   } else {
+   if (isNaN(value) || value === Infinity) m = isNaN(value) ? 1 : 0, e = eMax;
+   else {

and (conflict)

++<<<<<<< HEAD
 +    if (value * (c = Math.pow(2, -e)) < 1) {
 +      e--;
 +      c *= 2;
 +    }
 +    if (e + eBias >= 1) {
 +      value += rt / c;
 +    } else {
 +      value += rt * Math.pow(2, 1 - eBias);
 +    }
 +    if (value * c >= 2) {
 +      e++;
 +      c /= 2;
 +    }
 +
 +    if (e + eBias >= eMax) {
 +      m = 0;
 +      e = eMax;
 +    } else if (e + eBias >= 1) {
 +      m = (value * c - 1) * Math.pow(2, mLen);
 +      e = e + eBias;
 +    } else {
 +      m = value * Math.pow(2, eBias - 1) * Math.pow(2, mLen);
 +      e = 0;
 +    }
++=======
+     if (value * (c = Math.pow(2, -e)) < 1) e--, c *= 2;
+     value += e + eBias >= 1 ? rt / c : rt * Math.pow(2, 1 - eBias);
+     value * c >= 2 && ( e++, c /= 2 );
+     e + eBias >= eMax ? ( m = 0, e = eMax ) : e + eBias >= 1 ? ( m = (value * c - 1) * Math.pow(2, mLen), e = e + eBias) : (m = value * 
++>>>>>>> kenan/master

Those were horrible changes.
That's something you also do not do in a professional environment: make code hard to read.
It's not a game of who has the least line counts.

then the

 fs.symlink = function(destination, path, mode_, callback) {
    var mode = (typeof(mode_) == 'string' ? mode_ : null);
++<<<<<<< HEAD
 +  var callback_ = arguments[arguments.length - 1];
 +  callback = (typeof(callback_) == 'function' ? callback_ : null);
++=======
+   callback = (typeof(callback) == 'function' ? callback : null);
++>>>>>>> kenan/master
    binding.symlink(destination, path, mode, callback);
  };

@@@ -563,8 -562,7 +567,12 @@@ function writeAll(fd, buffer, offset, l

  fs.writeFile = function(path, data, encoding_, callback) {
    var encoding = (typeof(encoding_) == 'string' ? encoding_ : 'utf8');
++<<<<<<< HEAD
 +  var callback_ = arguments[arguments.length - 1];
 +  callback = (typeof(callback_) == 'function' ? callback_ : null);
++=======
+   callback = (typeof(callback) == 'function' ? callback : null);
++>>>>>>> kenan/master

as commented by that guy on your fork, that breaks the optional ability of mode.

I see what your trying to do, and def think the IDEA should be done, but gotta clean up the code.

just fix those var statements, why does the return types matter?

and @bnoordhuis his change is needed. it is a bug in node..

>>> node --harmony_block_scoping examples/benchmark_recv.js 

child_process.js:305
  var args = args ? args.slice(0) : [];
      ^^^^

node.js:202
        throw e; // process.nextTick error, or 'error' event on first tick
              ^
SyntaxError: Variable 'args' has already been declared
    at NativeModule.compile (node.js:514:14)

Kenan, I suggest you git branch your master to 'tmp', reset your master to upstream/master (joyents).
branch off of the new fresh master
git co -b fixnative
then
git merge --squash tmp

You'll get conflicts due to deleted files. stage the chunks that are valid (removing extra vars)
then revert the remaining ones that we've commented on as being bad.

Then simply open it as a new pull request from that branch, and be sure to indicate WHY this is an issue in clearer detail.

[aikar]

All of these hex->dec changes was also completely unneeded.

It's much easier to read in hex. Don't keep any of these changes.

[19h]

I was thinking of optimizing: hex-values have to be converted to decimals, costs runtime. Anyway, fixing.

[aikar]

That should all be optimized by V8 itself.

[aikar]

Don't forget: V8 is smarter than us all :P

[19h]

Not really, just a few billion times faster. :)

[aikar]

no really... V8 is smarter than you :P
don't try to outwit it, unless your a v8 developer.

[aikar]

this breaks the code.

If mode == your callback function, callback will be undefined.

[19h]

Thank you for that specific outlining of the problems, I will fix that and recommit. In fact, I was thinking about the callback-issue yesterday.. anyway, cheers.

Edit:
Also, the return types are required for strict Javascript. Any function should return at least something - this has been since ECMAScript. It's just conformity with the strict-JS standards.

[chjj]

Excuse me, what is your idea of "code style"? Do you want strict syntax, which, by V8 is accepted - or strict-breaking syntax which "follows the code style"? This is hilarious.. I was thinking of Node.JS being developers not bitching about JS-syntax... -.-"

I count 305 cols in child_process_legacy.js. That's some kind of record, ...and people wonder what I mean when I say John-Resig-style JS is like a disease? The void 0 stuff is also out of place, considering undefined is read-only in v8 now.

Ben is right not to merge this. It's too tricky. It's client-side JS sneaking into the server side. No one writes code like that and actually maintains it, in any project worth maintaining.

If you can't figure out what he meant by style conventions, it might be hopeless.

[19h]

This is the "style convention" the company I'm working in has standardized .. and we got used to. I'm sorry, it's our style of coding here, and we're actually maintaining code like this. But 'client-side JS sneaking into the server side'? That's quite unfair. We're pretty much only working with server-side code, and this is not criticizing the functional but stylistic features of coding.. I already said, I'll be revising the code.

Have a nice evening! :)

+edit: And as already said: as I considered this being build upon V8, I wanted to implement V8 standards. The V8-engine proposes the usage of void 0 instead of undefined. Anyway, I can revise that, at least to get the JS-_re_conformity implementations pulled.

[19h]

This is now recommitted.