- Base exception response content type again on accept header.

- The ``pyramid.httpexceptions`` classes named ``HTTPFound``,
  ``HTTPMultipleChoices``, ``HTTPMovedPermanently``, ``HTTPSeeOther``,
  ``HTTPUseProxy``, and ``HTTPTemporaryRedirect`` now accept ``location`` as
  their first positional argument rather than ``detail``.  This means that
  you can do, e.g. ``return pyramid.httpexceptions.HTTPFound('http://foo')``
  rather than ``return
  pyramid.httpexceptions.HTTPFound(location='http//foo')`` (the latter will
  of course continue to work).

4 files changed, 55 insertions, 38 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index 9a25a6b04..dd3673173 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -371,6 +371,15 @@ Backwards Incompatibilities
   it is basically intended to directly mirror the ``webob.Response`` API,
   which has many methods and attributes.
 
+- The ``pyramid.httpexceptions`` classes named ``HTTPFound``,
+  ``HTTPMultipleChoices``, ``HTTPMovedPermanently``, ``HTTPSeeOther``,
+  ``HTTPUseProxy``, and ``HTTPTemporaryRedirect`` now accept ``location`` as
+  their first positional argument rather than ``detail``.  This means that
+  you can do, e.g. ``return pyramid.httpexceptions.HTTPFound('http://foo')``
+  rather than ``return
+  pyramid.httpexceptions.HTTPFound(location='http//foo')`` (the latter will
+  of course continue to work).
+
 Dependencies
 ------------
 
diff --git a/TODO.txt b/TODO.txt
index fb72d42b5..f24b49263 100644
--- a/TODO.txt
+++ b/TODO.txt
@@ -4,6 +4,8 @@ Pyramid TODOs
 Must-Have
 ---------
 
+- Grep for IExceptionResponse, forgot what it does.
+
 - Copy exception templates from webob.exc into pyramid.httpexceptions and
   ensure they all work.
 
diff --git a/pyramid/httpexceptions.py b/pyramid/httpexceptions.py
index 6d689988e..68c40b098 100644
--- a/pyramid/httpexceptions.py
+++ b/pyramid/httpexceptions.py
@@ -144,15 +144,10 @@ class WSGIHTTPException(Response, HTTPException):
 
     # differences from webob.exc.WSGIHTTPException:
     #
-    # - bases plaintext vs. html result on self.content_type rather than
-    #   on request accept header
-    #
     # - doesn't use "strip_tags" (${br} placeholder for <br/>, no other html
     #   in default body template)
     #
-    # - sets a default app_iter onto self during __call__ using a template if
-    #   no body, app_iter, or unicode_body is set onto the response (instead of
-    #   the replaced version's "generate_response")
+    # - __call__ never generates a new Response, it always mutates self
     #
     # - explicitly sets self.message = detail to prevent whining by Python
     #   2.6.5+ access of Exception.message
@@ -160,7 +155,7 @@ class WSGIHTTPException(Response, HTTPException):
     # - its base class of HTTPException is no longer a Python 2.4 compatibility
     #   shim; it's purely a base class that inherits from Exception.  This
     #   implies that this class' ``exception`` property always returns
-    #   ``self`` (only for bw compat at this point).
+    #   ``self`` (it exists only for bw compat at this point).
     #
     # - documentation improvements (Pyramid-specific docstrings where necessary)
     #
@@ -212,17 +207,19 @@ ${body}''')
     def __str__(self):
         return self.detail or self.explanation
 
-    def _default_app_iter(self, environ):
+    def _set_default_attrs(self, environ):
         html_comment = ''
         comment = self.comment or ''
-        content_type = self.content_type or ''
-        if 'html' in content_type:
+        accept = environ.get('HTTP_ACCEPT', '')
+        if accept and 'html' in accept or '*/*' in accept:
+            self.content_type = 'text/html'
             escape = _html_escape
             page_template = self.html_template_obj
             br = '<br/>'
             if comment:
                 html_comment = '<!-- %s -->' % escape(comment)
         else:
+            self.content_type = 'text/plain'
             escape = _no_escape
             page_template = self.plain_template_obj
             br = '\n'
@@ -246,7 +243,7 @@ ${body}''')
         page = page_template.substitute(status=self.status, body=body)
         if isinstance(page, unicode):
             page = page.encode(self.charset)
-        return [page]
+        self.app_iter = [page]
 
     @property
     def wsgi_response(self):
@@ -256,8 +253,15 @@ ${body}''')
     exception = wsgi_response # bw compat only
 
     def __call__(self, environ, start_response):
+        # differences from webob.exc.WSGIHTTPException
+        #
+        # - does not try to deal with HEAD requests
+        #
+        # - does not manufacture a new response object when generating
+        #   the default response
+        #
         if not self.body and not self.empty_body:
-            self.app_iter = self._default_app_iter(environ)
+            self._set_default_attrs(environ)
         return Response.__call__(self, environ, start_response)
 
 class HTTPError(WSGIHTTPException):
@@ -388,23 +392,25 @@ class _HTTPMove(HTTPRedirection):
     """
     # differences from webob.exc._HTTPMove:
     #
-    # - not a wsgi app
-    #
     # - ${location} isn't wrapped in an <a> tag in body
     #
     # - location keyword arg defaults to ''
     #
+    # - location isn't prepended with req.path_url when adding it as
+    #   a header
+    #
+    # - ``location`` is first keyword (and positional) argument
+    #
     # - ``add_slash`` argument is no longer accepted:  code that passes
     #   add_slash argument to the constructor will receive an exception.
     explanation = 'The resource has been moved to'
     body_template_obj = Template('''\
-${explanation} ${location};
-you should be redirected automatically.
+${explanation} ${location}; you should be redirected automatically.
 ${detail}
 ${html_comment}''')
 
-    def __init__(self, detail=None, headers=None, comment=None,
-                 body_template=None, location='', **kw):
+    def __init__(self, location='', detail=None, headers=None, comment=None,
+                 body_template=None, **kw):
         super(_HTTPMove, self).__init__(
             detail=detail, headers=headers, comment=comment,
             body_template=body_template, location=location, **kw)
@@ -637,10 +643,12 @@ class HTTPMethodNotAllowed(HTTPClientError):
     """
     # differences from webob.exc.HTTPMethodNotAllowed:
     #
-    # - body_template_obj not overridden (it tried to use request environ's
-    #   REQUEST_METHOD)
+    # - body_template_obj uses ${br} instead of <br />
     code = 405
     title = 'Method Not Allowed'
+    body_template_obj = Template('''\
+The method ${REQUEST_METHOD} is not allowed for this resource. ${br}${br}
+${detail}''')
 
 class HTTPNotAcceptable(HTTPClientError):
     """
@@ -655,8 +663,7 @@ class HTTPNotAcceptable(HTTPClientError):
     """
     # differences from webob.exc.HTTPNotAcceptable:
     #
-    # - body_template_obj not overridden (it tried to use request environ's
-    #   HTTP_ACCEPT)
+    # - "template" attribute left off (useless, bug in webob?)
     code = 406
     title = 'Not Acceptable'
 
@@ -782,8 +789,7 @@ class HTTPUnsupportedMediaType(HTTPClientError):
     """
     # differences from webob.exc.HTTPUnsupportedMediaType:
     #
-    # - body_template_obj not overridden (it tried to use request environ's
-    #   CONTENT_TYPE)
+    # - "template_obj" attribute left off (useless, bug in webob?)
     code = 415
     title = 'Unsupported Media Type'
 
@@ -898,8 +904,7 @@ class HTTPNotImplemented(HTTPServerError):
     """
     # differences from webob.exc.HTTPNotAcceptable:
     #
-    # - body_template_obj not overridden (it tried to use request environ's
-    #   REQUEST_METHOD)
+    # - "template" attr left off (useless, bug in webob?)
     code = 501
     title = 'Not Implemented'
 
@@ -992,6 +997,7 @@ def default_exceptionresponse_view(context, request):
     return context
 
 status_map={}
+code = None
 for name, value in globals().items():
     if (isinstance(value, (type, types.ClassType)) and
         issubclass(value, HTTPException)
@@ -999,7 +1005,4 @@ for name, value in globals().items():
         code = getattr(value, 'code', None)
         if code:
             status_map[code] = value
-del name, value
-
-
-
+del name, value, code
diff --git a/pyramid/tests/test_httpexceptions.py b/pyramid/tests/test_httpexceptions.py
index 60bde460e..644051fcb 100644
--- a/pyramid/tests/test_httpexceptions.py
+++ b/pyramid/tests/test_httpexceptions.py
@@ -139,6 +139,7 @@ class TestWSGIHTTPException(unittest.TestCase):
         cls = self._getTargetSubclass()
         exc = cls('detail')
         environ = _makeEnviron()
+        environ['HTTP_ACCEPT'] = 'text/html'
         start_response = DummyStartResponse()
         body = list(exc(environ, start_response))[0]
         self.assertTrue(body.startswith('<html'))
@@ -149,7 +150,6 @@ class TestWSGIHTTPException(unittest.TestCase):
     def test_ctor_with_body_sets_default_app_iter_text(self):
         cls = self._getTargetSubclass()
         exc = cls('detail')
-        exc.content_type = 'text/plain'
         environ = _makeEnviron()
         start_response = DummyStartResponse()
         body = list(exc(environ, start_response))[0]
@@ -176,7 +176,6 @@ class TestWSGIHTTPException(unittest.TestCase):
     def test__calls_start_response(self):
         cls = self._getTargetSubclass()
         exc = cls()
-        exc.content_type = 'text/plain'
         environ = _makeEnviron()
         start_response = DummyStartResponse()
         exc(environ, start_response)
@@ -186,7 +185,6 @@ class TestWSGIHTTPException(unittest.TestCase):
     def test__default_app_iter_no_comment_plain(self):
         cls = self._getTargetSubclass()
         exc = cls()
-        exc.content_type = 'text/plain'
         environ = _makeEnviron()
         start_response = DummyStartResponse()
         body = list(exc(environ, start_response))[0]
@@ -195,7 +193,6 @@ class TestWSGIHTTPException(unittest.TestCase):
     def test__default_app_iter_with_comment_plain(self):
         cls = self._getTargetSubclass()
         exc = cls(comment='comment')
-        exc.content_type = 'text/plain'
         environ = _makeEnviron()
         start_response = DummyStartResponse()
         body = list(exc(environ, start_response))[0]
@@ -204,7 +201,6 @@ class TestWSGIHTTPException(unittest.TestCase):
     def test__default_app_iter_no_comment_html(self):
         cls = self._getTargetSubclass()
         exc = cls()
-        exc.content_type = 'text/html'
         environ = _makeEnviron()
         start_response = DummyStartResponse()
         body = list(exc(environ, start_response))[0]
@@ -213,8 +209,17 @@ class TestWSGIHTTPException(unittest.TestCase):
     def test__default_app_iter_with_comment_html(self):
         cls = self._getTargetSubclass()
         exc = cls(comment='comment & comment')
-        exc.content_type = 'text/html'
         environ = _makeEnviron()
+        environ['HTTP_ACCEPT'] = '*/*'
+        start_response = DummyStartResponse()
+        body = list(exc(environ, start_response))[0]
+        self.assertTrue('<!-- comment &amp; comment -->' in body)
+
+    def test__default_app_iter_with_comment_html2(self):
+        cls = self._getTargetSubclass()
+        exc = cls(comment='comment & comment')
+        environ = _makeEnviron()
+        environ['HTTP_ACCEPT'] = 'text/html'
         start_response = DummyStartResponse()
         body = list(exc(environ, start_response))[0]
         self.assertTrue('<!-- comment &amp; comment -->' in body)
@@ -222,7 +227,6 @@ class TestWSGIHTTPException(unittest.TestCase):
     def test_custom_body_template(self):
         cls = self._getTargetSubclass()
         exc = cls(body_template='${REQUEST_METHOD}')
-        exc.content_type = 'text/plain'
         environ = _makeEnviron()
         start_response = DummyStartResponse()
         body = list(exc(environ, start_response))[0]
@@ -233,7 +237,6 @@ class TestWSGIHTTPException(unittest.TestCase):
         la = unicode('/La Pe\xc3\xb1a', 'utf-8')
         environ = _makeEnviron(unicodeval=la)
         exc = cls(body_template='${unicodeval}')
-        exc.content_type = 'text/plain'
         start_response = DummyStartResponse()
         body = list(exc(environ, start_response))[0]
         self.assertEqual(body, '200 OK\n\n/La Pe\xc3\xb1a')