ogt / valid-url

Node module that provides URI validation functions
https://npmjs.org/package/valid-url
Other
215 stars 26 forks source link

Update index.js #3

Closed MykolaPrymak closed 11 years ago

MykolaPrymak commented 11 years ago

Realize the URI/HTTP/HTTPS/Web validation

MykolaPrymak commented 11 years ago

I know that other people ported this library, but take a look code.

ogt commented 11 years ago

Interesting, Did you run the test suite that sergei ported to see the extent to which using node's own url/util libraries the way you did, affects the functionality? If your code is having the same behavior as the perl-based - I would prefer to use yours - its more succint. (if this is so, ie it passes the tests, could you re-fork from current commit and re-send the PR )

I also liked that you added the node.js camel-case function names / aliased the perl ones. Nice touch.

odysseas

MykolaPrymak commented 11 years ago

I update the code to pass tests. Sergei did great job by porting the code.

2013/7/29 ogt notifications@github.com

Interesting, Did you run the test suite that sergei ported to see the extent to which using node's own url/util libraries the way you did, affects the functionality?

odysseas

— Reply to this email directly or view it on GitHubhttps://github.com/ogt/valid-url/pull/3#issuecomment-21748880 .

ogt commented 11 years ago

Great, thanks a lot. I merged in your changes. Do you mind applying at this odesk job - so as I can share there my good feedback?

Also, a couple more things for future reference:

ithaca:valid-url odysseas$ npm test

> valid-url@1.0.1 test /Users/odysseas/Dropbox/Home/code/projects/valid-url
> make test

node_modules/.bin/jshint index.js
index.js: line 43, col 9, Expected '}' to have an indentation at 13 instead at 9.
index.js: line 50, col 6, Missing semicolon.
index.js: line 6, col 13, 'util' is defined but never used.
index.js: line 15, col 31, 'decode_uri' is defined but never used.

4 errors
make: *** [lint] Error 2
npm ERR! Test failed.  See above for more details.
npm ERR! not ok code 0
ithaca:valid-url odysseas$ 
MykolaPrymak commented 11 years ago

About jshit error 'decode_uri' is defined but never used - this is "hack" - if you try to call decodeURI on non-uri string - you get a exception and then - return false; You can fix this by replace "return uri;" to "return (decodeURI(uri)) ? uri : false;" uitl has been used on testing purpose, and i forgot to remove it.

MykolaPrymak commented 11 years ago

In current realization the isURI not perform the real work. It return true on any string. Also thanks for the code review.