SS5: add trailing slash config and redirect #10538
Conversation
GuySartorelli
force-pushed
the
feature-5-trailing-slash-config-and-redirect
branch
from
ab5ab7d to
65641ef
Compare
|
Rebased, fixed issues, and squashed commits. I'm also going to mark this as a DRAFT until I have the related PRs fixed up and ready to go. |
GuySartorelli
force-pushed
the
feature-5-trailing-slash-config-and-redirect
branch
from
65641ef to
c4981fe
Compare
GuySartorelli
force-pushed
the
feature-5-trailing-slash-config-and-redirect
branch
2 times, most recently
from
ec8040b to
0cf7f2e
Compare
GuySartorelli
force-pushed
the
feature-5-trailing-slash-config-and-redirect
branch
from
0cf7f2e to
deb5d20
Compare
| * If set, the trailing slash configuration set in {@link Controller::add_trailing_slash} is enforced | ||
| * with a redirect. | ||
| */ | ||
| protected bool $forceTrailingSlash = true; |
There was a problem hiding this comment.
This is a badly named property (along with all the other "forceTrailingSlash*" properties.) It does not "force trailing slash" - instead it is basically "do not ignore the trailing slash config on Controller". The naming here makes this very confusing
We'll need to rename this, perhaps simply to "respectTrailingSlashConfig*"? Though there's probably something more better and more concise that would be used. Another option would be to rename this to "ignoreTrailingSlashConfig*" and then reverse the logic
There was a problem hiding this comment.
How about redirectWithTrailingSlashConfig? A bit verbose but it makes it clear, I think?
There was a problem hiding this comment.
hm. You're right it's not forcing the trailing slash, but rather its config value. The correct name would then be "forceTrailingSlashConfig"?
There was a problem hiding this comment.
I think 'force' is probably the wrong word here, though I think just 'enforce' works fine
So this would become enforceTrailingSlashConfig
Does that sound alright?
NOTE: There will be additional related PRs required for at least silverstripe/cms and silverstripe/admin. Co-authored-by: Guy Sartorelli <[email protected]>
GuySartorelli
force-pushed
the
feature-5-trailing-slash-config-and-redirect
branch
from
deb5d20 to
fbcf7dc
Compare
This PR adds trailing slash configuration to
Controllerand extendsCanonicalURLMiddlewareto include the trailing slash redirect.This covers criteria 2-4 of the parent issue
Parent issue