Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixes the init.author.* settings when doing npm init #3379

Closed
wants to merge 1 commit into from

3 participants

@mixu

The init.author.* settings in .npmrc were broken (ignored when running npm init).

This fixes the issue by pulling in the config options explicitly since I couldn't find a config-chain method to do that - npm.config.get() without params returns undefined which is why this broke.

Bump up init-package-json as well, since it was out of date, 0.0.7 was released 6 months ago, cf: https://github.com/isaacs/init-package-json

@mixu mixu npm.config.get() without params returns undefined which breaks the in…
…it.author.* settings in .npmrc. Pull in the config options explicitly and bump up init-package-json since it was outdated
520ee8a
@mixu

So you need something like this in ~/.npmrc to try this out:

 init.author.name = Mikito Takada
 init.author.email = foo@bar.com
 init.author.url = http://mixu.net/
@mfncooper
Collaborator

The functionality is definitely broken, but yipes, you're reaching deep into the implementation details of npmconf from inside an npm command, which seems pretty nasty to me. I'd say that a better solution would be to have init-package-json accept either a plain-jane object or an npmconf object, in the same way that npm-registry-client does, and then have it always use a getter to obtain the values, again as npm-registry-client does. However, it's @isaacs call as to whether it would be acceptable to have init-package-json know about npmconf in this way (or at least an object with a getter).

@mixu

Yes, there are three choices, either:

  1. reach into the implementation details (e.g. similar to lib/config.js) or
  2. make PromZard/init-package-json aware of npmconf objects/objects that have getters or
  3. add a getAll() / list() method to npmconf/config-chain which serializes the config values to json

since any of those can be acceptable and the choice depends on the maintainer, I just did the fastest thing that illustrates the solution and solves the issue. @isaacs what's your preference?

@isaacs
Owner

I think maybe promzard needs to be a bit more powerful than it is. It's a clever cute little thing, for better and mostly for worse. Often (as in this case) you want to be able to do some things that it simply can't do today, because it's cute rather than functional.

This is an ok patch, in that it fixes a bug with a pretty small amount of ugly kludging. A comment would be nice, but meh. I'm ok with landing it if someone else wants to, otherwise it's on my list for inclusion in the next npm release.

@mfncooper
Collaborator

@isaacs my feeling is that, even if they're already exposed in some places, we shouldn't be increasing the exposure of npmconf internals where that's easily avoidable. Since it would be much cleaner and simpler to use a 'get' wrapper like this, why not go down that path instead, and then we just pass npm.config to init-package-json and we're done. This doesn't require any changes to PromZard; it just means using config.get('foo') instead of config['foo'] in init-package-json.

@isaacs
Owner

@mfncooper That's fair. Pull req?

@mfncooper
Collaborator
@isaacs
Owner

Landed @mfncooper's changes. Thanks for bringing it up, @mixu! :)

I'd forgotten how nice it was for npm init to automatically drop my name and email address in there :) Thanks guys!

@isaacs isaacs closed this
@mixu

Awesome, glad to get this fixed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 26, 2013
  1. @mixu

    npm.config.get() without params returns undefined which breaks the in…

    mixu authored
    …it.author.* settings in .npmrc. Pull in the config options explicitly and bump up init-package-json since it was outdated
This page is out of date. Refresh to see the latest.
Showing with 9 additions and 2 deletions.
  1. +8 −1 lib/init.js
  2. +1 −1  package.json
View
9 lib/init.js
@@ -27,7 +27,14 @@ function init (args, cb) {
,"Press ^C at any time to quit."
].join("\n"))
- initJson(dir, initFile, npm.config.get(), function (er, data) {
+ var config = npm.config.list.reduce(function(prev, list) {
+ return prev.concat(Object.keys(list))
+ }, []).reduce(function (s, k) {
+ s[k] = npm.config.get(k)
+ return s
+ }, {})
+
+ initJson(dir, initFile, config, function (er, data) {
log.resume()
log.silly('package data', data)
log.info('init', 'written successfully')
View
2  package.json
@@ -62,7 +62,7 @@
"read-package-json": "~0.3.0",
"read-installed": "0",
"glob": "~3.1.21",
- "init-package-json": "0.0.6",
+ "init-package-json": "0.0.7",
"osenv": "0",
"lockfile": "~0.3.0",
"retry": "~0.6.0",
Something went wrong with that request. Please try again.