search

iron / router

Router middleware for the Iron web framework.
165 stars 74 forks source link

(feat) Allow trailing slash #84

Closed leoyvens closed 9 years ago

leoyvens commented 9 years ago

Partially fixes #82.

Urls with and without trailing slashes should be handled differently. A handler can exist with a trailing slash, and a different handler can exist without the trailing slash. Both should work independently.

Done.

An option, named something like RedirectTrailingSlash should exist. If it is set to true, and a requested route isn't found, but a route is found by adding (or removing) a trailing slash, a redirect is issued. This option should probably default to true.

The router now behaves as if such option was set to true. But the option is not implemented.

fn handle was refactored. Hopefully the result is clear and concise. These are my first lines of Rust, all feedback is appreciated!

leoyvens commented 9 years ago

Merged upstream changes (#85). Further refactoring. Is there some recommendation on commit squashing? Not used to rebasing things.

reem commented 9 years ago

Looks great to me! Can you squash all the commits together to remove the merge commits? Otherwise this is ready to merge.

leoyvens commented 9 years ago

I've tried to squash the commits with all sorts of git rebase. But because of the merging and coflicts, it seems non-trivial to squash them, at least for me. I can create a new PR with a clean history, would that be preferable?

reem commented 9 years ago

@leodasvacas the easiest way (if traditional git rebase -i is not working for some reason) is to just git reset HEAD~3 --soft then just recommit all the changes in a new commit, then push to this branch and pass --force to override the commits that have already been pushed which are now deleted. That should just work.

leoyvens commented 9 years ago

@reem Thanks for the assistance! I was avoiding that because I imagined it would generate a conflict on your end, but if that's fine, cool, it's done!

reem commented 9 years ago

Looks like you were right, there is now a merge conflict :/

Since I don't want to block you on git stuff, I can just take it from here. Just for your information, what I'll do is pull this branch locally then run git pull --rebase upstream master where upstream is this repo. Then, when I fix conflicts it won't generate a merge commit.

reem commented 9 years ago

Merged manually.