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

force_root_path / add_root_path / add_trailing_slash #7

Open
jaimeiniesta opened this issue Apr 2, 2020 · 2 comments
Open

force_root_path / add_root_path / add_trailing_slash #7

jaimeiniesta opened this issue Apr 2, 2020 · 2 comments

Comments

@jaimeiniesta
Copy link
Contributor

Related to the PR I just created for add_trailing_slash, I wanted to propose a change in force_root_path.

I thought it would work more like add_root_path in NormalizeUrl, that is, only converting an empty path into "/", but respecting non-empty paths like /some/dir.

Instead I see that force_root_path completely replaces any path into the root path. In this case I think we should consider also removing the query and fragment:

  • example.com/some/dir#faqs?page=2 -> example.com/

But I'm not sure about that, maybe we should have other options like wipe_query and wipe_fragment.

Or, include add_root_path as in the above explanation, this would convert URLs like:

  • example.com -> example.com/
  • example.com/some/dir -> example.com/some/dir/
  • example.com/some/dir#top?p=2 -> example.com/some/dir/#top?p=2

What do you think?

@ZuraGuerra
Copy link
Collaborator

It makes sense to me to wipe the query along with the fragment, seems like a missing case. Wanna tackle the changes for it?

I just merged your PR for adding trailing slashes

@jaimeiniesta
Copy link
Contributor Author

Sure, I'll submit a new PR next week to wipe the query and fragment when using force_root_path.

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