Reformatting URI.js + Still passes all tests #21

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants

Passes JSLint with 1 exception - Unsafe character in RegExp, line 93.
Still passes all unit tests!

All code needs refactoring and commenting.
Seems to be a lot of code duplication - I may be able to help with this at some point.

LiamGoodacre added some commits Mar 24, 2012

Formatted URI.js
Now passes JSLint with 1 exception - Unsafe character in RegExp, line
93.
All code needs refactoring and commenting.
Seems to be a lot of code duplication - I may be able to help with this
at some point.
Formatting
URI.js reformatted.
Still passes all tests.
Owner

rodneyrehm commented Mar 25, 2012

I appreciate your effort - thanks!
It's difficult to see what code you actually changed due to the large amount of indentation changes, so it will take me (quite) a while to sift through this.

I am, however, no particular fan of two-space indentation, var i, length, key;, renaming variables ("undefined" to "undef") for no apparent (say "explained") reason, introducing new functions, etc…

The aim of my edits were to get it to pass JSLint. The main changes include:

Adding scoping containers, for example; lines 1088 to 1103 where "var q = p.query;" is only used by the function following it: "p.query = function (v, build) {...".

Shifting all variable declarations to the top of their scope and replacing the previous declaration with just an assignment (if necessary).

Other edits are for making JSLint happy, such has moving function building out of loops, for example; lines 170 to 183 where the function, now named "process", used to be within the loop at line 181. The "undefined" argument, was renamed to "undef", as JSLint argues if you try to name a variable with a reserved term.

Owner

rodneyrehm commented Mar 27, 2012

I'm not going to merge your request because it is violating my style of coding. I cherry-picked some of your changes and will include them in a later version. I thank you for pointing out some obvious flaws (function definitions in loops, for example).

I'm really happy about people trying to improve code of others. Generally I disapprove of changing code style (2 space indentation, really?) adhering to the absolutely insane jslint, etc. The Javascript Tools TextMate Bundle does one (some less insane) jslinting I can happily live with.

If you feel strongly about certain changes you've made, please point out why you've made them and how they improve anything. If a change is made merely to satisfy the jslint-beast, well, they're probably not worth it.

@rodneyrehm rodneyrehm closed this Mar 27, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment