add non_whitespace? predicate

[george-carlin]

See https://github.com/dry-rb/dry-validation/issues/213

This is such a common use case (testing that a string is not only non-empty but contains non-whitespace characters, à la String#present? if you're using ActiveSupport) that I feel it makes perfect sense to include as a 'core' predicate.

However, I don't think filled_str? (which @solnic suggested in https://github.com/dry-rb/dry-validation/issues/213) is a good name, mainly because it uses the word "filled" in a way that's inconsistent with the existing filled? method: str?("") is true, filled?("") is true, but filled_str?("") wouldn't be true, which doesn't make sense. non_whitespace? is the clearest name I could come up with... although there might be something better.

Note that if you pass an object where the concept 'non whitespace' doesn't make sense (e.g. an Array or a Hash), this predicate will raise an error. (This is only true because I've written it like "regex =~ input"; if it was "input =~ regex" then non-stringy objects would just return nil.) I feel like raising an error makes more sense since if you're calling non_whitespace on an Array or an Integer etc. you're probably doing something wrong somewhere.

Is raising an error the right approach? Some other predicates already do it, e.g. key? will raise an error if passed an object that doesn't respond to key?.

[george-carlin]

To follow up on the error-raising thing: maybe it would be better to explicitly raise a custom error that says something like "non_whitespace? predicate not supported for #{input}". Otherwise you'll get a cryptic "no implicit conversion of (whatever) into String" TypeError, where it's not clear at all that non_whitespace? is what's causing the problem.

[fran-worley]

I like this idea, and agree that it should be a core predicate but am not convinced by the name. Non_whitespace isn't particularly good English and no_whitespace doesn't really explain the purpose as this predicate is testing to make sure a string is more than just whitespace rather than testing whether there is any whitespace in the string.

Could we make it a special type of string? Otherwise you end up having to test that it's a string and then if it's got more than whitespace.

I'll have a think and try and make some alternative name suggestions! As I say I love the idea!

[fran-worley]

Done some googling and have some suggestions:

1. printable or visible characters or chars for short
2. not_blank, not_empty

[fran-worley]

Thinking aloud... Could we maybe have a special custom type which strips right (and left?) whitespace ? That would mean that using the filled predicate would work as it is when using the special type.

[george-carlin]

I don't like not_empty because it contradicts Ruby's own String#empty: !" ".empty? returns true but not_empty(" ") would return false.

@solnic already discussed adding a 'strip string' type here... I'm not familiar with how to use dry-types (I'm still very new to the whole dry-rb ecosystem) so I can't comment.

I still think that this predicate should be available without having to worry about custom types, since it's such a common use case. You might want to validate that the string is present (to use "present" the way activesupport uses it) without actually coercing the input.

On another note, do you think it's a good idea to raise an error if the question "does it contain non-whitespace characters?" makes no sense for the given input? (I think the only inputs that would make sense are strings and symbols, am I missing anything?) Or should it return false in this case? I think it depends on the final name that's decided for the predicate and what feels intuitively right based on the name.

[george-carlin]

How about non_blank? (or not_blank?)? Semantically, I think that "blank" makes sense as a word meaning "not whitespace", and this would avoid confusion/conflict with the way dry-v and dry-logic are already using terms like 'empty'.

This is also consistent with how ActiveSupport uses the term 'blank' in String#blank?. (I share @solnic's distaste for ActiveSupport, but since 99% of people learning dry-rb will be coming here from Rails, I reckon it's worth trying not to confuse them too much.)

[solnic]

I'm having some second thoughts on this. Don't you think we should sanitize input explicitly instead of having more complex predicates that would match regexps? (regexps matching is dead slow btw).

[backus]

Don't you think we should sanitize input explicitly instead of having more complex predicates that would match regexps?

In my original use case (why I opened #213) we are using dry-v for validating and rejecting invalid inputs. Dry validation is helping us sanitize by rejecting bad values. What is the alternative?

[solnic]

In my original use case (why I opened #213) we are using dry-v for validating and rejecting invalid inputs. Dry validation is helping us sanitize by rejecting bad values. What is the alternative?

I just started thinking that we should strip whitespaces before validating, then you don't need any regex matching

[backus]

@solnic I think that would make life harder for us as dry-v users. For example, we have plenty of other custom predicates which need to make sure there is no whitespace in the input string.

[fran-worley]

I definitely favour having a custom type of text or stripped string rather than an ever more complex reflex matching predicate.

If the special type was registered in dry validation you could do something like:

required(:key).filled(:text?)
Or
required(:key).filled(:stripped_string?)

This approach serves two goals:

1. We can validate non whitespace without needin a new predicate;
2. We actually cleanup the input by stripping the white space which stops the need for processing after validation, or things like nilify.

[george-carlin]

So it seems there are two possible approaches:

1 - strip the string before validation:

# e.g. in a Reform object:
property :name, type: Types::Stripped::String

validation do
  required(:name).filled
end

2 - pass the string to dry-v as-is and validate with a new predicate.

property :name, type: Types::String

validation do
  required(:name).filled(:text?) # or whatever the name would be
end

Are you saying that you'd rather add 1)?

I gave an example of how the Types::Stripped::String could be implemented here, although I'm not sure I have the best approach. (And in retrospect I'm not sure Types::Stripped::String is the best name, as it implies that there's a whole category of types called Stripped when in fact this 'category' only has one entry. Maybe Types::Coercible::StrippedString? The Coercible namespace seems like the obvious place to put it.)

[fran-worley]

@georgemillo. Option 1 is my preference (though stripped can't be the namespace). I think this Type should be a standard anyway to cleanup strings with accidentally added whitespace.

I think it's better off under the Form (and JSON) namespace as Coercible is for Kernal based coercion.

[solnic]

Thanks for all your input but I'm closing this because: #42 (comment)