Closed phikal closed 9 years ago
Looks good for the most part. A few things before I pull
endIndex
is a bit off. For starters, if there is no period in the sentence, an error will occur as endIndex
will then equal -1
, which will cause an error. Furthermore, by looking only for .jsml
, we avoid inadvertently messing up the name of stuff. A file named file.min
, for instance, will be changed to file.html
, which we don't want. I think it'd be better to output it as file.min.html
. So for starters, I'd revert the endIndex
assignmentisString(json)
with json.isAString
is problematic, as isAString
isn't a method. So if you could revert that, that would be awesome.Otherwise, everything looks good. Thanks!
The point isAString
is that I modified the prototype of the string
object, somewhere at the beginning of the file,to have this variable and
have it contain true. That means that all objects of string contain this,
and it's easier to detect than it was before. But if you really want to, I
can revert it.
There was a problem with the endIndex
implementation, and that was that it just always seemed to add .html
to the end of the file since input.length
( I belive ) always had a higher value than the first position of .jsml
. Math.min
could have worked, but I wrote it into one line, by just having a regex remove .jsml
and add .html
to the end. If there is no .jsml
, nothing will be removed and it will only add .html
. I hope that was what you wanted it to do.
Oh, got it. I didn't see the isAString
prototype the first time around. And the regex tweak you did should work just fine. Merging now. Thanks!
In this PR I:
I concidered lib/optparse.js stable enough for it not to be debugged, so I removed some redundant features and compressed it from ~300 lines to ~40. Absolutely not-readable by any means, but smaller.
Edit: lib/optparse.js used to have
11387
charachters, now it has4404
. That's about 2.5 times less. I think it's worth it.