Skip to content

bpo-31537: Fix bug in readline documentation example code #3925

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

Merged
merged 1 commit into from
Oct 10, 2017

Conversation

infinitewarp
Copy link
Contributor

@infinitewarp infinitewarp commented Oct 9, 2017

Use get_current_history_length to get the current length of the history, not get_history_length. This fixes an issue where the example code's call to append_history_file in the save function would never actually write any lines to the file.

Fixes https://bugs.python.org/issue31537

https://bugs.python.org/issue31537

@@ -0,0 +1,2 @@
Fix incorrect usage of `get_history_length` in readline documentation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use double backticks: ``get_history_length``

Single backtick means Sphinx will use the value of the default_role setting to decide what to do with a markup like `foo bar` and it can cause regressions in documentation if someone changes the value of default_role by using the default-role directive:

`foo bar`

.. default-role:: subscript

`spam eggs`

will be rendered as

<p><cite>foo bar</cite></p>
<p><sub>spam eggs</sub></p>

See http://docutils.sourceforge.net/docs/ref/rst/directives.html#default-role for details.

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I didn't expect the Spanish Inquisition!. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

Use `get_current_history_length` to get the current length of the history,
not `get_history_length`. This fixes an issue where the example code's
call to `append_history_file` in the `save` function would never actually
write any lines to the file.
@infinitewarp
Copy link
Contributor Author

I didn't expect the Spanish Inquisition!

@bedevere-bot
Copy link

Nobody expects the Spanish Inquisition!

@berkerpeksag, @Mariatta: please review the changes made to this pull request.

Copy link
Member

@berkerpeksag berkerpeksag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, thank you!

@miss-islington
Copy link
Contributor

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

@Mariatta
Copy link
Member

Thanks @infinitewarp and congrats on your first contribution to CPython 🌮

@Mariatta Mariatta added the docs Documentation in the Doc dir label Oct 10, 2017
@miss-islington
Copy link
Contributor

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

@bedevere-bot
Copy link

GH-3948 is a backport of this pull request to the 3.6 branch.

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Oct 10, 2017
Change the code example from using `get_history_length` to `get_current_history_length`.
(cherry picked from commit eeb5ffd)
Mariatta pushed a commit that referenced this pull request Oct 10, 2017
Change the code example from using `get_history_length` to `get_current_history_length`.
(cherry picked from commit eeb5ffd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants