Repository navigation
Add information about violations to error message - #554
Conversation
|
Seems reasonable to me once y'all agree in bufbuild/protovalidate#536 :) |
| violation.message.as_deref().unwrap_or_default(), | ||
| violation.rule_id.as_deref().unwrap_or_default(), | ||
| ) { | ||
| ("", "") => f.write_str("[unknown]"), |
There was a problem hiding this comment.
if both are present, why not display both?
There was a problem hiding this comment.
This was to apply the change made to Go in bufbuild/protovalidate-go#291 (comment) here. I don't feel too strongly either way but it was suggested in bufbuild/protovalidate#536, lean towards going with it
| impl fmt::Display for ValidationError { | ||
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| f.write_str("validation failed") | ||
| write_violation(f, &self.violations[0])?; |
There was a problem hiding this comment.
maybe support a # flag to display all violation messages.
There was a problem hiding this comment.
This is a great idea for rust! Python doesn't have anything like that (just the thrown exception), so let's keep this as-is for now and possibly add it in the future if supporting rust itself to prevent dead code here
jonbodner-buf
left a comment
jonbodner-buf
left a comment
There was a problem hiding this comment.
Some additional ideas we should consider standardizing
|
@jonbodner-buf thanks! I think on bufbuild/protovalidate#536 we are quite close to settling on doing minimal changes there. So if what we have here seems ok, would like to go with it |
|
Approved it. |
This follows the same style as protovalidate-es, one violation in the message and a count if there are more than one.
Fixes #546
/cc @stefanvanburen