Skip to content

Remove the custom smartypants code [rebased]#3666

Merged
tk0miya merged 8 commits intosphinx-doc:1.6-releasefrom
mitya57:1.6-release
Apr 25, 2017
Merged

Remove the custom smartypants code [rebased]#3666
tk0miya merged 8 commits intosphinx-doc:1.6-releasefrom
mitya57:1.6-release

Conversation

@mitya57
Copy link
Copy Markdown
Contributor

@mitya57 mitya57 commented Apr 25, 2017

Remove the custom smartypants code, instead rely on docutils’ ‘smart_quotes’ option which is available since docutils 0.10. See #3345 for details.

This adds support for internationalization: our code supported only English quotes, while docutils code supports 27 different languages.

Add tests for English, French and Russian smart quotes.
Remove sphinxquotedblleft and sphinxquotedblright, should be no longer needed.

mitya57 added 8 commits April 25, 2017 14:17
@mitya57
Remove the custom smartypants code
edc7f30 
Instead rely on docutils’ ‘smart_quotes’ option which is available
since docutils 0.10.

This adds support for internationalization: our code supported only
English quotes, while docutils code supports 27 different languages.

Closes sphinx-doc#498, sphinx-doc#580, sphinx-doc#3345, sphinx-doc#3472.
@mitya57
Monkey patch docutils’ educateQuotes function to fix apostrophes hand…
c857f52 
…ling
@mitya57
Add tests for English, Russian and French smart quotes
1f9e251
@mitya57
Remove sphinxquotedblleft and sphinxquotedblright, no longer needed
4f4014c
@mitya57
Deprecate the html_use_smartypants option
9236beb
@mitya57
Backport fixes for French quotes from docutils
f99f5b2
@mitya57
Remove bulk_text_processors methods, they are no longer needed
a9cbb42
@mitya57
Also monkey-patch the educate_tokens from Docutils
4721767 
To fix https://sourceforge.net/p/docutils/bugs/317/.
@mitya57 mitya57 mentioned this pull request Apr 25, 2017
Closed
@tk0miya
Copy link
Copy Markdown
Member

tk0miya commented Apr 25, 2017

LGTM!

@tk0miya tk0miya added this to the 1.6 milestone Apr 25, 2017
@jfbu
Copy link
Copy Markdown
Contributor

jfbu commented Apr 25, 2017

LGTM !

@tk0miya tk0miya merged commit 961432e into sphinx-doc:1.6-release Apr 25, 2017
@tk0miya
Copy link
Copy Markdown
Member

tk0miya commented Apr 25, 2017

Merged. Thank you for great work!

@jfbu good reviewing!

@jfbu
Copy link
Copy Markdown
Contributor

jfbu commented Apr 25, 2017

@mitya57 thanks for all your efforts, we count upon you to continue backporting docutils fixes if needed ;-)

tk0miya added a commit that referenced this pull request Apr 25, 2017
@tk0miya
Update CHANGES for PR #3666
50609f2
@jfbu
Copy link
Copy Markdown
Contributor

jfbu commented May 2, 2017

Hi @mitya57 do you mind if we add your name at https://github.com/sphinx-doc/sphinx/blob/1.6-release/CHANGES#L28 in a "Thanks ... " notice for 1.6 release ?

@jfbu
Copy link
Copy Markdown
Contributor

jfbu commented May 2, 2017

... and I will ask the same from G. M. at docutils

@mitya57
Copy link
Copy Markdown
Contributor Author

mitya57 commented May 2, 2017

@jfbu Of course I do not mind.

@mitya57 mitya57 deleted the 1.6-release branch May 2, 2017 13:34
@jfbu
Copy link
Copy Markdown
Contributor

jfbu commented May 2, 2017

@mitya57 ok, great! sometimes people prefer anonymity. I will add a thanks to G. M. too !

jfbu added a commit that referenced this pull request May 2, 2017
@jfbu
Add Thanks note to @mitya57 (ref PR #3666) and docutils@milde
b616f1d
@mitya57 mitya57 mentioned this pull request May 17, 2017
Merged
@ericholscher
Copy link
Copy Markdown
Contributor

ericholscher commented May 22, 2017

Just wanted to note that having Sphinx recommend users use a docutils.conf seems like a bad user experience. From what I've seen, a Sphinx user has never had to ever use this config file, and likely shouldn't need to ever use it. Is there a reason not to allow a user to set it in Sphinx and pass that along into docutils, instead of requiring an entirely separate configuration file?

@mitya57
Copy link
Copy Markdown
Contributor Author

mitya57 commented May 23, 2017

@ericholscher In #3562 (comment), I proposed two options: dropping the option completely or making it map to the docutils option. Then @tk0miya voted for removing it.

A regular user would rarely need to use docutils.conf: SmartyPants are enabled by default and correspond to the project language. That file should be used either when one wants to disable SmartyPants or choose another set of quotes. The latter was impossible in previous Sphinx versions, so even with an external configuration file it is an improvement rather than a regression.

What is your use case?

@hynek
Copy link
Copy Markdown

hynek commented May 23, 2017

I think the main pain point here is that the warning seems to imply that we need a global config to have nice quotes. Maybe change the wording to “it’s on by default, if you need further configurability refer to…”?

@mitya57 mitya57 mentioned this pull request May 24, 2017
Merged
@mitya57
Copy link
Copy Markdown
Contributor Author

mitya57 commented May 24, 2017

Good idea, done in #3795.

@tk0miya
Copy link
Copy Markdown
Member

tk0miya commented May 26, 2017

Sphinx has supports docutils.conf from the past (maybe 1.2 or 1.3). It's not new feature.
So far, docutils provides better default settings. So it is not needed to move the options to sphinx's, I think.

jfbu added a commit to jfbu/sphinx that referenced this pull request May 27, 2017
@jfbu
Fix Docutils restrains smart quotes to its own supported languages
4f14266 
Closes: sphinx-doc#3788, sphinx-doc#3806

Trick Docutils into thinking the document is with language set to
``'en'`` else its smartquotes transform will refuse to apply, even if it
knows the language, in case the language has no Docutils localisation
available.

Refs: sphinx-doc#3666
jfbu added a commit to jfbu/sphinx that referenced this pull request May 27, 2017
@jfbu
Fix Docutils restrains smart quotes to its own locales
19dfe77 
Closes: sphinx-doc#3788, sphinx-doc#3806

Trick Docutils into thinking the document is with language set to
``'en'`` else its smartquotes transform will refuse to apply, even if it
knows the language, in case the language has no Docutils localisation
available.

Refs: sphinx-doc#3666
This was referenced May 27, 2017
Closed
Closed
Closed
jfbu added a commit that referenced this pull request May 28, 2017
@jfbu
Fix #3781: instruct SmartQuotes of nodes to skip
1f451ce 
Relates #3666, #3787
This was referenced May 28, 2017
Merged
Closed
Closed
jfbu added a commit to jfbu/sphinx that referenced this pull request May 31, 2017
@jfbu
Backport docutils.utils.smartquotes fixes as of r8097
0b0bbfc 
This incorporates fixes from 0.14rc1 which were not in the PR sphinx-doc#3666
merged in 1.6.1, and extra language configurations and fixes for quotes.

Only active if Docutils is at 0.13.1 or earlier.
This was referenced May 31, 2017
Merged
Merged
Closed
This was referenced Jun 17, 2017
Closed
Closed
Open
@cocoatomo cocoatomo mentioned this pull request Oct 10, 2017
Closed
sambrightman added a commit to sambrightman/sphinxcontrib-emacs that referenced this pull request Jan 21, 2018
@sambrightman
Fix builds for Sphinx>=1.6 (due to sphinx-doc/sphinx#3666)
3285fc4
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

No reviews

Assignees

No one assigned

Labels

internals:internationalisation markup

Projects

None yet

Milestone

1.6

Development

Successfully merging this pull request may close these issues.

5 participants

@mitya57 @tk0miya @jfbu @ericholscher @hynek