Overall cleanup of lib.

[19h]

This fixes strict-warnings & errors.
This time, the source is understandable to at least Javascript beginners and is ready to go, again, breaks no tests which aren't already broken. This commit is required as new features of V8 require strict mode, which can't be, yet, given the unchanged lib-files, as they throw strict-warnings (no return of function, et al.) and strict-errors (redeclaration of variables).

In total, 19 files changed, there are 240 insertions. and 219 deletions.

Cheers, finally.

[ry]

We need to do perf tests before merging.

[tj]

this makes me die inside (returning null)

[tj]

loloololol

[bproctor]

Just a quick observation, but the second test of "tail" will always be true, so it could just be reduced to if (trailingSlash)

[thejh]

You know, I think you could save a few more chars by using || instead of && :D

[mmalecki]

Lets just change it into 140bytes competition.

[tj]

pull a _why and make everything as cryptic as possible, that's what good programmers do. job security ftw

[thejh]

(!tail && !isAbsolute) && (tail = '.');
tail||isAbsolute||tail='.';

Anything shorter?

[19h]

I'm sorry. Really discussing like that about 5, respectively 2, lines of code? I can restore it, if you really want to.

[aikar]

We told you not to do this kind of stuff in the last pull request :/

Sacrificing code readability to lower line count is not cool.

[mmalecki]

Sorry, WTF? These are changes for the sake of changes. You've broken every single JavaScript style guide, made source code ugly for no profit. And no, constructions you've used are not understandable for JS beginners.

Also, can you demonstrate (with a test case, preferably) where using those new features throws an error or an warning? Or where using them would make our code better (faster/more DRY)?

[19h]

Are you serious? This is a continuation of my previous pull-request. The last time I committed, more changes were needed, that is, the committers themselves corrected the issues. #1804 - and some eloquent words would be appropriate. "You've broken every single JavaScript style guide," - really? I don't know if you really are capable of JS in the name of ECMAScript 5, if not, please, just keep quiet. (see strict mode and the given link).

P.s.: I don't know what kind of people you are - in what environment do you get bitched by "developers" complaining about code, without constructively discussing it? This is getting crazy here - or you're just too decadent about people trying to keep up clean work. If you still don't get the code, please, get a real hobby.

Anyway, Cheers to Poland.

[tj]

You're right, we should discuss the code, which I think others have done in the previous commits. The reality many of these changes are very awkward, core definitely could use a cleanup but changing if statements to expr statements is not any cleaner, nor are weird ternary ops with parenthesized conditions for no reason. I avoid strict mode so I dont know if the return null thing is necessary or not, if it is then JavaScript just got a little more sucky.

[TooTallNate]

My quick little test shows that always including a return value is not necessary: https://gist.github.com/1294126

[thejh]

"You've broken every single JavaScript style guide," - really? I don't know if you really are capable of JS in the name of ECMAScript 5, if not, please, just keep quiet.

Do you know the difference between "style guide" and "language specification"? From what I've seen, you're saying: "What, one 1000-pages-book that consists of one sentence violates basic style rules? It is valid english, you know!"

[mmalecki]

"You've broken every single JavaScript style guide," - really? I don't know if you really are capable of JS in the name of ECMAScript 5

Oh. So, ES5 automagically makes > 100 char lines nice? Cool feature.

P.s.: I don't know what kind of people you are - in what environment do you get bitched by "developers" complaining about code, without constructively discussing it? This is getting crazy here - or you're just too decadent about people trying to keep up clean work.

Clean work? It's not a clean.

If you still don't get the code, please, get a real hobby.

Bozo bit flipped.

[thejh]

About style: Why the extra indent? Do you think it looks good to indent the lines you wrote more than the lines others wrote? That's kind of selfish, you know.

[19h]

When changing the code, first, I removed "if (key || char) {" and "}", then completed the "this.emit('keypress', char, key);" to the given return-statement. I didn't want to express anything else, though. Didn't see that, I'm sorry.

[thejh]

Btw, you forgot to "fix" src/node.js, I think. Protip: Don't bother. Bad idea anyway.

[19h]

@thejh It's clean and strict anyway, so that joke does not work.

[thejh]

@KenanSulayman it is? Oh, I'm sorry, it just looked to me like you wanted every function to return.

[koichik]

@KenanSulayman said:

Quick check @ specification of ECMAScript and yes, there it was.

Can you tell me where it is explained? I cannot find it.
ES5 (also ES5.1) "12.9 The return statement" does not have a "Strict Mode Restrictions" section.
And, "Annex C (informative) The Strict Mode of ECMAScript" does not explain return statement.

[19h]

As it seems, Komodo referenced the ECMAScript-specification in error. Anyway this is profilactic - a Mozilla-errorcheck explains, either returning nothing at all or if, in any case of the function flow something should be returned. (e.g. http://cl.ly/2N462X191l0m342O1f41)

[thejh]

Anyway this is profilactic - a Mozilla-errorcheck explains, either returning nothing at all or if, in any case of the function flow something should be returned.

Errrmmm... no. Not if you know that nobody will look at the return value of the function because it's async and return is only used for control flow (e.g. return cb(err)). Yes, it warns you for a reason. But just that someone says "you should normally avoid using eval" doesn't mean that you should never use it. In many cases, it's bad, yes. But as a programmer, you should be able to tell whether it is in the specific case. For example, my IRC bot has a !admin eval <code> command, and it uses eval(). But that's not by accident - I want to be able to manipulate variables in my modules scope.

So, if you just blindly follow the hints of an error checking tool, you aren't the programmer, you are the computer. Sorry.

[koichik]

@KenanSulayman - Neither Komodo nor Mozilla-errorcheck is ECMAScript's specification.
The specification is here.
Again, can you tell me where it is explained in the specification?

[koichik]

@KenanSulayman - Can you close this request and divide it into some categories?
example:

1. breaks specification (strict mode).
2. breaks new features of v8 (--harmony_xxx).
3. warnings from your tools (return, etc).
4. your opinion (conditional operator, etc).

Then, please send each requests.
Probably, I will +1 to 1 and 2.

And, Please remember @bnoordhuis's advice.

preferably multiple small ones - say one per module - instead of a single big patch.

[19h]

Sounds okay.

+Edit: What's the problem anyway? There're no functional changes. Anyway --harmonycan be called, and is fixed right now, as it seems. It's not that much change, that it'd require a split-off of code to be representable, in my opinion.

[thejh]

@KenanSulayman The problem is that your changed code doesn't look the way many people are used to. Your code doesn't respect existing conventions. It doesn't even care. It contains a bunch of changes that most of the people here think of as reducing the beautifulness of the code (to express it in a nice way). I think that you didn't yet give us any reason for why those ugly return nulls are needed or what using ternaries that seem to just make the lines longer is good for. That's why you should split up your pull request. Some changes might be needed, and others seem to be opinionated and bad-looking. Split them apart so that the good stuff can end up in core.

[yocontra]

The hostility in these comments is absurd. Take it down a notch, it's just code.

[aikar]

Keep this for Pull Req #1

[aikar]

I marked 5 lines (yes... only 5) that are actually needed and should be seperated as a new pull request.

I marked them with "Keep this for Pull Req #1"
If you really want, make a 2nd pull request for all the return/return null changes.

But I honestly do not think its worth the trouble unless you can demonstrate a performance improvement from it.
As commented above, its not part of any strict mode violations. If it has some positive benefit to the running of node, then fine, prove it and issue a pull request.

But if its purely for cosmetic purposes to not make komodo complain (and I use komodo too, I just ignore em :P), then its absolutely not necessary as future commits will not honor it and it will be pointless.

If a function is not expected to return anything, then theres no reason to put a return statement on everything.

Returning null vs returning undefined gains you nothing if your not using that result.

Id be willing to think it would actually be detrimental to performance to add them where return values are not expected.

[19h]

@thejh Well said, thank you. I will rethink the changes of the return-parts. Anyway, in my unaccepted, previous commit, which was taken down by @bnoordhuis, actually improved the overall-performance, as bitwise-operations are in any way faster. That is, I got used to implement bitwise-operations by C++, it's a pity I'm coding against some conventions.. but I think function > style. However, thanks', I'm going to fix my commit.

@contra Kind of. When I saw it, it scared the shit out of me. Seemed like Facebook of Coding with Punch-Button to me.

@aikar Thank you, really much! New pull-request on its way.

[bnoordhuis]

Closing. @aikar already pointed out the good bits, please submit those as a new pull request. Not interested in the 'return / return null' changes.