i18n

[jprochazk]

• Closes Custom error messages and localization #7

Adds an API for customizing error messages. This can be used to both customize the defaults in english and also as a way to translate error messages. The default implementation uses the same error messages as before.

The core trait is garde::I18n, which contains one method for each error path in each rule. An implementation of this trait is installed via garde::with_i18n.

use garde::{Validate, i18n::{I18n, with_i18n}};

struct Czech;

impl I18n for Czech {
    fn length_lower_than(&self, min: usize) -> String {
        format!("musí obsahovat alespoň {min} znaků")
    }

    fn email_invalid(&self, _error: &str) -> String {
        format!("email je neplatný")
    }

    // etc.
}

#[derive(Validate)]
struct User {
    #[garde(length(min = 3))]
    name: String,
    #[garde(email)]
    email: String,
}

let user = User {
    name: "Jan Novák".to_string(),
    email: "invalid-email".to_string(),
};

let result = with_i18n(Czech, || user.validate());

It is possible for the handler passed to with_i18n to hold state, which allows for patterns like storing translations in an external file (e.g. fluent), and running arbitrary code within each error message factory method.

[simlay]

This is a very thorough PR. I am impressed. I reviewed it mostly because reviewing is a good way to learn and to show some appreciation.

Anyway, I hope you don't mind a drive-by review.

[simlay]

It's weird to me that imports go after module docstrings.

[siblingsofthevoid]

I mean if you think about it, it makes quite a lot of sense. The module docstring documents essentially the current file.

The imports then are part of that file
and after that you get additional module declarations.

If you think about this a bit reverse, putting a docstring on a mod some_module; instead of into its mod.rs (or some_module.rs) basically acts the same.

e.g. this in main.rs or lib.rs is just as valid

/// this does a thing
mod some_thing;

as this in some_thing.rs

//! this does a thing

// code and imports here

The only time where this can't be done is the entry
point, since the entry point has no super module. And then it's just easier to allow either way in all cases than to only allow it in the entry point. The latter option is usually preferred, since the docs for a module can end up being really long. That way it doesn't clog up your super module with documentation that has nothing to do with its functionality.

[simlay]

Not like it matters much but throwing this into an MSRV subsection on the readme is pretty common.

[siblingsofthevoid]

Since I would personally love to see this feature implemented, I will leave a review later. After a quick look i do have a few thoughts about improving the default english error messages, but I'll leave a proper review in the evening.

[siblingsofthevoid]

Apologies, it seems that "evening" became a few more weeks. Luckily, those weeks gave me a lot of time to think, so here it is.

I like the direction this PR is taking. The idea is really cool and I'd love to see it come out.

Some of the things I am suggesting here, may be out of scope for this PR, but may be worth looking at in another PR. Other things, however, are a bit larger, like the default error messages, which I often find too vague or too "my mom wouldn't know what to do with that". For those errors, I gave more clear examples that would improve the user experience if one were to simply use the default errors with no translation or anything else applied.

All in all It's a very solid PR and I genuinely like how simple the i18n trait is to use.

[siblingsofthevoid]

Same here, I think it is better to tell the user what alphanumeric means.

Suggested change

	fn alphanumeric_invalid(&self) -> String {
	"not alphanumeric".to_owned()
	}
	fn alphanumeric_invalid(&self) -> String {
	"Only letters and numbers are allowed".to_owned()
	}

[siblingsofthevoid]

I think defining what characters are in violation would be better.

Suggested change

	fn ascii_invalid(&self) -> String {
	"not ascii".to_owned()
	}
	fn ascii_invalid(&self, invalid_chars: &[&str]) -> String {
	format!("Only letters, numbers and special characters (! \" # $ % & \\ ' ( ) * + , - . / @ ] ^ _ ` { | } ~ : ; < = > ?) are allowed.")
	}

Technically ascii control characters are also allowed, but users don't know about those.

[siblingsofthevoid]

Same here, The user can see that the field is unset, why not tell them that it is required, to make it more clear WHY we want them to change that.

Suggested change

	fn required_not_set(&self) -> String {
	"not set".to_owned()
	}
	fn required_not_set(&self) -> String {
	format!("Field is required.")
	}

[siblingsofthevoid]

As with the other one tell the user what to do, instead of what is wrong.

Suggested change

	fn suffix_missing(&self, pattern: &str) -> String {
	format!("does not end with \"{pattern}\"")
	}
	fn suffix_missing(&self, pattern: &str) -> String {
	format!("should end with \"{pattern}\"")
	}

[siblingsofthevoid]

Suggested change

	fn matches_field_mismatch(&self, field: &str) -> String {
	format!("does not match {field} field")
	}
	fn matches_field_mismatch(&self, field: &str) -> String {
	format!("Should match {field} field")
	}

[siblingsofthevoid]

same as suffix_missing

Suggested change

	fn prefix_missing(&self, pattern: &str) -> String {
	format!("value does not begin with \"{pattern}\"")
	}
	fn prefix_missing(&self, pattern: &str) -> String {
	format!("Should start with \"{pattern}\"")
	}

[siblingsofthevoid]

I think these could be a much nicer error if they were either combined or another error was triggered if BOTH min and max were set.

Suggested change

	fn range_lower_than(&self, min: &str) -> String {
	format!("lower than {min}")
	}
	fn range_greater_than(&self, max: &str) -> String {
	format!("greater than {max}")
	}
	fn range_lower_than(&self, min: &str) -> String {
	format!("Should be at least {min}.")
	}
	fn range_greater_than(&self, max: &str) -> String {
	format!("Should be at most {max}")
	}
	fn range_error(&self, min: &str, max &str) -> String {
	format!("Should be a number between {min} and {max}")
	}

[siblingsofthevoid]

I honestly think this error should differentiate between lists and strings.
In that case we could have Way nicer errors here:

Suggested change

	fn length_lower_than(&self, min: usize) -> String {
	format!("length is lower than {min}")
	}
	fn length_greater_than(&self, max: usize) -> String {
	format!("length is greater than {max}")
	}
	fn string_length_lower_than(&self, min: usize) -> String {
	format!("Should be at least {min} characters long.")
	}
	fn string_length_greater_than(&self, max: usize) -> String {
	format!("Should be at most {max} characters long")
	}
	fn list_length_lower_than(&self, min: usize) -> String {
	format!("Should contain at least {min} elements.")
	}
	fn list_length_greater_than(&self, max: usize) -> String {
	format!("Should contain at most {max} elements")
	}

[siblingsofthevoid]

I think all Errors should also have the field's name/path passed in. You may ask why one should be doing that, but the answer is pretty simple:

Error keys for fields that may need context. error-username-shortand error-password-short could be entirely different wordings. As you can see from the rest of the ideas i had with errors, they are usually very specific and for users not to get confused, that's a good thing. Thus we must know what error-key to return (or what error to build based on said key).

This is why I think that the field's name should be passed into every function.That way error messages can be built using, for example, fluent or another translation framework. This would be especially useful for SSR applications (as rare as they are)