mycoboco / wcwidth.js

a javascript porting of C's wcwidth()
http://code.woong.org/wcwidth.js
Other
12 stars 2 forks source link

Added you as a collaborator #8

Closed timoxley closed 9 years ago

timoxley commented 10 years ago

I wanted to make a few significant changes to wcwidth to bring it on par with typical node modules.

I've performed these changes over here:

https://github.com/timoxley/wcwidth

Let me make it clear, I'm not trying to steal your work at all, just needed control to make necessary improvements if I'm going to depend on this module in columnify:

I'm happy drop my changes in as a commit into this repo and remove mine if you add me as a collaborator and are happy with the changes I've made. Feedback/suggestions welcome. Also could do with more test cases.

mycoboco commented 10 years ago

I'm busy these days for other projects so it would be helpful for you to join as a collaborator. The modified version on your local copy looks good, and I have some suggestions:

I'm looking forward hearing from you.

timoxley commented 10 years ago

the if statement and its conditional has a separate line

I'll meet you half-way: we can do your if statement thing if you avoid adding unnecessary semicolons.

there seems no way to set an option globally

wcwidth.config returns a function.

mycoboco commented 10 years ago

Seeing your mentioning of unnecessary semicolons, I checked some other node.js projects and found they also tend not to use them. IIRC, Crockford's book (that is the only book I read before starting to code in JavaScript) says against omitting semicolons due to JavsScript's unintuitive rule for auto-insertion of semicolons. Have dropping semicolons constituted a coding style for node.js projects? I'm asking this not because I still want to use terminating semicolons but because it's just unfamiliar style to me since I've spent most of my time with C. I agree with your proposal; we can avoid unnecessary semicolons with my if statement thing.

With regard to the global option thing, my hasty scan of your code missed that wcwidth.config returns a function with a closure. Sorry. Your approach has no problem.

Okay, now I'm happy with your changes, and as you suggested, I'll drop .js from the project name. For now, I want to leave the documentation files (README.md, INSTALL.md and so on) as they are because my personal pages are periodically machine-generated from them and I'll modify them altogether soon. After that, I think we can release a new version.

If you have any suggestions, please let me know.

I deeply appreciate you for contribution to the project. This is a good chance for me to learn about node.js projects and I'm going to apply what I learned from you to my other JavaScript projects. :-)

mycoboco commented 10 years ago

My sincere apologies for my late response to this issue. I've been extremely busy due to my project at work.

Your proposed changes are reflected in 9cda0b41e990c671b94f7f0abaed686ba0c05cc3 almost unchanged except that:

I added your name and a link to github page on readme and license; I hope you don't mind it.

If you agree with these changes, I'll publish this as version 1.0.0.

mycoboco commented 9 years ago

Seemed that no more interest was shown on this issue, I've decided to release 26d6bb6e1e5f50152d683b6f806351dea61868e6 as v1.0.0 with getting the project name back to wcwidth.js by myself.