Skip to content
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

Replace strip_www with optional redirect to www/non-www #452

Merged
merged 1 commit into from
Dec 29, 2015

Conversation

fullyint
Copy link
Contributor

Discussion in #312.

This www_redirect is on a per-site basis whereas strip_www was for all sites or none.

This includes extra logic to avoid redirecting subdomains, based on comments in #312. However, I wonder if some people would actually want www.sub.example.com to redirect to sub.example.com. If we can allow the redirect for subdomains, we could drop this portion of the conditional logic:

... and host.split('.') | difference('www') | count <= 2

(refs: split, difference, count/length)

@retlehs
Copy link
Member

retlehs commented Dec 29, 2015

However, I wonder if some people would actually want www.sub.example.com to redirect to sub.example.com

👍 (ignore my previous comment)

@fullyint fullyint force-pushed the www-redirect branch 2 times, most recently from 42db688 to 7beed93 Compare December 29, 2015 18:04
@fullyint
Copy link
Contributor Author

Thanks @retlehs. I adjusted to allow redirects for subdomains:

- {% for host in item.value.site_hosts if item.value.www_redirect | default(true) and host.split('.') | difference('www') | count <= 2 %}
+ {% for host in item.value.site_hosts if item.value.www_redirect | default(true) %}

@swalkinshaw
Copy link
Member

Tested and works great 👍

Needs a docs update as well.

swalkinshaw added a commit that referenced this pull request Dec 29, 2015
Replace strip_www with optional redirect to www/non-www
@swalkinshaw swalkinshaw merged commit fd9a69d into roots:master Dec 29, 2015
@fullyint fullyint deleted the www-redirect branch December 29, 2015 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants