Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
syntax fixes in src/node.js
Browse files Browse the repository at this point in the history
  • Loading branch information
joshaven authored and ry committed Dec 18, 2009
1 parent d8e69d3 commit 7873639
Showing 1 changed file with 31 additions and 30 deletions.
61 changes: 31 additions & 30 deletions src/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,23 +12,23 @@ GLOBAL.include = function () {

GLOBAL.puts = function () {
throw new Error("puts() has moved. Use require('sys') to bring it back.");
}
};

GLOBAL.print = function () {
throw new Error("print() has moved. Use require('sys') to bring it back.");
}
};

GLOBAL.p = function () {
throw new Error("p() has moved. Use require('sys') to bring it back.");
}
};

process.debug = function () {
throw new Error("process.debug() has moved. Use require('sys') to bring it back.");
}
};

process.error = function () {
throw new Error("process.error() has moved. Use require('sys') to bring it back.");
}
};

GLOBAL.node = {};

Expand Down Expand Up @@ -118,9 +118,9 @@ process.mixin = function() {
--i;
}

for ( ; i < length; i++ )
for ( ; i < length; i++ ) {
// Only deal with non-null/undefined values
if ( (options = arguments[ i ]) != null )
if ( (options = arguments[ i ]) != null ) {
// Extend the base object
for ( var name in options ) {
var src = target[ name ], copy = options[ name ];
Expand All @@ -130,18 +130,19 @@ process.mixin = function() {
continue;

// Recurse if we're merging object values
if ( deep && copy && typeof copy === "object" )
if ( deep && copy && typeof copy === "object" ) {
target[ name ] = process.mixin( deep,
// Never move original objects, clone them
src || ( copy.length != null ? [ ] : { } )
, copy );

// Don't bring in undefined values
else
} else {
target[ name ] = copy;

}
}

}
}
// Return the modified object
return target;
};
Expand Down Expand Up @@ -195,7 +196,7 @@ process.Promise = function () {
process.EventEmitter.call();
this._blocking = false;
this._hasFired = false;
}
};
process.inherits(process.Promise, process.EventEmitter);

process.Promise.prototype.timeout = function(timeout) {
Expand Down Expand Up @@ -435,18 +436,18 @@ GLOBAL.setTimeout = function (callback, after) {
timer.addListener("timeout", callback);
timer.start(after, 0);
return timer;
}
};

GLOBAL.setInterval = function (callback, repeat) {
var timer = new process.Timer();
timer.addListener("timeout", callback);
timer.start(repeat, repeat);
return timer;
}
};

GLOBAL.clearTimeout = function (timer) {
timer.stop();
}
};

GLOBAL.clearInterval = GLOBAL.clearTimeout;

Expand Down Expand Up @@ -508,14 +509,14 @@ var posixModule = createInternalModule("posix", function (exports) {
} else {
promise.emitSuccess.apply(promise, arguments);
}
}
};
}

// Yes, the follow could be easily DRYed up but I provide the explicit
// list to make the arguments clear.

exports.close = function (fd) {
var promise = new process.Promise()
var promise = new process.Promise();
process.fs.close(fd, callback(promise));
return promise;
};
Expand All @@ -525,7 +526,7 @@ var posixModule = createInternalModule("posix", function (exports) {
};

exports.open = function (path, flags, mode) {
var promise = new process.Promise()
var promise = new process.Promise();
process.fs.open(path, flags, mode, callback(promise));
return promise;
};
Expand All @@ -535,7 +536,7 @@ var posixModule = createInternalModule("posix", function (exports) {
};

exports.read = function (fd, length, position, encoding) {
var promise = new process.Promise()
var promise = new process.Promise();
encoding = encoding || "binary";
process.fs.read(fd, length, position, encoding, callback(promise));
return promise;
Expand All @@ -547,7 +548,7 @@ var posixModule = createInternalModule("posix", function (exports) {
};

exports.write = function (fd, data, position, encoding) {
var promise = new process.Promise()
var promise = new process.Promise();
encoding = encoding || "binary";
process.fs.write(fd, data, position, encoding, callback(promise));
return promise;
Expand All @@ -559,7 +560,7 @@ var posixModule = createInternalModule("posix", function (exports) {
};

exports.rename = function (oldPath, newPath) {
var promise = new process.Promise()
var promise = new process.Promise();
process.fs.rename(oldPath, newPath, callback(promise));
return promise;
};
Expand All @@ -569,7 +570,7 @@ var posixModule = createInternalModule("posix", function (exports) {
};

exports.rmdir = function (path) {
var promise = new process.Promise()
var promise = new process.Promise();
process.fs.rmdir(path, callback(promise));
return promise;
};
Expand All @@ -579,7 +580,7 @@ var posixModule = createInternalModule("posix", function (exports) {
};

exports.mkdir = function (path, mode) {
var promise = new process.Promise()
var promise = new process.Promise();
process.fs.mkdir(path, mode, callback(promise));
return promise;
};
Expand All @@ -589,7 +590,7 @@ var posixModule = createInternalModule("posix", function (exports) {
};

exports.sendfile = function (outFd, inFd, inOffset, length) {
var promise = new process.Promise()
var promise = new process.Promise();
process.fs.sendfile(outFd, inFd, inOffset, length, callback(promise));
return promise;
};
Expand All @@ -599,7 +600,7 @@ var posixModule = createInternalModule("posix", function (exports) {
};

exports.readdir = function (path) {
var promise = new process.Promise()
var promise = new process.Promise();
process.fs.readdir(path, callback(promise));
return promise;
};
Expand All @@ -609,7 +610,7 @@ var posixModule = createInternalModule("posix", function (exports) {
};

exports.stat = function (path) {
var promise = new process.Promise()
var promise = new process.Promise();
process.fs.stat(path, callback(promise));
return promise;
};
Expand All @@ -619,7 +620,7 @@ var posixModule = createInternalModule("posix", function (exports) {
};

exports.unlink = function (path) {
var promise = new process.Promise()
var promise = new process.Promise();
process.fs.unlink(path, callback(promise));
return promise;
};
Expand Down Expand Up @@ -752,7 +753,7 @@ function findModulePath (id, dirs, callback) {
path.join(dir, id + ".js"),
path.join(dir, id + ".node"),
path.join(dir, id, "index.js"),
path.join(dir, id, "index.addon"),
path.join(dir, id, "index.addon")
];

var searchLocations = function() {
Expand All @@ -768,7 +769,7 @@ function findModulePath (id, dirs, callback) {
return;
}
searchLocations();
})
});
};
searchLocations();
}
Expand Down Expand Up @@ -933,7 +934,7 @@ if (process.ARGV[0].indexOf('/') > 0) {
process.ARGV[0] = path.join(cwd, process.ARGV[0]);
}

if (process.ARGV[1].charAt(0) != "/" && !/^http:\/\//.exec(process.ARGV[1])) {
if (process.ARGV[1].charAt(0) != "/" && !(/^http:\/\//).exec(process.ARGV[1])) {
process.ARGV[1] = path.join(cwd, process.ARGV[1]);
}

Expand Down

55 comments on commit 7873639

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah man :( why is everyone so semi-colon crazy, especially after a function literal :S

@creationix
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because that's the accepted JavaScript best practice. Sure it doesn't matter since V8 has good and reliable parser, but when doing client side work, they're essential. Especially for IE. It's just easier to keep the same strict style everywhere.

@creationix
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Besides, I use JSLint.org to check my code, and it helps find real bugs as well as petty formatting issues. It reduces the noise of the error report if you go ahead and follow the formatting rules. JavaScript is a very misunderstood language. For example, the rule in JSLint that says that only one var statement is allowed in a function seemed silly at first to me. Then after debugging some nasty issue in my TopCloud framework for a week, I found it was related to having a var statement late in a function body. JSLint would have saved me a week of pain and suffering.

@cloudhead
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yikes I cant stand JSLint, "best practices" if you dont understand the language perhaps it gives a good basis but its
no substitution for just learning a language. If anything all the braces / semi-colons just clutter the code making it less readable.
jQuery is probably the best example of this, great library, terrible conventions

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But perhaps you like Ruby's "end" as well ;) :D

@creationix
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it were just conventions, I might agree, but it's more. While those semicolons and braces on line ends are optional sometimes, this isn't always the case. Remembering the cases where they are needed and where they aren't needed can be terribly confusing. Not to mention that this changes a little across implementations.

Saying that JSLint is a crutch that keeps us from learning the language means you completely miss the point of JSLint. You shouldn't take any advice from the program unless you understand why it's telling you to do something. There are cases where some of the suggestions are wrong. It's a tool for people who do understand the language.

If you really learned and understood the language, you would understand why most of the JSLint "best practices" really matter. Sometimes Douglass Crockford goes a little too far towards strictness. JavaScript as a whole isn't that good a language, but there is a subset of it that's really nice and reliable. These best practices help up stick to that good subset.

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes but if you understand when they are needed then your good to go. I only really use them with anonymous self-calling functions that I do not want to be expressions.

Most of it is common sense stuff that people should be practicing regardless, like using var.. thats just obvious. JSLint bitches about dumb things like:

var foo
if (foo = something())

Even things like using 'with', there is nothing wrong with it if you know what your doing, Douglass just spreads these stupid conventions and now everyone is like OMG OMG semi-colons and braces everywhere

@creationix
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that the conventions are a bit strict and verbose, but that makes it easiest to understand for most people.

Javascript is a verbose language, get used to it. I think consistency is what matters most when talking about code conventions. Doug's stuff is good enough, especially if you know when to break the rules. I think it's a good idea for Node's core javascript to follow a common convention. And the widely accepted, and strict conventions from Doug are probably a good way to go. We can of course adjust and ignore the stuff that doesn't apply to us since we're not in the browser.

Also assignment expressions can be dangerous even though they're clever and concise. I know from personal experience. People aren't used to looking for side effects in expressions. Use them when it's best to do so. There are cases when using a with statement is the only way to accomplish something. It's fine to break the rules in these cases, but being sloppy about braces and semicolons just because it makes your code a few characters shorter isn't a good idea.

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exactly, but as far as im concerned if you cant understand js without lots of curlies then you shouldnt be contributing.
It does not have to be, you guys just dont utilize the grammar. I agree with consistency which is why none of my node
commits are in "my" style, and wont be.

What you really have to think is, if this person cannot even read my "advanced" js, then should he really be
contributing to my library? I would say no personally. Its like Drupal with PHP, PHP is such a crappy language
that Drupal gets tons of new developers contributing garbage that just flood the community making it difficult to
find a reliable module, which is why I stopped working on Drupal core and PHP all together.

It makes it more legible IMO, less eye travel, but I suppose people used to seeing braces everywhere will be all "holy whats going on here!!"

Compare my Sass(not finished) vs your Haml, do you really think swamps of braces are more readable? http://gist.github.com/259682

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nothing wrong with concise if you understand the lang tho. Like I wish JS has ||=, sure some people wont understand
but its alot better than:

options = options || {}
options ||= {}

likewise with &&= etc

@akahn
Copy link

@akahn akahn commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, paint it blue!
—defunkt

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope lol paint it red!! ; after } === fail

@cpojer
Copy link

@cpojer cpojer commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to jump in but lines 43-46 in haml.js are flawed because they only replace the first occurrence, no idea who that code belongs to, just quickly browsed over it.

@visionmedia: if (a) b, c else d, e is a horrible code style (I take it the second file in the gist belongs to you?). I'm all "holy whats going on here!!" not because I do not understand it, but because I consider this bad practice. Of course code style is subjective but you are basically ignoring general conventions most JavaScript code follows. If you do not want people to contribute because you think that your style is in some way elitist or better than what everyone else does you are instantly losing a broad base of possible contributors. I would not dare to use coding style as a way to raise the bar for contributions.

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should consider it, because you will get better contributions. Trust me once your used to it, things are much easier to read. I do understand that people are set in their ways, but hey someone has to break the mold :P

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cpojer
Copy link

@cpojer cpojer commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You just followed Rule A*. Good stuff.

Rule A
When running out of arguments, pull out random old code from someone, show it out of context and say bad things about it. Works all the time, I guarantee it.

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha well no seriously, you basically are saying that functional programming is a bad practice. Its not my fault 90% of
people who write JavaScript dont understand it, or dont understand it well enough. Like the people who dont even get ++foo foo++,
thanks to them we have to use += 1 in ruby lol

@noonat
Copy link

@noonat noonat commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can't expect everyone to know that nuances of JS just to be able to safely edit your code. I agree that it's prettier without them, but if you're trying to code defensively then semicolons are important. Most teams aren't full of people who understand JS to that depth, and JSLint validation saves those teams a ton of pain.

@brianmario
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used to not use semicolons in JS much, but ran into issues with various minification tools (namely yui-compressor) unless the code was "pristine" formatted JS.

That being said, it's almost never a bad thing to stick to a documented convention. Using a tool like JSLint means you'll have consistent JS formatting rules across your codebase which makes code-review and scanning for chunks of code infinitely easier IMO. It has nothing to do with not understanding the language (although it might for some).

A great example is the C code in MRI (ruby 1.8 branch). Inconsistent tab/spaces, indentation and other conventions make reading the code WAY harder. It doesn't hurt anyone to be consistent. In fact it helps everyone. If peppering an extra character around my code means my team will be able to work faster, and be able to use a broader range of tools then it's a win.

My 2 cents...

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed with the first statement, but since when are we minifying server-side code?

@brianmario
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well there's obviously no need to (except maybe a slightly faster parse time? :P)
But if everyone used the same conventions for JS, everyone can read it all the same. And if you don't need to minify server-side code, what's the problem with an extra character anyway? ;)

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because its ugly lol i consider code art, and I prefer to have functional code, less bugs

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Plus }; is not any more readable than } haha .... Im sure everyone agrees with that

@brianmario
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough. I prefer them, but can read code either way. To each-his-own I guess...

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup lol, I feel bad for starting a huge debate haha, just feel strongly about functional stuff.. and stuff that.. looks good

@brianmario
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't feel too bad, passion is the main ingredient in innovation ;)

@cpojer
Copy link

@cpojer cpojer commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brianmario: I like that.

@erichocean
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh, maybe this isn't the best time to bring up that I've refactored node to compile as an add-on to v8... :/

@creationix
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I coded for years in other languages with braces on new-lines instead of at the end of the line like is common in JS. When I started doing heavy javascript work I forced myself to switch to the new style. At first it felt ugly and wrong, but then with time I learned to actually prefer it.

I believe that it is good to like how the code you write looks, but it's perfectly possible to train our eyes to "like" something that follows conventions.

Besides, most everything in the Crockfordian conventions has technical roots, not based on taste or preference. Half those technical reasons aren't valid in the world of node, but it's common to write client-side code too on a node project and there it does matter.

It's just a matter of taste, I prefer to use the same style on both sides if possible. Also after many large JS projects, I've learned to like and enjoy the conventions. They are beautiful to me and I'd guess to many others.

@brianmario: innovation is awesome.

@tj
Copy link

@tj tj commented on 7873639 Dec 18, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got away from } else { blah blah after I was done with PHP. PHP is silly though not many things are expressions like you cant even do array_split(..)[0] or something, so I like to milk grammars for all they have lol

@cloudhead
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@visionmedia: what does functional programming have anything to do with including/omitting ; and {} ? and how does omitting them lead to less bugs? I'd definitely seeing as a potential source of bugs/mistakes, especially when multiple people are working on the same code.

@tj
Copy link

@tj tj commented on 7873639 Dec 19, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It depends but in some ways yes, because it promotes less or no state. ; has no place in server side javascript, with very few exceptions. }; is simply silly... the rest is tolerable

@sprsquish
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My reason for sticking with semi-colons: I write for both front and backend. I don't want to have to have to remember minor syntax differences just because I've switched from one to the other.

Arguments like these are why gofmt is such a great idea.

@raggi
Copy link

@raggi raggi commented on 7873639 Dec 21, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Um, how about patch the shitty JS parsers? Re-use them to write a decent minification tool that understands what an expression is, and what it isn't (so that people aren't writing syntax for a tool). The language has a damn standard, it's not like the meaning of a statement is actually vague.

You guys are arguing on the basis that your tools are a pile of crap, and you're fixing the symptoms not the bug. You're applying a "good practice" on the grounds that it's "good practice" to succumb to crap software. That's the worst kind of developer malpractice.

Typical internet argument, well done, all of you.

@creationix
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@raggi, The language does have a standard, it's terrible and specifies that the parsers implement these crazy rules. If you understood the history of JavaScript and how it was never really thought through or designed it would make sense.

Ryan has made it clear that he wants to use vanilla V8 Javascript, I think that's a good idea. The V8 guys have made it clear that they want to keep in sync with JavaScriptCore, and so basically, not following the spec, or even patching bugs in V8, if there were any, is not the place of node.

And to clear up what I mean by the spec specifying the crazy whitespace sensitive rules, here is the relevant part of the official spec.

https://gist.github.com/4658280967e5a1fba668

Basically it says that semicolons ARE NOT optional to be valid code, but parsers are required to implement a pre-parser that tries to guess where to put the semicolons. Hence, omitting them is both dangerous and slows down the parser because it has to "repair" your code first.

@raggi
Copy link

@raggi raggi commented on 7873639 Dec 21, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it says exactly that: "For convenience, however, such semicolons may be omitted from the source text in certain situations."

Pay very specific attention to the words "are described by". This does not mean "is literally replaced by, and should be written so in your code", it means "described as".

The examples are there to describe scenarios to the programmer. They're not there to say "the world is going to end if you do these things".

I don't see any evidence in the spec to agree with your opinion. I see how you could misunderstand the meaning of the spec, but I can't see how it says that the code is illegal, as it specifically says it's legal in the header, then "basically" it says "here's how you could think of how this feature works".

As for the V8 guys keeping with the ECMAScript spec, well, if the spec doesn't allow missing semi-colons, then scripts without them shouldn't compile.

If the grammar required them, it would require them. It does not require them. The language is defined by the grammar from the spec.

@raggi
Copy link

@raggi raggi commented on 7873639 Dec 21, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that this portion of the spec is misleading to the reader, and could be drastically improved. It's also fair to say that this is written like this for historical reasons. It is not fair to say that there is any good reason, particularly when writing server side scripts for modern ECMA-262 interpreters / compilers, to maintain "backward compatibility" for very old JavaScript interpreters in old web browsers.

@tj
Copy link

@tj tj commented on 7873639 Dec 21, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I agree if it was blasphemy then it would be flat out required in the grammar. I have written many a parser and its not so much that you "insert" separator tokens (newlines, semi-colons, etc), its just that you allow specific exceptions. The grammars are the one thing that seem to actually be consistent (mostly) between each interpreter, not that it matters in this case

@creationix
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not saying that anyone is evil for not putting semicolons in their javascript. They were made optional intentionally so that it's easier for beginners to learn the language. I'm just saying that it's better for us that know better to follow the grammar and not rely on hacks that were added into the language to help beginners. There are dangerous situations where it guesses wrong. While these situations happen a little less in a controlled environment like node, they still happen.

Also, as far as the technicality of my statement. From the spec:

"..is also not a valid ECMAScript sentence, but is transformed by automatic semicolon insertion into.."

Or in other words, the code missing semicolons does not follow the grammer, and so the pre-parser/parser tries to fix it. If the parser actually fixes the code in one pass or does it in a separate pre-parser pass doesn't matter. The point is that code missing semicolons is invalid and must be repaired before it can be executed. Either we can do the work and explicitly put the semicolons where we meant for them to go, or we can trust in the implementation to do the right thing. Since the spec tells implementations to basically "If you see an error, than back up and add a semicolon to see if it fixes the error", I don't trust it one bit.

You're welcome to do whatever style helps bring beautiful code to pass, but don't accuse others of being nazis for not trusting this terrible algorithm to insert the required semicolons for us.

@raggi
Copy link

@raggi raggi commented on 7873639 Dec 21, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a descriptive section of the spec. Does this not make sense?

Remember, they said "may be described as" not "must be written as".

If you like writing semi-colons, you like it; but please, do at least try to understand what I'm pointing out, the bit that you're referring to is not "'the only way' to implement a valid parser", it's a description of how developers may think about how the language deals with splitting up expressions. Inserting explicit separators is pointless, as the compiler already knows where it should insert them - that's the point. If it didn't know how to do this, it wouldn't be able to compile them. Specifically, this is discussed in the white-space section of the spec, where the spec clearly states that LineTerminators have special cases that have semantic meaning to expressions:

Like white space characters, line terminator characters are used to improve source text readability and to separate tokens (indivisible lexical units) from each other. However, unlike white space characters, line terminators have some influence over the behaviour of the syntactic grammar. In general, line terminators may occur between any two tokens, but there are a few places where they are forbidden by the syntactic grammar. Line terminators also affect the process of automatic semicolon insertion (7.9). A line terminator cannot occur within any token except a StringLiteral. Line terminators may only occur within a StringLiteral token as part of a LineContinuation.

You're wasting your own time, and by pushing this, you're wasting everyone else's time too. The compiler does it for you, by spec. The spec says it does. That's the way the language is, that's the way it's going to be from now on. By spec. Forevermore. You don't need semi-colon heavy code. That's just oldschool wastage.

The spec does not tell implementations to "fix errors" - you've got the idea wrong. I'm repeating myself one more time, for good measure: that's descriptive not literal. Implementations do not have to "fix" your code first, that's just a way for you to think about it. A generalised grammar can actually use syntactic elements, such as line terminators, to decide when expressions are terminated. It does not have any real requirement to "insert characters" into it's internal representation of the token stream in order to represent valid ECMAScript token streams.

Please try to understand the meaning of "described by" before posting back and referencing abstract descriptions as specification points, again. I didn't miss that entire section... In fact, I'm responding to it.

As for "easier for beginners", I don't believe that motivation, not one bit. As a non-beginner, I'd waste significant time and keystrokes on ";" characters, over the years, whilst a beginner would waste very little time and effort on it. It's more of an advantage to me than it is for any beginner. That is until I have to defend myself for doing something that the spec says quite clearly (outside of descriptive sections): "For convenience, however, such semicolons may be omitted from the source text in certain situations."

Furthermore, should the intent of the spec be for "advanced programmers" to obsessively insert semi-colons, I would have expected that to appear in this otherwise quite complete list of practical advice:

The resulting practical advice to ECMAScript programmers is: A postfix ++ or -- operator should appear on the same line as its operand. An Expression in a return or throw statement should start on the same line as the return or throw token. A Identifier in a break or continue statement should be on the same line as the break or continue token.

The only other "practical advice" that seems relevant is:

In the circumstance that an assignment statement must begin with a left parenthesis, it is a good idea for the programmer to provide an explicit semicolon at the end of the preceding statement rather than to rely on automatic semicolon insertion.

I don't see anywhere, any practical advice saying "programmers should use semi-colons to avoid broken language implementations or to work with old minification tools".

If you want to pull quotes out of the text out of context, and read between the lines, then try this one on for size:
"is not a valid sentence in the ECMAScript grammar, even with the automatic semicolon insertion rules"

See the "even with", one could, just as easily, interpret this to mean that that the grammar actually includes automatic semicolon insertion rules, nominally. Funny that, being that it's in the spec.

Regardless of whether or not "the grammar" (which is not strictly defined in this discussion as yet - or in the spec) does or doesn't (which is really an implementation detail) - the spec includes this stuff (which is what I'm referring to as "the grammar"). It's not something that's tacked on in an appendix, or an extension. It's in the spec. That means it's what the language is. There's no escaping that. Do you disagree that http://www.ecma-international.org/publications/files/ECMA-ST/ECMA-262.pdf is the canonical source for the definition of this language? Do you honestly believe that your mental concept is actually a truer representation of the specification of this language than the specification itself? I really don't get where you're coming from, apart from history.

I'm happy to be proven wrong, but I see this in the spec. I don't see the spec saying it doesn't include portions of itself. I don't see it claiming that beginners should do one thing, and non-begginners should do another. I don't see it claiming that "insertion should be performed" even. I see it saying that automatic semi-colon insertion is a good way to describe the manner in which expressions may be terminated. I also see it clearly stating that whitespace is syntactically relevant to the grammar, and should be treated as such by developers and implementors.

@erichocean
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move this discussion to the node.js mailing list or the IRC channel. It is no longer on-topic for this commit (assuming it ever was).

@raggi
Copy link

@raggi raggi commented on 7873639 Dec 21, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure, sadly, that's actually what was originally annoying me about the discussion. it's old, void, and wrong :-(

@andrewkolesnikov
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Holy sh*t, comments are brilliant.

@creationix
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, so to get back to my original point. Joshaven uses JSLint to clean up code. We've discussed this with Ryan quite a while ago, and he's fine with using it to clean up node's internal code. The official code conventions for node are posted at http://wiki.github.com/ry/node/contributing. While it doesn't specifically require semicolons, it does mention the other fixes that Ryan got straight from JSLint from what I presented to him.

I don't understand where all the hostility is coming from. It's just a simple fact that these are the conventions for node, and Joshaven't commit was accepted by Ry and pushed into the main code. I don't see why the big fuss.

@raggi
Copy link

@raggi raggi commented on 7873639 Dec 22, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You fail to understand the spec, and instead of saying "fair enough" you pretend you've been bullied, claiming hostility. Countering inconsistent, incomplete arguments based on assumptions made from the suggestions made by bad tools is not bullying, it's rational challenge.

The only hostility I have comes from the fact that you're still digging a hole and changing your tune on the way down. First it was a best practice (which actually, came from an aforementioned crappy tool, JSLint), then you claimed it was part of the spec (I corrected you on this, twice). Now you're telling me it's part of the conventions for node (however, that's missing from the documentation you referenced too).

I like the size of the crater you've dug, but would you please stop shouting in it, as it's starting to echo.

If this is now the official convention for node, then so be it, but it's a shame, given the heritage of the advice, and my parting comment on this discussion is merely to place my -1 that convention, before it's written into the projects guidelines. I'm sure there are others that feel the same way.

I'm out, Merry Christmas.

@mikekelly
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Grow up.

@creationix
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see this argument was already had elsewhere, but John Resig put an end to it. I respect John and Douglass Crockford, they've done a lot of good for JavaScript. I will comment no more as I now see this is simply flame bait. I apologize I didn't see sooner. http://github.com/jquery/jquery/commit/ddb86f8d5bd1bd21b2beeeea55baf505b47dfed5#comments

@quickredfox
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All I gotta say's we got passionate people here and I look forward to all your commits and releases... I guess this one wont go away without a " You $%#^ Semicolon Nazi* " and thus voila!

@tj
Copy link

@tj tj commented on 7873639 Dec 29, 2009

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont like Johns JS at all lol, he has the knowledge but its still meh.. and look at his PHP its terrible

@aheckmann
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This thread is hilarious.

@subtleGradient
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tldr. But I have to add my own opinion here too. ;)

When contributing to a shared project I follow the conventions of the project.

When working on my own personal stuff I choose to avoid unnecessary brackets and semicolons.
The rules for when they are required are extremely easy (for me) to remember.

Jslint is a very useful tool. I use different configuration files for each style of project that I work on. You can opt out of warnings for missing semicolons and stuff.

When working with other developers it's very important to agree on a style and stick to it. It's really not a big deal.
If you can't agree on a style, either convince everyone on the project to switch or quit the project. IMHO ofc ;)

@joshaven
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, I simply cleaned up some code and let Ry know... he liked the changes and was glad that he didn't have to fuss with doing it himself... and BOOM! every is talking about it. Are some people out there feeling guilty about the cleanliness of their code? I think I found a soft spot.

@c4milo
Copy link

@c4milo c4milo commented on 7873639 Jul 19, 2010

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 the code must remains simple, consistent, readable and maintainable. It's the only way we can avoid stupid bugs.

@joshaven
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good code is properly tested. The function of good code is self-evident. Problems in good code are easily found.

I completely agree that good code is simple, consistent, readable & maintainable but I am not sure that is concreate enough. I am not sure everyone would agree what good code looks like. I personally don't think that the placement of... (say, optional semicolons, etc)... make much difference.

Functions should be simple in style and process, variable names should be descriptive, data should be well structured, and most importantly everything should be well documented and tested.

Please sign in to comment.