jvandemo / generator-angularjs-library

A generator for Yeoman to generate the boilerplate for creating an AngularJS library
117 stars 33 forks source link

don't prefix upper case libraryNames with - #33

Closed denisos closed 8 years ago

denisos commented 8 years ago

This change is to address issue/28. this._.dasherize() prefixes upper case libraryName with - . e.g. "My New Library" became -my-new-library. So my solution is to convert the libraryName to lowercase before calling dasherize.

jvandemo commented 8 years ago

@DenisOS — Awesome, will try out the changes and get back to you asap. Appreciate your effort!

jvandemo commented 8 years ago

@DenisOS — converting to lowercase here before dasherizing would always result a string without dashes, no?

Suppose:

original => tolowercase => dasherized

"projectOne" => "projectone" => "projectone" "ProjectOne" => "projectone" => "projectone"

I think you may need the decapitalize function (see here) so it would:

original => decapitalize => dasherized

"projectOne" => "projectOne" => "project-one" "ProjectOne" => "projectOne" => "project-one"

What do you think? Thanks!

denisos commented 8 years ago

Hi Jurgen, good catch. You're correct. projectOne becomes projectone. Let me try decapitalize and test it locally. If it works I'll push it. fyi by mistake I also included a change to the gulfile to watch and run unit tests when specs change, I think I should have created another fork for that change alone.

btw thanks for building this generator! Good blogs too.

jvandemo commented 8 years ago

@DenisOS — Thank you and don't worry about the Gulpfile changes. They are definitely a useful addition as well so feel free to keep them in this PR.

Thanks again for jumping on this, much appreciated!

denisos commented 8 years ago

Hi Jurgen, I think decapitalize is not present in the version used by this library. When I try to use decapitalize I get an undefined error. i.e. this fails dasherized: this..dasherize(this..decapitalize(props.libraryName)), but same line with slugify instead of decapitalize succeeds dasherized: this..dasherize(this..slugify(props.libraryName)),

I logged console.log(this._); and see dasherize and slugify but no decapitalize. From the console log, I noticed version for str as v2.4.0.

I checked the changelog for underscore string and v3.0.0 said "New functions decapitalize, pred, dedent and replaceAll" https://github.com/epeli/underscore.string/blob/master/CHANGELOG.markdown#300 It appears decapitalize() is not present in the library version we're using. :-(

Please double check.

If I'm right, then options include:

dasherized: this._.dasherize(props.libraryName.charAt(0).toLowerCase() + props.libraryName.slice(1)),

This is what the same as what underscore decapitalize does: https://github.com/epeli/underscore.string/blob/master/decapitalize.js.

jvandemo commented 8 years ago

@DenisOS — Great work!

I think the native string function option is fine as there may be breaking changes (as dictated by semver) between _ v2 and v3.

Thanks for all year efforts! :+1:

denisos commented 8 years ago

Hi Jurgen, no worries, I suspected a library upgrade could be a non trivial undertaking. I pushed my fix to use native js code to convert 1st char to lower.

jvandemo commented 8 years ago

@DenisOS — Fantastic, thank you for your contribution!

denisos commented 8 years ago

no problemo, my pleasure, thank you for sharing this generator Jurgen

jvandemo commented 8 years ago

Fixes #28.