Just bikeshadding about the name: what about Charset?

I like Charset very much! I find it clearer. I updated the PR. I kept theencodings constraint option name to match the name of themb_detect_encoding() function argument.

My thoughts: On all my apps, I'd want to reject any non-UTF-8 content. It might already be the case when the DB refuses to persist invalid UTF-8. So this contraints looks both needed and not needed since I think we might want to add this check earlier when processing the input.
Does that make sense?

@alexandre-daubois can you share your use case more precisely? When can one submit non-UTF-8 in your case?

Thank you for the review, comments are addressed 👍

final suits me fine, added 👍

Thank you@alexandre-daubois.

Hi@alexandre-daubois

There is a small problem in the implementation, i poke you there but if you prefer i can create a full issue

As you use the "valid" encodings in your call to mb_detect_encodings, if the value isnot encoded in one of those values, the function will always return false.

Meaning you'll never be able to display anything in the {{detected}} value of the message.

if (!\in_array($detected = mb_detect_encoding($value, $constraint->encodings, true), (array) $constraint->encodings, true)) {    $this->context->buildViolation($constraint->message)        ->setParameter('{{ detected }}', $detected)        ->setParameter('{{ encodings }}', implode(', ', $constraint->encodings))        ->setCode(Charset::BAD_ENCODING_ERROR)        ->addViolation();  }

With '😊' and ['ASCII'], the generated message will be

The detected character encoding is invalid (). Allowed encodings are ASCII.

Oh and just another (minor) thing.. the mb_string extension seems to not be required by Validator, but the mb_string polyfill does only check ASCII, UTF-8 and latin-ish encodings. Maybe something to add in doc, or to check at runtime ?

Hi! Thank you for letting me know Simon, I'll provide a fix for this 🙂

Why the constraint is namedCharset when it validates the encoding? Why it's notEncoding?

It even uses themb_detect_encoding internally, which is named..._encoding not..._charset :-D

Can you please open a new issue to discuss this before the 7.2 release? Thanks

@OskarStark This feature has been shipped with 7.1 already.

Indeed, time flies 😄