Merge lp:~mwhudson/loggerhead/relative-links into lp:loggerhead

[Michael Hudson-Doyle]

[Michael Hudson-Doyle (mwhudson)]

This branch generates relative links apart from in redirects.

[Matt Nordhoff (mnordhoff)]

Poking around with this patch, it works, AFAICT.

I don't know enough about Loggerhead/Paste's URL handling to review it, though.

However, there is one problem, the feeds:

* <id> needs to be changed back to use absolute URLs.

* The Feed Validator also recommends using absolute URLs for <link rel="self" href="..." /> for maximum interoperability.

* The rest of the feeds can stick with relative URLs.

Once that's fixed, if you're sure nothing else needs to be changed, I have no objections to landing this. But as I said, I don't think I have the expertise to know if I should have any objections. :P

review: Needs Fixing

[Matt Nordhoff (mnordhoff)]

> * The Feed Validator also recommends using absolute URLs for <link rel="self"
> href="..." /> for maximum interoperability.

NB: It doesn't mind relative URLs in the other <link> tags.

[Matt Nordhoff (mnordhoff)]

Superseded by lp:~mnordhoff/loggerhead/relative-links.

review: Disapprove

Unmerged revisions

=== modified file 'loggerhead/apps/branch.py'
--- loggerhead/apps/branch.py	2009-10-17 06:35:33 +0000
+++ loggerhead/apps/branch.py	2009-11-27 00:35:21 +0000
@@ -88,10 +88,11 @@
             if v is not None:
                 qs.append('%s=%s' % (k, urllib.quote(v)))
         qs = '&'.join(qs)
-        return request.construct_url(
-            self._environ, script_name=self._url_base,
-            path_info=unicode('/'.join(args)).encode('utf-8'),
-            querystring=qs)
+        path_info = urllib.quote(
+            unicode('/'.join(args)).encode('utf-8'), safe='/~:')
+        if qs:
+            path_info += '?' + qs
+        return self._url_base + path_info
 
     def context_url(self, *args, **kw):
         kw = util.get_context(**kw)
@@ -152,7 +153,9 @@
         path = request.path_info_pop(environ)
         if not path:
             raise httpexceptions.HTTPMovedPermanently(
-                self._url_base + '/changes')
+                request.construct_url(
+                    self._environ, script_name=self._url_base,
+                    path_info='/changes'))
         if path == 'static':
             return static_app(environ, start_response)
         cls = self.controllers_dict.get(path)