dashbitco / nimble_options

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

Add `non_empty_string` type #107

Closed tcoopman closed 1 year ago

tcoopman commented 1 year ago

Seeing that: :non_empty_keyword_list, :pos_integer, :non_neg_integer exist, I think it would make sense to have a :non_empty_string as well?

What do you think?

josevalim commented 1 year ago

Hi @tcoopman , what is the use case? I only ask because with string the concept of empty may be a bit more complicated (for examples, do you want to be empty if it has no whitespaces?).

tcoopman commented 1 year ago

Yeah, I've thought about that. For me spaces are not allowed. So, right now I trim first and then check length.

To be fair I'm trying to use this library as a lighter validation library instead of using ecto. Maybe that's not intended.

My ideal case would probably be similar to what some ecto validations offer: type: {:string, min: 2, max: 1, format: ~r/.../}

josevalim commented 1 year ago

Yes, then I would say that's outside of the scope of this library. This is more for validating options than user input. :) Thanks!

moxley commented 1 year ago

Hi all, I'm working on a busy codebase that uses NimbleOptions. At first, the package seemed like a good fit for some new changes I am working on. It involves a function that optionally takes some options, and it falls back to application configuration values if the options aren't provided. Kind of like this:

def get_access_token(credentials \\ get_credentials()) do
  # API call to get an access token
  ...
end

defp get_credentials do
  config = Application.get_env(:my_app, __MODULE__, [])
  Keyword.get(config, :credentials, [])
end

In the application configuration for this codebase, there are lots of fields that have empty strings as values: secret: "". Those configurations get swapped out at boot time with real values. An empty string isn't a valid value, and NimbleOptions considers this a valid value even though it's empty and the option is marked required, so it won't work for this use case. I could go through all the instances of "" in the app config and replace them with nil, but that would require getting buy-off from the lead engineer on the project who established the practice of setting these config values to "", just to work around a limitation of NimbleOptions. It's unlikely that will go as desired, so I'll probably end up writing a wrapper around Ecto to make this work.

whatyouhide commented 1 year ago

writing a wrapper around Ecto to make this work

Do you mean around NimbleOptions?

In any case, you can still use required: true and then use a custom type to validate non-empty strings. Something like

defmodule MyApp.OptionsHelpers do
  def non_empty_string do
    {:custom, __MODULE__, :validate_non_empty_string, []}
  end

  def validate_non_empty_string(""), do: {:error, "expected non-empty"}
  def validate_non_empty_string(str) when is_binary(str), do: {:ok, str}
  def validate_non_empty_string(other), do: {:error, "expected string"}
end

And then use MyApp.OptionHelpers.non_empty_string() as the type.

moxley commented 1 year ago

In any case, you can still use required: true and then use a custom type to validate non-empty strings. Something like

I love this @whatyouhide!!! I will use this technique. Thank you.