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
Change standard validator behaviour w.r.t. nil/blank handling #37
Comments
I think we should simply remove the blank value checking from other validators. Unfortunately, this is problematic for |
I've been thinking about this for my own purposes, and aside from the macro idea, what if the {:blank-msgs {:user-name "can't be blank."}} then Or what if the non-nil-compatible validators swapped in a sane zero-value (e.g. a blank string) when faced with operating on nil? Then the checks could be removed as you suggest. Let me know if any of this interests you and if I get anywhere I'll put a PR together. |
I'm not inclined to modify The problem with non-presence validators not verifying |
To be more specific, What do you say? |
I'm not sure, to be honest. Imagining a slightly contrived web use-case with a
Maybe it's just my opinion but I don't think this is good for UX: "can't be blank." makes the rest superfluous. It's the kind of thing I'm used to seeing on ancient java-servlet-based webapps for multinationals/government. This is a contrived example that can be mostly be reduced to a single Then again maybe this library isn't made for me? I don't mean to be overly critical - the above is meant purely constructively. I'm very grateful for the work you guys put in. |
The only suggestion I have is to make standard validators besides |
@lorddoig I perhaps haven't made this clear but I'd be happy to review a pull request that does the changes per my comment above (your ideas are welcomed, too). I have little time to dedicate to Validateur but I agree that the current behaviour is not great and we should change it and slap a |
@michaelklishin sorry for the 3+ month delay. I'm using this library again and have a couple of ideas - I figure applying the validation functions monadically should allow us to return one error at a time for any given field, in the order the rules are defined, without reworking the existing code too much. I think the current behaviour could also be retained with this approach, and users could choose which they want. Monads and I are not the best of mates, but if I find the time I'll give it a go. |
@lorddoig thanks! :) |
With regard to the following example on this page.
This gives the impression that presence and format are separate validations when, in reality, the
format-of
test is also going to check for presence meaning both validations are pushing "can't be blank." into the errors set. This isn't noticeable in this example because of the use of sets: it only shows up once.However, if one wishes to make this change to the above example:
then this happens:
I honestly just spent the best part of 2 hours debugging this, before I realised I was debugging the wrong functions.
I'm scrolling through the source here and it seems like pretty much every
-of
function has anallow-blank
orallow-nil
option that defaults to false and has it's own, baked in default forblank-message
of "can't be blank." So if you wish to combine them with a non-defaultblank-message
you either have to a) setallow-blank
to true on all but one (which both obscures intent and raises the probability of a refactoring disaster); or b) duplicate your desired blank message across them all.The
-of
methods returning function closures limits the options for solving this a bit - the only thing I can think of is makingvalidation-set
a macro that looks forpresence-of
calls and rewrites the other functions as necessary, but this seems uber-kludgy. Sorry for being all problems and no solutions.The text was updated successfully, but these errors were encountered: