bpo-35044: Use the :exc: role for the exceptions in the doc by matrixise · Pull Request #10037 · python/cpython

matrixise · 2018-10-22T12:14:22Z

https://bugs.python.org/issue35044

serhiy-storchaka

Too much links is not good. If some thing is mentioned multiple times in one paragraph or in few consequent short paragraph, it is enough to create a link only from the first of them. In many cases links were omitted for purpose.

serhiy-storchaka · 2018-10-22T12:56:08Z

Doc/howto/functional.rst

-Inside a generator function, ``return value`` causes ``StopIteration(value)``
-to be raised from the :meth:`~generator.__next__` method.  Once this happens, or
-the bottom of the function is reached, the procession of values ends and the
+Inside a generator function, ``return value`` causes :exc:`StopIteration` to be


The current wording is more correct. StopIteration(value) will be raised.

serhiy-storchaka · 2018-10-22T12:59:35Z

Doc/whatsnew/3.5.rst

 The interaction of generators and :exc:`StopIteration` in Python 3.4 and
 earlier was sometimes surprising, and could conceal obscure bugs.  Previously,
-``StopIteration`` raised accidentally inside a generator function was
+:exc:`StopIteration` raised accidentally inside a generator function was


There is a link to StopIteration above. Too many links to the same target distract attention.

matrixise · 2018-10-24T16:56:48Z

@serhiy-storchaka do you want to continue the review? thank you

vstinner · 2018-10-24T22:13:24Z

Doc/faq/design.rst

-  would raise a KeyError exception because the id of the ``[1, 2]`` used in the
-  second line differs from that in the first line.  In other words, dictionary
-  keys should be compared using ``==``, not using :keyword:`is`.
+  would raise a :exc:`KeyError` exception because the id of the ``[1, 2]`` used


Please leave "in the" on the same line to reduce the number of modified lines in your change. It's fine if the line is "too long".

vstinner · 2018-10-24T22:13:32Z

Doc/howto/functional.rst


-Inside a generator function, ``return value`` causes ``StopIteration(value)``
-to be raised from the :meth:`~generator.__next__` method.  Once this happens, or
+Inside a generator function, ``return value`` causes ``StopIteration(value)`` to


matrixise · 2018-10-25T05:04:10Z

@vstinner updated, thanks for your review.

serhiy-storchaka · 2018-10-25T06:20:08Z

Doc/whatsnew/3.6.rst

 Import now raises the new exception :exc:`ModuleNotFoundError`
 (subclass of :exc:`ImportError`) when it cannot find a module.  Code
-that current checks for ``ImportError`` (in try-except) will still work.
+that current checks for :exc:`ImportError` in try-except) will still work.


There is already a link to this exception in this paragraph.

I don't think that it's an issue. I prefer to always use :exc: role to always have the same formatting.

If you want to get the same formatting, you can use :exc:`!ImportError`. But it is more verbose.

Since the formatting is not missed, but it just don't create a link, I'm sure this was done for purpose.

I don't why it's an issue to have two links to the same exception in the same paragraph. IMHO it's a good thing :)

suggestions? remove/keep? tell me ;-)
thanks

Links have many disadvantages:

They distract attention.

Unintentional mouse click causes loading different page.

It is harder to copy linked text.

They increase the html file size and the time of its loading and rendering.

Links have great advantages which overwhelm disadvantages. But if repeat the same link multiple times, its advantages are left constant, while disadvantages are increased with every repetition.

@serhiy-storchaka done, thanks

serhiy-storchaka · 2018-10-25T06:20:51Z

Doc/whatsnew/3.7.rst

 :func:`subprocess.run`, or in a :class:`~subprocess.Popen`
 context manager.  It now waits a short amount of time for the child
-to exit, before continuing the handling of the ``KeyboardInterrupt``
+to exit, before continuing the handling of the :exc:`KeyboardInterrupt`


There is already a link to this exception in this paragraph.

serhiy-storchaka · 2018-10-25T06:22:09Z

Misc/NEWS.d/next/Documentation/2018-10-22-14-09-58.bpo-35044.qjvNtI.rst

@@ -0,0 +1 @@
+Fix the documentation with the role exc for the appropriated exception.


"Patch by ..."

And perhaps highlight "exc"? ``exc``

+1 to format properly "exc" :) I hesitated to ask the same change.

@serhiy-storchaka I wanted to use :exc: but there was an exception during the build process of the doc. it's the reason why I simply used exc, but ok I am going to change that. Thanks

miss-islington · 2018-10-26T10:52:14Z

Thanks @matrixise for the PR, and @vstinner for merging it 🌮🎉.. I'm working now to backport this PR to: 3.6, 3.7.
🐍🍒⛏🤖

bedevere-bot · 2018-10-26T10:52:27Z

GH-10122 is a backport of this pull request to the 3.7 branch.

(cherry picked from commit e483f02) Co-authored-by: Stéphane Wirtel <stephane@wirtel.be>

miss-islington · 2018-10-26T10:52:30Z

Sorry, @matrixise and @vstinner, I could not cleanly backport this to 3.6 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker e483f02423917dc4dfd25f46e5b9e6fce304777d 3.6

vstinner · 2018-10-26T10:55:31Z

Sorry, @matrixise and @vstinner, I could not cleanly backport this to 3.6 due to a conflict.

If the backport cannot be automated, I don't think that it's worth it.

Thanks @matrixise for your contribution and @serhiy-storchaka for the review ;-)

(cherry picked from commit e483f02) Co-authored-by: Stéphane Wirtel <stephane@wirtel.be>

bpo-35044: Use the :exc: role for the exceptions in the doc

e81e092

matrixise requested review from gvanrossum and ilevkivskyi as code owners October 22, 2018 12:14

serhiy-storchaka reviewed Oct 22, 2018

View reviewed changes

Fix the doc

89a0373

gvanrossum removed their request for review October 22, 2018 15:19

Update with the recommendations of Serhiy

1c3c462

matrixise force-pushed the bpo-35044 branch from 8f0c6da to 1c3c462 Compare October 22, 2018 15:34

the-knights-who-say-ni added the CLA signed label Oct 22, 2018

bedevere-bot added docs Documentation in the Doc dir awaiting review labels Oct 22, 2018

vstinner reviewed Oct 24, 2018

View reviewed changes

Update with the recommendations of Victor

7fb33cc

serhiy-storchaka reviewed Oct 25, 2018

View reviewed changes

matrixise added 2 commits October 25, 2018 13:57

Use formatting for the role and add Path by...

33918b1

Fix after review

71d425b

matrixise force-pushed the bpo-35044 branch from 6bb4a8c to 090db60 Compare October 25, 2018 14:17

remove the role :exc:

8b69437

matrixise force-pushed the bpo-35044 branch from 090db60 to 8b69437 Compare October 25, 2018 14:19

serhiy-storchaka approved these changes Oct 26, 2018

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting review labels Oct 26, 2018

serhiy-storchaka added needs backport to 3.6 labels Oct 26, 2018

vstinner merged commit e483f02 into python:master Oct 26, 2018

bedevere-bot removed the awaiting merge label Oct 26, 2018

bedevere-bot removed the needs backport to 3.7 label Oct 26, 2018

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 26, 2018

bpo-35044, doc: Use the :exc: role for the exceptions (pythonGH-10037)

84b9f4d

(cherry picked from commit e483f02) Co-authored-by: Stéphane Wirtel <stephane@wirtel.be>

miss-islington assigned vstinner Oct 26, 2018

vstinner removed the needs backport to 3.6 label Oct 26, 2018

miss-islington added a commit that referenced this pull request Oct 26, 2018

bpo-35044, doc: Use the :exc: role for the exceptions (GH-10037)

ec10b70

(cherry picked from commit e483f02) Co-authored-by: Stéphane Wirtel <stephane@wirtel.be>

		@@ -0,0 +1 @@
		Fix the documentation with the role exc for the appropriated exception.

Uh oh!

Conversation

matrixise commented Oct 22, 2018 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matrixise commented Oct 24, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matrixise commented Oct 25, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

matrixise Oct 25, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Oct 26, 2018

Uh oh!

bedevere-bot commented Oct 26, 2018

Uh oh!

miss-islington commented Oct 26, 2018

Uh oh!

vstinner commented Oct 26, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

matrixise commented Oct 22, 2018 •

edited by bedevere-bot

Loading

matrixise Oct 25, 2018 •

edited

Loading