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

Clean URLs with and without trailing slash #181

Merged
merged 1 commit into from
Jun 5, 2019

Conversation

niamu
Copy link
Contributor

@niamu niamu commented May 25, 2019

This pull request is the other half of the work related to cryogen-project/cryogen-core#119.

@lacarmen
Copy link
Member

lacarmen commented Jun 1, 2019

I had to update the server.clj code so that navigating to /blog would work with :no-trailing-slash and :dirty

(defn update-path 
  [config req-uri]
  (if (or (= req-uri "") (= req-uri "/") (= req-uri (.substring (:blog-prefix config) 1)))
    (path req-uri "index.html")
    (path (str req-uri ".html"))))

(defn wrap-subdirectories
  [handler]
  (fn [request]
    (let [{:keys [clean-urls] :as config} (read-config)
          req-uri (.substring (url-decode (:uri request)) 1)
          res-path (condp = clean-urls
                     :trailing-slash (path req-uri "index.html")
                     :no-trailing-slash (update-path config req-uri) 
                     :dirty (update-path config req-uri))]
      (or (resource-response res-path {:root "public"})
          (handler request)))))

@lacarmen
Copy link
Member

lacarmen commented Jun 1, 2019

@niamu Could you double check that we have all the cases covered? I tested everything myself but a 2nd pair of eyes is always better :)

@niamu
Copy link
Contributor Author

niamu commented Jun 1, 2019

Yup, you're right. I must have not had a :blog-prefix set when testing that case. I've resolved it now and confirmed the other cases are still working.

@lacarmen
Copy link
Member

lacarmen commented Jun 5, 2019

Thanks @niamu, looks like everything is working!

I'm going to merge this + the cryogen-core change but I will hold off on the docs one until I release everything together with cryogen-project/cryogen-core#117

@lacarmen lacarmen merged commit 3881b44 into cryogen-project:master Jun 5, 2019
@lacarmen
Copy link
Member

lacarmen commented Jun 5, 2019

Actually, I changed my mind. I'm going to do two releases for the two changes since the project structure one will introduce breaking changes and users might want to get the clean-urls change but not the project structure change.

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.

2 participants