Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Validator] AddSlug constraint#58542

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
fabpot merged 1 commit intosymfony:7.3fromraffaelecarelle:slug-constraint
Jan 6, 2025

Conversation

raffaelecarelle
Copy link
Contributor

@raffaelecarelleraffaelecarelle commentedOct 11, 2024•
edited
Loading

QA
Branch?7.3
Bug fix?no
New feature?yes
Deprecations?no
Issues
LicenseMIT

Introduce Slug constraint class for validating strings as slugs.
Add unit tests to verify correct behavior for valid and invalid slugs.

darthf1, raffaelecarelle, 5baddi, and andreybolonin reacted with thumbs up emoji
Copy link
Member

@fabpotfabpot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Can you add a test with a custom regex?

raffaelecarelle reacted with thumbs up emoji
@raffaelecarelle
Copy link
ContributorAuthor

Can you add a test with a custom regex?

Done

@fabpot
Copy link
Member

Thank you@raffaelecarelle.

@fabpotfabpot merged commit18d08ff intosymfony:7.3Jan 6, 2025
9 checks passed
@Kocal
Copy link
Member

Kocal commentedApr 29, 2025•
edited
Loading

Hey, are we fine with(new AsciiSlugger())->slug('My article title') failing to be asserted by this newSlug assertion, since the generated slug isMy-article-title, with an uppercase character?

{
public const NOT_SLUG_ERROR = '14e6df1e-c8ab-4395-b6ce-04b132a3765e';

public string $message = 'This value is not a valid slug.';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This message needs to be added in translation files (so that the translators from the community can provide translations for it in core, like other default messages of constraints)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

this has already been done in#59383

@xabbuh
Copy link
Member

@Kocal see#60426

Kocal reacted with heart emoji

nicolas-grekas added a commit that referenced this pull requestMay 16, 2025
…results (xabbuh)This PR was merged into the 7.3 branch.Discussion----------[Validator] let the `SlugValidator` accept `AsciiSlugger` results| Q             | A| ------------- | ---| Branch?       | 7.3| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        |Fix#58542 (comment)| License       | MITCommits-------d51f563 let the SlugValidator accept AsciiSlugger results
symfony-splitter pushed a commit to symfony/validator that referenced this pull requestMay 16, 2025
…results (xabbuh)This PR was merged into the 7.3 branch.Discussion----------[Validator] let the `SlugValidator` accept `AsciiSlugger` results| Q             | A| ------------- | ---| Branch?       | 7.3| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        |Fixsymfony/symfony#58542 (comment)| License       | MITCommits-------d51f563ed3c let the SlugValidator accept AsciiSlugger results
wouterj added a commit to wouterj/symfony that referenced this pull requestMay 19, 2025
fabpot added a commit that referenced this pull requestMay 20, 2025
This PR was merged into the 7.3 branch.Discussion----------[Validator] Revert Slug constraint| Q             | A| ------------- | ---| Branch?       | 7.3| Bug fix?      | no| New feature?  | no| Deprecations? | no| Issues        | Reverts#58542| License       | MITAfter an internal discussion, I think it's best to revert the introduction of the `Slug` constraint:* There is no official standard for slugs, or what a slug means exactly. Within Symfony, it would be best to comply with what our `AsciiSlugger` is doing, but a [more common approach](https://en.wikipedia.org/wiki/Clean_URL#Slug) is to only allow lowercase characters.* The constraint adds little on top of the existing `Regex` constraint, especially when using it's `regex` option to customize the pattern for slugs.Considering this, I believe it is always better for applications to be clear on what they allow in a slug using the `Regex` constraint.Commits-------5789965 Revert Slug constraint
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@stofstofstof left review comments

@xabbuhxabbuhxabbuh left review comments

@fabpotfabpotfabpot approved these changes

@derrabusderrabusAwaiting requested review from derrabus

Assignees
No one assigned
Projects
None yet
Milestone
7.3
Development

Successfully merging this pull request may close these issues.

7 participants
@raffaelecarelle@fabpot@Kocal@xabbuh@stof@derrabus@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp