Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Turn off dot-notation for .config only? #1305

Open
f0zi opened this issue Feb 21, 2019 · 8 comments · May be fixed by yargs/yargs-parser#459
Open

Turn off dot-notation for .config only? #1305

f0zi opened this issue Feb 21, 2019 · 8 comments · May be fixed by yargs/yargs-parser#459
Labels

Comments

@f0zi
Copy link

f0zi commented Feb 21, 2019

Hi, is there a way to selectively turn off dot-notation for .config()?

I have something like this:

var args = require('yargs')
.config('config', 'Configuration file', filename => { // < I actually load a yaml file here
    server: { port: 8080 }
    members: {
        "bar.foo.net": {}
        "baz.org": {}
    }
})
.argv

I would still like --server.port to work, but I also need members to be enumerated and accessible the way I wrote them above.

Right now I have a workaround: In the config file I replace the dots in keys with something else (degree symbol) and undo the replacement on the argv object. I'm wondering if there is a better way to do this.

@SkReD
Copy link

SkReD commented Jul 15, 2019

Also hit by this problem when try to use npm modules version as keys in configuration file

@bcoe bcoe added the question label Jul 15, 2019
@bcoe
Copy link
Member

bcoe commented Jul 15, 2019

@SkReD @f0zi does turning off dot-notation completely not do the trick for you? I'm worried about the number of permutations it would add to configuration if we start allowing it to be configured on a feature by feature basis.

@f0zi
Copy link
Author

f0zi commented Jul 16, 2019

It would, but that's not what I would like it to do because I would still like the dot notation to work for command line arguments. It's only in the config file that it breaks when there are dots in keys, as in

{
    "server": {
        "port": 9000
    },
    {
        "www.example.com": {
            "more": "settings"
    }
}

... where I would still like --server.port to work without the dot notation breaking the "www.example.com" key up in three levels.

I don't mind if addressing the URL key from the command line does not work / does not do the right thing.

@bcoe
Copy link
Member

bcoe commented Jul 16, 2019

@f0zi what if we properly handled dot notation in config, but allowed this to be turned off?

or, we do handle it properly right now? or is it buggy.

@f0zi
Copy link
Author

f0zi commented Jul 16, 2019

Yes, that's what I was asking for in this thread. Also, I don't need it but I actually think it would be useful to set the character you split on, e.g. maybe have an option that is either a character or a bool (off).

I think it works as intended, but I'm not sure what the intention was and I don't think I agree with it. To me it's hard to justify changing the layout of a tree that is given to you based on some character in a key name. While I can see how it can be useful in certain circumstances, IMHO it should not be enabled by default. If someone wanted that feature, they could do it beforehand/afterwards, but if one does not want that feature and it's impossible to turn off then the only solution is to work around it like I explained in my top post.

Also, disclaimer, the product I needed this for is released and I might not actually change it even if you fix this. I do think that it would improve this otherwise great library if you would fix this though. :)

@bcoe
Copy link
Member

bcoe commented Jul 16, 2019

@f0zi I tend to agree with you about the API surface, I doubt that many folks are doing this:

{
  "foo.bar": "hello"
}

and expecting:

{foo: {bar: 'hello}}

Perhaps your original suggestion is the best route to take.

@bcoe
Copy link
Member

bcoe commented Jul 16, 2019

feel like submitting a patch adding config-dot-notation with a default value of false, and we'll make it a major?

@f0zi
Copy link
Author

f0zi commented Jul 21, 2019

I can try, I'm not sure if I'm up to it time-wise though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants