Opened 3 years ago

Last modified 3 years ago

#7683 enhancement new

Improve rendering of twisted.python.usage related documents

Reported by: Jonathan Ballet Owned by: Jonathan Ballet
Priority: normal Milestone:
Component: core Keywords: doc
Cc: Branch: branches/doc-fixes-7683
branch-diff, diff-cov, branch-cov, buildbot
Author: tenth

Description (last modified by Jean-Paul Calderone)

While reviewing #7330 I read some of the doc related to twisted.python.usage and there are documents which are mis-rendered:

I will propose a patch which slightly improves the rendering of these documents. There are only cosmetic changes.

Attachments (2)

7683-1.patch (10.9 KB) - added by Jonathan Ballet 3 years ago.
7683-2.patch (11.1 KB) - added by Jonathan Ballet 3 years ago.
Updates from 3

Download all attachments as: .zip

Change History (12)

Changed 3 years ago by Jonathan Ballet

Attachment: 7683-1.patch added

comment:1 Changed 3 years ago by Jonathan Ballet

Keywords: review added

I dug the history to get the links for "I DON’T KNOW WHAT TO DO WITH THIS LINK!" (from https://twistedmatrix.com/trac/browser/tags/releases/twisted-13.2.0/doc/core/howto/options.xhtml#L416 )

comment:2 Changed 3 years ago by David Sturgis

Author: tenth
Branch: branches/doc-fixes-7683

(In [43424]) Branching to 'doc-fixes-7683'

comment:3 Changed 3 years ago by Adi Roiban

Changes looks good. Both apidocs and narative docs. Thanks!

Not sure what to say about this change. Maybe is OK to also keep PROGRAM_COMMAND in the example to make it clear that is a sub-command called after command.

-    In this case, C{"<program> holyquest --horseback --for-grail"} will cause
+    In this case, C{"holyquest --horseback --for-grail"} will cause

There is no newsfile for this change.

comment:4 Changed 3 years ago by Adi Roiban

Another comment. Maybe this patch should also include the changes from #7329 to have a single patch for this documentation.

What do you think?

comment:5 in reply to:  3 Changed 3 years ago by Jonathan Ballet

Hi adiroiban,

thank for the review!

Replying to adiroiban:

Not sure what to say about this change. Maybe is OK to also keep PROGRAM_COMMAND in the example to make it clear that is a sub-command called after command.

-    In this case, C{"<program> holyquest --horseback --for-grail"} will cause
+    In this case, C{"holyquest --horseback --for-grail"} will cause

I don't ... know why I removed this. You are right, <program> has to stay here, it doesn't really make sense otherwise.

There is no newsfile for this change.

I will add one.

Changed 3 years ago by Jonathan Ballet

Attachment: 7683-2.patch added

Updates from 3

comment:6 in reply to:  4 Changed 3 years ago by Jonathan Ballet

Replying to adiroiban:

Another comment. Maybe this patch should also include the changes from #7329 to have a single patch for this documentation.

What do you think?

Sadly, I didn't see (look for) #7329 before posting my patch :(

I had a look at this patch, and it's a subset of the one I posted, plus the changes on :: don't necessarily improve things as Docutils already knows how to deal with these (see http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#literal-blocks)

I know #7329 has ancestry priority over my patch, but mine fixes the same things plus all the rest. It's not very nice for the work of fbrennen and rwall in #7329 but I think it would be easier to close #7329 and merge mine instead.

comment:7 Changed 3 years ago by Adi Roiban

From my part, is ok to merge this patch.

comment:8 Changed 3 years ago by Jean-Paul Calderone

Keywords: review removed
Owner: set to Jonathan Ballet

Thanks. There are some good changes here, but they're not all good. Some of the new C{} usage is incorrect (for example, --help is not code to be marked up; likewise for doc/core/howto/options.rst).

I'm probably going to apply #7329 shortly. Then this patch can be updated to remove the changes that that ticket makes and to address the other minor formatting issues.

comment:9 Changed 3 years ago by Jean-Paul Calderone

Description: modified (diff)

https://twistedmatrix.com/documents/current/api/twisted.python.usage.Options.html is not exactly readable as it lacks proper reStructuredText formatting

Note that this is not a valid defect description. Twisted uses epytext markup for docstrings - not reStructuredText. Since #7329 fixes the epytext markup issues, updating the description to remove this point.

comment:10 in reply to:  8 Changed 3 years ago by Jonathan Ballet

Replying to exarkun:

Thanks. There are some good changes here, but they're not all good. Some of the new C{} usage is incorrect (for example, --help is not code to be marked up; likewise for doc/core/howto/options.rst).

I can't find another way in Epydoc's documention to mark inline text to be rendered as "fixed font" formatting, like C{} does. (Plus, I was following the existing documentation style in the same file which does this, for Completion/Unix/Command/_twisted for example.)

Is there another preferred way, or do I have to remove them?

Note: See TracTickets for help on using tickets.