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

Fix option name for smart quotes in sphinxdoc conf #6422

Merged
merged 1 commit into from Apr 21, 2019

Conversation

@atugushev
Copy link
Contributor

@atugushev atugushev commented Apr 19, 2019

Fixes transform double dashes to a long dash. For the instance you can see this issue in this section.

Correct option name described here: https://www.sphinx-doc.org/en/master/usage/configuration.html#confval-smartquotes


Before

Снимок экрана 2019-04-19 в 11 10 24


After

Снимок экрана 2019-04-19 в 11 18 37

@cjerdonek
Copy link
Member

@cjerdonek cjerdonek commented Apr 19, 2019

Thanks, @atugushev! Can you add a doc news entry since this is a user-visible change / improvement? https://pip.pypa.io/en/stable/development/contributing/#news-entries

Loading

@atugushev atugushev force-pushed the fix-smart-quotes branch 2 times, most recently from d3c145a to 442166f Apr 19, 2019
@atugushev
Copy link
Contributor Author

@atugushev atugushev commented Apr 19, 2019

@cjerdonek done!

Loading

@cjerdonek cjerdonek force-pushed the fix-smart-quotes branch from 442166f to 3d6af51 Apr 20, 2019
@cjerdonek
Copy link
Member

@cjerdonek cjerdonek commented Apr 20, 2019

On second thought, how would you feel about using backticks instead (code formatting) for such options appearing inline in the text? It seems like "smart quoting" leads to better formatting for true text, and that way we could avoid having to sacrifice good formatting for one over good formatting for the other. Also, on the page you linked to, I noticed a number of spots that aren't using backticks when it should, e.g.

by default is “<venv path>/src/SomeProject”

or are using backticks incorrectly, e.g.

(if not disabled via `--no-index`)

Loading

@cjerdonek
Copy link
Member

@cjerdonek cjerdonek commented Apr 20, 2019

Also, in cases where using backticks might not work correctly (e.g. nested situations like inside :ref:), can you do something like slash-escape the dashes to disable the smart-quoting?

Loading

@atugushev
Copy link
Contributor Author

@atugushev atugushev commented Apr 20, 2019

slash-escape the dashes to disable the smart-quoting

I've tried slash-escape, but it doesn't work as expected.

It seems like "smart quoting" leads to better formatting for true text, and that way we could avoid having to sacrifice good formatting for one over good formatting for the other.

Agree. Maybe it's better to turn smartquotes on and configure this plugin to not trasform dashes by option smartquotes_action = "qe" (exclude D). See details for smartquotes_action.

Loading

@atugushev atugushev force-pushed the fix-smart-quotes branch from 3d6af51 to c0c7d82 Apr 20, 2019
@atugushev
Copy link
Contributor Author

@atugushev atugushev commented Apr 20, 2019

@cjerdonek now it looks better! Thanks for fixing the news entry text!

Loading

@cjerdonek
Copy link
Member

@cjerdonek cjerdonek commented Apr 20, 2019

Yes, I think it's an improvement! But there's still the issue that options appearing inside Sphinx "roles" like :ref: are getting formatted differently because it doesn't permit nested backticks. E.g. (though the second one is without your change)

Screen Shot 2019-04-19 at 9 28 13 PM

Screen Shot 2019-04-19 at 9 28 32 PM

And also textual dashes won't get converted as a result.

What about not putting option flags inside roles, and instead doing something like putting the hyperlink afterwards in a footnote or parentheses?

Loading

@cjerdonek cjerdonek force-pushed the fix-smart-quotes branch from c0c7d82 to 5a5241d Apr 21, 2019
@cjerdonek cjerdonek force-pushed the fix-smart-quotes branch from 5a5241d to 61520b9 Apr 21, 2019
@cjerdonek
Copy link
Member

@cjerdonek cjerdonek commented Apr 21, 2019

@atugushev I fixed up some of the wording, etc. and included more explanation of why smartquotes_action is being set. If it looks good to you, I'll merge.

Loading

@atugushev
Copy link
Contributor Author

@atugushev atugushev commented Apr 21, 2019

LGTM 👍

Loading

@cjerdonek cjerdonek merged commit 7130b1d into pypa:master Apr 21, 2019
29 checks passed
Loading
@cjerdonek
Copy link
Member

@cjerdonek cjerdonek commented Apr 21, 2019

Thanks again for this improvement! 😄

Loading

@lock
Copy link

@lock lock bot commented May 28, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

Loading

@lock lock bot added the S: auto-locked label May 28, 2019
@lock lock bot locked as resolved and limited conversation to collaborators May 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants