-
-
Notifications
You must be signed in to change notification settings - Fork 165
Use definition lists rather than blockquotes #107
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
Conversation
Such change may mess up existing stylesheets that assume something about the generated html. |
I'm aware, and so I'm inclined to raise this issue and close the PR. But what we do currently is pretty weird. |
FWIW, the background to my concern here is about trying to style the definition list... only to discover that we're not using a definition list, hence the term has no distinguishing markup around it. |
The code is soon 10 years old, so if I wrote it I don't remember the reason for this any more. |
yes, I suspect it's accidental or to handle some case where people were
putting blank lines between param name and description in the docstring.
sigh. I suppose we will hack solutions on the client side or none at all
…On 6 Sep 2017 10:15 am, "Pauli Virtanen" ***@***.***> wrote:
The code is soon 10 years old, so if I wrote it I don't remember the
reason for this any more.
If a reason is not obvious, it may well be there's no reason...
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#107 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAEz66NVK10HutJlrbFcXfeXN66gHeW0ks5sfeQLgaJpZM4PNqZg>
.
|
Or, fix it to be consistent, and wait for complaints to pop up? |
Well, there's no problem of internal inconsistency... it's just a weird use of RST and HTML. |
I'd like to have this fixed, because it produces much more pleasant HTML... But I'm a bit of an idealist. One could have a switch on it. Also, something I need to check: remove any blank lines at the beginning of the description. |
Make blockquote behaviour optional. Not yet tested
I've pushed a change which uses definition lists by default but allows users requiring the legacy styling to switch this on with a config option to be removed in release 0.10. WDYT? I've also changed the tests to be sensitive to the presence or absence of blank lines, but to ignore multiple (and those at the beginning or end), when comparing rendered and expected reST. Presence and absence of single blank lines is (very) significant in reST! |
TBH, I would just make the change, maybe break stuff, and wait for complaints. rebase needed. |
I don't see the harm in keeping an alternative, though. Is it worth the
effort of changing it back to not have an option?
|
No harm in having an option --- I didn't see before commenting that you had already implemented the option. |
This issue results from numpy#107: the explicit bold styling was removed because Sphinx definition lists bolded the term by default. However, without ** surrounding param name, a name like word_ was being treated as a link. I also attempted to replace the _ with \_ but found that the backslashes showed in the HTML. Thus I have reverted to wrapping param names in **.
* Remove numpydoc_use_blockquotes See #107 * Update docstring
This probably falls under the category of "if it ain't broke, don't fix it", but I note that we're strangely using blockquotes for parameter listings instead of definition lists. UPDATED: now this PR proposes to use definition lists by default, with a switch to use the legacy blockquotes.
From RST docs:
(emphasis mine)
As shown in the screenshots below, there are formatting differences in the default stylesheets:
:
has the classclassifier-delimiter
and what follows has the classclassifier
definition lists (with this patch):

blockquotes (at master)

Does anyone know of disadvantages to definition lists, or why they have not been used here?
This patch should probably be extended to ensure that the first line of a parameter description is not blank, or it will revert to the blockquote styling.
This also highlights a problem with numpydoc's tests: vertical whitespace is significant in RST, particularly its presence/absence, but tests currently ignore blank lines, which is a Bad Idea.