Skip to content

bpo-30721: Addresses minor fixes of adding ? to redirection syntax#3131

Merged
ncoghlan merged 2 commits into
python:masterfrom
CuriousLearner:fixes-issue30721
Aug 18, 2017
Merged

bpo-30721: Addresses minor fixes of adding ? to redirection syntax#3131
ncoghlan merged 2 commits into
python:masterfrom
CuriousLearner:fixes-issue30721

Conversation

@CuriousLearner
Copy link
Copy Markdown
Member

@CuriousLearner CuriousLearner commented Aug 18, 2017 •
edited by bedevere-bot
Loading

Addresses the minor fixes for PR #2345

cc @ncoghlan

https://bugs.python.org/issue30721

ncoghlan
ncoghlan requested changes Aug 18, 2017
View reviewed changes
Copy link
Copy Markdown
Contributor

@ncoghlan ncoghlan left a comment

Choose a reason for hiding this comment

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

While English does in general suggest that trailing punctuation should go inside quotation marks (although I've never personally understood that guideline), in this case it should definitely be outside, as what's inside is expected to be a Python expression, not English text.

Comment thread Objects/abstract.c Outdated
"unsupported operand type(s) for %.100s: "
"'%.100s' and '%.100s'. Did you mean \"print(<message>, "
"file=<output_stream>)\"",
"file=<output_stream>)?\"",
Copy link
Copy Markdown
Contributor

@ncoghlan ncoghlan Aug 18, 2017

Choose a reason for hiding this comment

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

The question mark should be outside the quote marks, since it isn't part of the suggested Python code.

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Aug 18, 2017

A Python core developer, ncoghlan, 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 ncoghlan along with any other core developers
who have left a review that you're ready for them to take another look
at this pull request.

@CuriousLearner
Copy link
Copy Markdown
Member Author

CuriousLearner commented Aug 18, 2017

I didn't expect the Spanish Inquisition!

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Aug 18, 2017

Nobody expects the Spanish Inquisition!

@ncoghlan: please review the changes made to this pull request.

@ncoghlan ncoghlan merged commit a7c449b into python:master Aug 18, 2017
@CuriousLearner
Copy link
Copy Markdown
Member Author

CuriousLearner commented Aug 18, 2017

Thanks @ncoghlan :)

ncoghlan pushed a commit to ncoghlan/cpython that referenced this pull request Aug 19, 2017
@CuriousLearner @ncoghlan
bpo-30721: Add missing '?' to new error message (pythonGH-3131)
c6ff845
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ncoghlan ncoghlan ncoghlan approved these changes

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@CuriousLearner @bedevere-bot @ncoghlan @Mariatta @the-knights-who-say-ni