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

Fixed #6681 -- Don't break docutils when rendering reStructuredText. #1277

Closed
wants to merge 10 commits into
base: master
from

Conversation

Projects
None yet
8 participants
@mrmachine

mrmachine commented Jun 17, 2013

No description provided.

@mrmachine

This comment has been minimized.

Show comment
Hide comment
@mrmachine

mrmachine Jun 18, 2013

This latest commit avoids the need to set a global default at all, so it should be compatible with any 3rd party code that needs to publish reStructuredText, and any 3rd party code that changes the global default like we were won't break admindocs.

We should probably squash these before merging, but I've left the earlier commit just to show the progression for any reviewers.

mrmachine commented Jun 18, 2013

This latest commit avoids the need to set a global default at all, so it should be compatible with any 3rd party code that needs to publish reStructuredText, and any 3rd party code that changes the global default like we were won't break admindocs.

We should probably squash these before merging, but I've left the earlier commit just to show the progression for any reviewers.

akaariai and others added some commits Aug 6, 2013

Fixed ordering related test failure
Also PEP8 + python_2_unicode_compatible cleanup done.
Merge pull request #1441 from loic/ticket16986
Fixed #16986 -- Model.clean() can report errors on individual fields.
Fixed #20852 - Fixed incorrectly generated left quotes in docs.
Sphinx generates left single quotes for apostrophes after
code markup, when right single quotes are required. The
easiest way to fix this is just by inserting the unicode
character for a right single quote.

Instances of the problem were found by looking for
">‘" in the generated HTML.
Fixed #15511 -- Allow optional fields on ``MultiValueField` subclasses.
The `MultiValueField` class gets a new ``require_all_fields`` argument that
defaults to ``True``. If set to ``False``, individual fields can be made
optional, and a new ``incomplete`` validation error will be raised if any
required fields have empty values.

The ``incomplete`` error message can be defined on a `MultiValueField`
subclass or on each individual field. Skip duplicate errors.
@timgraham

This comment has been minimized.

Show comment
Hide comment
@timgraham

timgraham Aug 6, 2013

Member

Any idea if we can add a test for this?

Member

timgraham commented Aug 6, 2013

Any idea if we can add a test for this?

@mrmachine

This comment has been minimized.

Show comment
Hide comment
@mrmachine

mrmachine Aug 7, 2013

Rebased onto master. Tests added. Lucky, because I found a bug in my implementation. Now we also restore the default role after temporarily setting it to cmsreference.

Do you want me to squash all these commits down?

Thanks for the review!

mrmachine commented Aug 7, 2013

Rebased onto master. Tests added. Lucky, because I found a bug in my implementation. Now we also restore the default role after temporarily setting it to cmsreference.

Do you want me to squash all these commits down?

Thanks for the review!

@@ -43,3 +44,43 @@ def test_xview_class(self):
user.save()
response = self.client.head('/xview/class/')
self.assertFalse('X-View' in response)
class DefaultRoleTest(TestCase):

This comment has been minimized.

@timgraham

timgraham Aug 7, 2013

Member

These tests should be skipped if docutils isn't installed

try:
    import docutils
except ImportError:
    docutils = None


@unittest.skipUnless(docutils, "no docutils installed.")
class DefaultRoleTest(TestCase):
@timgraham

timgraham Aug 7, 2013

Member

These tests should be skipped if docutils isn't installed

try:
    import docutils
except ImportError:
    docutils = None


@unittest.skipUnless(docutils, "no docutils installed.")
class DefaultRoleTest(TestCase):
``cmsreference``. See #6681.
"""
try:
from docutils.core import publish_parts

This comment has been minimized.

@timgraham

timgraham Aug 7, 2013

Member

can this be removed with the skip logic I added? If not, could you add a comment indicating the purpose?

@timgraham

timgraham Aug 7, 2013

Member

can this be removed with the skip logic I added? If not, could you add a comment indicating the purpose?

@timgraham

This comment has been minimized.

Show comment
Hide comment
@timgraham

timgraham Aug 7, 2013

Member

It's easy enough to squash the commits when merging and having the individual commits makes reviewing a bit easier -- thanks again!

Member

timgraham commented Aug 7, 2013

It's easy enough to squash the commits when merging and having the individual commits makes reviewing a bit easier -- thanks again!

@timgraham

This comment has been minimized.

Show comment
Hide comment
@timgraham

timgraham Oct 4, 2013

Member

merged in bcd4c3f - thanks!

Member

timgraham commented Oct 4, 2013

merged in bcd4c3f - thanks!

@timgraham timgraham closed this Oct 4, 2013

@mrmachine mrmachine deleted the thirstydigital:tickets/6681-rst-default-role branch Feb 6, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment