twisted / twisted

Event-driven networking engine written in Python.
https://twisted.org
Other
5.59k stars 1.17k forks source link

Improve rendering of twisted.python.usage related documents #7683

Open twisted-trac opened 10 years ago

twisted-trac commented 10 years ago
multani's avatar @multani reported
Trac ID trac#7683
Type enhancement
Created 2014-10-16 13:53:08Z
Branch https://github.com/twisted/twisted/tree/doc-fixes-7683

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:

Searchable metadata ``` trac-id__7683 7683 type__enhancement enhancement reporter__multani multani priority__normal normal milestone__ branch__branches_doc_fixes_7683 branches/doc-fixes-7683 branch_author__tenth tenth status__new new resolution__None None component__core core keywords__doc doc time__1413467588305380 1413467588305380 changetime__1416161109891824 1416161109891824 version__None None owner__multani multani ```
twisted-trac commented 9 years ago
exarkun's avatar @exarkun set owner to @multani

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.

twisted-trac commented 10 years ago
multani's avatar @multani commented

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 )

twisted-trac commented 10 years ago
tenth10th's avatar @tenth10th commented

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

twisted-trac commented 9 years ago
adiroiban's avatar @adiroiban commented

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

What do you think?

twisted-trac commented 9 years ago
exarkun's avatar @exarkun commented

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.

twisted-trac commented 9 years ago
adiroiban's avatar @adiroiban commented

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.

twisted-trac commented 9 years ago
multani's avatar @multani commented

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.

twisted-trac commented 9 years ago
multani's avatar @multani commented

Replying to adiroiban:

Another comment. Maybe this patch should also include the changes from [#7329](https://github.com/twisted/twisted/issues/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](https://github.com/twisted/twisted/issues/7329)](https://github.com/twisted/twisted/issues/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.

twisted-trac commented 9 years ago
adiroiban's avatar @adiroiban commented

From my part, is ok to merge this patch.

twisted-trac commented 9 years ago
multani's avatar @multani commented

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?