-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Doc rework (user guide, part 3) #4700
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4700 +/- ##
=======================================
Coverage 82.12% 82.12%
=======================================
Files 288 288
Lines 38002 38002
Branches 5894 5894
=======================================
Hits 31210 31210
Misses 5473 5473
Partials 1319 1319 Continue to review full report at Codecov.
|
@tk0miya Is there anything I can do to help push this forward? I've another 20 patches/~4 pull requests queued up behind this that I'd like to close out 😅 |
879729f
to
1e88f81
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for late. Reviewed. Basically LGTM!
doc/contents.rst
Outdated
usage/restructuredtext/roles | ||
usage/restructuredtext/directives | ||
usage/restructuredtext/field-lists | ||
usage/restructuredtext/domains |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems basics
, roles
, directives
and field-lists
are listed at usage/restrcturedtext/index.rst
. So I think these entry is not needed.
https://github.com/sphinx-doc/sphinx/pull/4700/files#diff-76b0f70ed3bdb440fdf39711c7d2d989R15
BTW, usage/restructuredtext/domains
is not listed up at usage/restructuredtext/index.rst
. Is there any reason?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems basics, roles, directives and field-lists are listed at usage/restrcturedtext/index.rst. So I think these entry is not needed.
https://github.com/sphinx-doc/sphinx/pull/4700/files#diff-76b0f70ed3bdb440fdf39711c7d2d989R15
That's a good point. I'll fix this.
Code highlighting can be enabled for these literal blocks on a document-wide | ||
basis using the :rst:dir:`highlight` directive. The :rst:dir:`code-block` | ||
directive can be used to set highlighting on a block-by-block basis. These | ||
directives are discussed later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can also use highlight_language
confval for project-wide basis.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I will added this.
.. todo:: | ||
|
||
Is there any form of explicit markup that *isn't* a directive? If not, why | ||
bother making this distinction? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm? Did you read the page the "Explicit markup" refers?
http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#explicit-markup-blocks
All footnotes, citations and so on are explicit markups.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, I did not 😄 I will link to that document from here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, we already link to it. I just did not think of citations and footnotes as different to directives, when they clearly are. My mistake
@@ -293,6 +302,7 @@ Docutils supports the following directives: | |||
- :dudir:`highlights`, :dudir:`pull-quote` (block quotes with their own | |||
class attribute) | |||
- :dudir:`compound <compound-paragraph>` (a compound paragraph) | |||
- :dudir:`code-block` (literal blocks with a specific syntax) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code-block
is not an official directive of docutils. So it is not listed on the document:
http://docutils.sourceforge.net/docs/ref/rst/directives.html
In docutils, it is called as code
directive. But we don't support it at this moment (see #2155).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, this works now since at least 0.14. See here. Did I miss something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is an alias mainly for English. We can use it even for other languages. But docutils emits level messages.
$ cat test.py
.. code-block:: python
"hello"
In English mode, it works well (I specified -r 1
to display INFO message):
$ rst2html.py -r 1 -l en test.py > /dev/null
$
In Japanese mode, it also works well. But it emits a INFO message:
$ rst2html.py -r 1 -l ja test.py > /dev/null
test.py:1: (INFO/1) No directive entry for "code-block" in module "docutils.parsers.rst.languages.ja".
Using English fallback for directive "code-block".
In addition, :dudir:
role creates a hyperlink for http://docutils.sourceforge.net/docs/ref/rst/directives.html#%s
, but it does not exist.
So this hyperlink will be broken. Did you check it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, so it's just an alias. My mistake. Given that we don't support the 'code' directive, as you note, I'll just drop this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
|
||
:fieldname: Field content | ||
|
||
Sphinx provides custom behavior for field lists compared to docutils. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not a custom behavior of Sphinx. In docutils, it is called as bibliographic fields.
http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#bibliographic-fields
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The point I was trying to make was that these bibliographic fields do different things in Sphinx compared to docutils. In docutils, these would simply be rendered. In Sphinx, they trigger special behavior. Therefore, Sphinx provides custom behavior. Did I miss something or is there some way I can clarify this further?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about "custom behavior for bibliographic fields"? I think file-wide metadata is a kind of bibliographic fields.
Side note:
In docutils, these would simply be rendered.
The behavior can be changed from command line option. For example, LaTeX writer uses them to create cover page.
$ rst2latex.py --help
...
--use-docutils-docinfo Attach author and date to the document info table.
(default)
--use-latex-docinfo Attach author and date to the document title.
...
$ cat test.py
:author: sphinx
:date: today
hello world
default:
$ rst2latex.py test.py | grep -A10 -e 'Title Data' -e 'Body'
%%% Body
\begin{document}
% Docinfo
\begin{center}
\begin{tabularx}{\DUdocinfowidth}{lX}
\textbf{Author}: &
sphinx \\
\textbf{Date}: &
today \\
\end{tabularx}
With --use-latex-docinfo
option:
$ rst2latex.py --use-latex-docinfo test.py | grep -A10 -e 'Title Data' -e 'Body'
%%% Title Data
\title{}
\author{sphinx}
\date{today}
%%% Body
\begin{document}
\maketitle
hello world
\end{document}
You can see that LaTeX writer converts the bibliographic fields to Title Data columns.
Inside docutils, the bibliographic fields are represented as docinfo node. And it is described as following:
The docinfo element may be rendered as a two-column table or in other styles. It may even be invisible or omitted from the processed output.
In semantic view, they are only represents metadata. And the representers (a.k.a. writers and builders) should outputs it in better form. (I know almost users does not mind that, and they consider docinfo will be rendered.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about "custom behavior for bibliographic fields"? I think file-wide metadata is a kind of bibliographic fields.
Yup, this makes sense to me. So long as it get's the custom aspect of this across, I'm happy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
:rst:dir:`default-domain` directive) will have the domain name explicitly | ||
prepended when named (e.g., when the default domain is C, Python functions | ||
will be named "Python function", not just "function"). | ||
The name of the default :doc:`domain </usage/restructuredtext/domains>`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you replace :ref:
by :doc:
? I think cross referencing by labels is more flexible than by docnames.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tend to prefer doc
when referring to an entire document and ref
when I want to refer to specific section of a doc. Basically, if I see an anchor at the very top of a file, I replace it with doc
. I've done this in a few places here so I hope it's not a big deal 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I understand.
encountered. If there is no ``highlight`` directive in the file, the global | ||
highlighting language is used. This defaults to ``python`` but can be | ||
configured using the :confval:`highlight_language` config value. The following | ||
values are supported: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If there is no
highlight
directive in the file, the global highlighting language is used.
This is wrong. The project global highlighting language (a.k.a. highlight_language
confval) is used until highlight
directive appeared first in the file:
::
This literal block is highlighted by :confval:`highlight_language`.
.. highlight:: some-lang
This literal block is highlighted by `highlight` directive.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, this was the original text. However, as you note below this was actually wrong. I'll update this accordingly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually this makes sense as is. If there is no highlight
directive in the file. If there is no highlight
directive in the file, you cannot expect to encounter it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, let's get updated :-)
values are supported: | ||
|
||
* ``none`` (no highlighting) | ||
* ``python`` (the default when :confval:`highlight_language` isn't set) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, the original text was wrong. Now, 'default'
has been used for default setting.
Changed in version 1.4: The default is now 'default'. It is similar to 'python3'; it is mostly a superset of 'python'. but it fallbacks to 'none' without warning if failed. 'python3' and other languages will emit warning if failed. If you prefer Python 2 only highlighting, you can set it back to 'python'.
http://www.sphinx-doc.org/en/master/config.html#confval-highlight_language
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I can correct this.
|
||
>>> 1 + 1 | ||
2 | ||
>>> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the description for doctest_block
notation was lost. Is that intended?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I doubt it. Let me look again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved this into the basics
document since it's a docutils feature (that I never knew about!)
|
||
Use it like this:: | ||
This language is used until the next ``highlight`` directive is encountered. | ||
As discussed previously, ``language`` can be any lexer alias supported by |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*language*
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1, I will change this.
No problem 😄 Delighted to have this reviewed 🎉 |
This is taken from the existing 'rest' documents. Little to no modifications are necessary, thankfully. Signed-off-by: Stephen Finucane <stephen@that.guru>
This is essentially the 'markup/inline' with a few small fixes. There are also some modifications to the basic rST guide to highlight what a role is; we were doing this for directives but not roles. Signed-off-by: Stephen Finucane <stephen@that.guru>
These are very poorly documented at present, especially given their power and use in basically all non-Napoleon based Python documentation. Signed-off-by: Stephen Finucane <stephen@that.guru>
These are going to form the basis of a future 'directive' document, so we do some cleanup before this happens. There are a number of cleanup items. - Some paragraphs are reworded or clarified - Semantic markup is added where possible - Everything is wrapped to ~80 characters Signed-off-by: Stephen Finucane <stephen@that.guru>
There's simply no need to artificially divide up the documentation on directives into multiple, hard-to-navigate documents. Gather all documentation for directives into one easy-to-reference guide, starting with the former 'toctree' document. There are no changes to the content. Signed-off-by: Stephen Finucane <stephen@that.guru>
Continue building up this combined doc by adding the contents of the former 'para' document. There are no changes to the content. Signed-off-by: Stephen Finucane <stephen@that.guru>
Continue building up this combined doc by adding the contents of the former 'code' document. There are no changes to the content. Signed-off-by: Stephen Finucane <stephen@that.guru>
Finish building up this combined doc by adding the contents of the former 'misc' document. There are no changes to the content. Signed-off-by: Stephen Finucane <stephen@that.guru>
This makes the section more consistent with the rest of the document. This adds a new section to the basics guide for doctest blocks, which are a docutils thing. It also update the default highlight language, which is now 'default'. Signed-off-by: Stephen Finucane <stephen@that.guru>
Signed-off-by: Stephen Finucane <stephen@that.guru>
Does this already ready to merge? I can't check that because this has very large changeset. BTW, couldn't you force push on updating? That also forces me to search changes. With appending commits, github shows me changes from last view. It is easy to review to me. |
Yes, I've resolved all your comments. I think it's ready to merge.
That's fair. I'm used to using Gerrit which works on a patch-level basis so force pushing is expected there. I will not force push going forward. |
Merged. Thank you for great work! |
Add a reStructuredText section to the user guide. This is mostly done by shuffling around existing content. The only complicated piece is the combination of four existing documents, listed below, into one. These are merged in the following order.
doc/markup/toctree.rst
doc/markup/code.rst
doc/markup/para.rst
doc/markup/misc.rst
These are all placed into the new file,
doc/usage/restructuredtext/directives.rst
.I'm not sure how to make this easier to review. I have ensured that rework is kept separate from renaming where this might cause confusion but that's all I can do. Let me know if I can help!