dashbitco / nimble_options

A tiny library for validating and documenting high-level options. 💽
Apache License 2.0
507 stars 38 forks source link

:keyword_list and :nonempty_keyword_list should apply nested options defaults #51

Closed whatyouhide closed 4 years ago

whatyouhide commented 4 years ago

Today, I have this schema:

schema = [
  pool_options: [
    type: :keyword_list,
    default: [],
    keys: [
      protocol: [type: :atom, default: :http1],
      size: [type: :integer, default: 10]
    ]
  ]
]

However, if I validate [] I get this:

NimbleOptions.validate!([], schema)
#=> [pool_options: []]

when in reality I'd expect that the nested defaults are propagated up, having the expected behavior be:

NimbleOptions.validate!([], schema)
#=> [pool_options: [protocol: :http1, size: 10]]

@josevalim @msaraiva I can work on this if we agree it's the right call. Thoughts?

wojtekmach commented 4 years ago

+1 from me.

Btw, I think in this particular case the default: [] is misleading as that's not what we expect to happen, maybe we should warn or document it?

Worth mentioning that today we could set it to default: [protocol: :http1, size: 10] but it doesn't compose well, with the same schema as above with that minor change, we have:

iex> NimbleOptions.validate!([pool_options: [protocol: :http2]], schema)
[pool_options: [protocol: :http2]] # (no port!)

so again, +1 for the proposal.

msaraiva commented 4 years ago

@whatyouhide are you using the version on master? I believe this has been addressed in #50. More details can be found here https://github.com/dashbitco/nimble_options/issues/49#issuecomment-698406926.

Using master I get:

[
  pool_options: [
    type: :keyword_list,
    default: [],
    keys: [
      protocol: [type: :atom, default: :http1],
      size: [type: :integer, default: 10] 
    ]
  ]
]
NimbleOptions.validate!([], schema)
#=> [pool_options: [size: 10, protocol: :http1]]
whatyouhide commented 4 years ago

@msaraiva is right, this is fixed in master :)