-
-
Notifications
You must be signed in to change notification settings - Fork 315
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
[TwigComponent] Improve exception message when component not found #1031
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea!
Should we list all components in case it didn't find any alternatives? 🙂 |
I'm thinking no. I don't think the Symfony code base does that. |
da3b611
to
43769c7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
levenshtein! I like this - can you rebase?
43769c7
to
eeef204
Compare
Rebased |
$lev = levenshtein($lowerName, $lowerType); | ||
|
||
if ($lev <= $nameLength / 3 || str_contains($lowerType, $lowerName)) { | ||
$alternatives[] = $type; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about low cost check first instead of doing both?
$lev = levenshtein($lowerName, $lowerType); | |
if ($lev <= $nameLength / 3 || str_contains($lowerType, $lowerName)) { | |
$alternatives[] = $type; | |
} | |
if (str_contains($lowerType, $lowerName)) { | |
$alternatives[] = $type; | |
} elseif (levenshtein($lowerName, $lowerType) <= $nameLength / 3) { | |
$alternatives[] = $type; | |
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you going to get this exception all the time? 🙂
|
||
$message .= implode('", "', $alternatives).'"?'; | ||
} else { | ||
$message .= ' And no matching anonymous component template was found.'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Just tried it out, it's super useful! Do you think we can apply the Levenshtein to look also for AnonymousComponent?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🤔 probably all available twig templates then should be iterated :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I'm not using anonymous components, then I'll leave it for you as that was your feature :)
This is great - thanks Tomas! |
I have over 150 components and to me it's just useless to list all component names in exception message and to look for the right name in a very long list.
Used same code to get alternatives like in Symfony codebase.