-
Notifications
You must be signed in to change notification settings - Fork 117
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
Preserve search and hash in redirect #127
Conversation
One job still fails but that seems to be a different issue: |
What if the redirect URL already has a query? I don't understand the motivation behind trying to modify |
@pathawks we're talking a static generator right? I can't think of a situation where I agree modifying the refresh meta tag has not a ton of value, neither has modifying the link. If you have JavaScript, the But.. it do think it would should let the meta refresh at 1s to make sure JavaScript can handle te redirect before the browser does. |
Either it is important or it isn't. A page could have been integrated into a section of another page. Perhaps the URL contains a hash of the section in the new page that contains the old content. I feel like this greatly increases the complexity of the template, and I worry that there are some edge cases where problems could creep up. |
What I meant is that the afaik Sure, the it could be that the search and hash no longer serve a purpose on the new page, but in that case it wouldn't hearth anything either. It just wouldn't do anything. In the meanwhile, if would still do anything... without this fix it doesn't let you. |
What I mean is, if I had a page With these new changes, if somebody lands on |
Ah, you mean when using redirect_to? Although afaik I will make a change to accommodate. |
All checks green, except still this odd one: https://travis-ci.org/jekyll/jekyll-redirect-from/jobs/177239657 |
Not your fault. Looks like it's time for us to stop testing against Ruby 1.9 |
@pathawks did you have a chance to have another look at this? @jekyllbot just staled #123 so I'd love to see this (updated and) merged. |
Not sure what happened here, but I think the ability to preserve the hash and query string when redirecting is pretty important. For example, I have a page at That being said, I understand the concerns outline here. Perhaps there could be an additional flag in the frontmatter to preserve these?
|
No, adding options would increase complexity, not reduce it. Any solution would require JavaScript which is far more complex than our existing code. I’m not convinced it’s worth the trade. |
@pathawks i'm not arguing that it makes this less simple. all I'm saying is that it's a useful feature and something i would expect to be able to do with a tool that facilitates redirects. as you can see from the issues on the repo i'm not the only one who thinks this. using a flag to enable the feature would prevent some of the potential drawbacks and edge-cases you've outlined as it would require users to specifically opt-in. |
Thank you for the Pull Request. Code looks solid, but I don't think this feature is worth the complexity it adds to the plugin. |
Closes #123