hapipal / confidence

Dynamic, declarative configurations
Other
263 stars 43 forks source link

bad control chars when using confidence command line #23

Closed lloydbenson closed 10 years ago

lloydbenson commented 10 years ago

When doing a npm install confidence (latest version):

confidence@0.12.1 node_modules/confidence ├── hoek@1.5.2 ├── boom@2.5.1 (hoek@2.4.1) ├── optimist@0.6.1 (wordwrap@0.0.2, minimist@0.0.10) └── alce@1.0.0 (estraverse@1.3.2, esprima@1.0.4)

I took the example in the confidence readme.

$ cat -vt config.json { "key1": "abc", "key2": { "$filter": "system.env", "production": 1 } } when running node_modules/.bin/confidence --filter.system.env=qa -c config.json > result.json I get this:

$ cat -vt result.json { ^I"key1": "abc" }

Note the extra control character that wasn't in the original file. This causes issues when trying to load this into hapi later, because it is invalid json. I have a more complex setup that I use and I get the following error when its auto generated but that file has a ton of ^I chars as well:

[1] servers position 0 fails because client is required at Object.exports.assert (../node_modules/hoek/lib/index.js:425:11) at Object.exports.assert (../node_modules/hapi/lib/schema.js:15:10) at Function.internals.Pack.compose (../node_modules/hapi/lib/pack.js:699:12) at ../node_modules/hapi/lib/cli.js:112:19 at ../node_modules/hapi/lib/cli.js:94:9 at LOOP (fs.js:1358:14) at process._tickDomainCallback (node.js:459:13)

This seems to have broken for me sometime on August 13th. It worked fine in the morning but broke sometime in the afternoon if that is helpful. I suspect a dependency since I don't think the top level version was changed.

Note: I did get around the problem by adding a | tr -d '\t' before I write the file, and that lets it start up just fine.

kpdecker commented 10 years ago

Do you see the error with Hapi 6.5.1? That client message is a known issue in 6.5.0.

On Thu, Aug 14, 2014 at 9:49 AM, Lloyd Benson notifications@github.com wrote:

When doing a npm install confidence (latest version):

confidence@0.12.1 node_modules/confidence ├── hoek@1.5.2 ├── boom@2.5.1 (hoek@2.4.1) ├── optimist@0.6.1 (wordwrap@0.0.2, minimist@0.0.10) └── alce@1.0.0 (estraverse@1.3.2, esprima@1.0.4)

I took the example in the confidence readme.

$ cat -vt config.json { "key1": "abc", "key2": { "$filter": "system.env", "production": 1 } } when running node_modules/.bin/confidence --filter.system.env=qa -c config.json > result.json I get this:

$ cat -vt result.json { ^I"key1": "abc" }

Note the extra control character that wasn't in the original file. This causes issues when trying to load this into hapi later, because it is invalid json. I have a more complex setup that I use and I get the following error when its auto generated but that file has a ton of ^I chars as well:

[1] servers position 0 fails because client is required at Object.exports.assert (../node_modules/hoek/lib/index.js:425:11) at Object.exports.assert (../node_modules/hapi/lib/schema.js:15:10) at Function.internals.Pack.compose (../node_modules/hapi/lib/pack.js:699:12) at ../node_modules/hapi/lib/cli.js:112:19 at ../node_modules/hapi/lib/cli.js:94:9 at LOOP (fs.js:1358:14) at process._tickDomainCallback (node.js:459:13)

This seems to have broken for me sometime on August 13th. It worked fine in the morning but broke sometime in the afternoon if that is helpful. I suspect a dependency since I don't think the top level version was changed.

Note: I did get around the problem by adding a | tr -d '\t' before I write the file, and that lets it start up just fine.

— Reply to this email directly or view it on GitHub https://github.com/hapijs/confidence/issues/23.

lloydbenson commented 10 years ago

https://github.com/hapijs/confidence/pull/24 fixes the ^I char.

lloydbenson commented 10 years ago

Merged so closing this up.