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

Static matching strategy could ignore ending slash #75

Closed
maderlock opened this issue Jul 10, 2017 · 4 comments
Closed

Static matching strategy could ignore ending slash #75

maderlock opened this issue Jul 10, 2017 · 4 comments

Comments

@maderlock
Copy link

The static strategy would be a good one to use in many simple cases, except that when trying it out I found that about half my links failed because a slash was appended. I should almost certainly canonicalise my links, but this still feels a little fragile.

Would it be better to allow an options slash on the end of links that do not have one, or perhaps you think that in this case we should be using regex (my solution here) or doubling every rule to match slashes.

@der-workfloh
Copy link
Contributor

der-workfloh commented Jul 10, 2017

Hi @maderlock ,

thanks for the feedback, much appreciate it!

I think the best way would be to canonicalize, because I think at the end "/foo/bar" and "/foo/bar/" representing the same endpoint. Is there any reason why you think this could be fragile?

I think doubling every rule would be an overhead mess, and regex should be for more complex stuff.

@maderlock
Copy link
Author

No, I don't think that would be fragile. I cannot think of a useful case where you would want a bareword to differ from that same bareword plus a slash - nobody would find that intuitive I think

der-workfloh pushed a commit that referenced this issue Jul 11, 2017
der-workfloh pushed a commit that referenced this issue Jul 11, 2017
#75 Static matcher now canonicalizes url and rule to omit differences…
@der-workfloh
Copy link
Contributor

Fixed this issue with RC3 of v2.1.0

@maderlock
Copy link
Author

That's great, thanks. I'll pull this into the site I'm working on and see how it fares in user testing.

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

No branches or pull requests

2 participants