Skip to content
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

Update pep-0008.txt #438

Merged
merged 15 commits into from
Nov 6, 2017
Merged

Update pep-0008.txt #438

merged 15 commits into from
Nov 6, 2017

Conversation

toonarmycaptain
Copy link
Contributor

@toonarmycaptain toonarmycaptain commented Oct 25, 2017

Consistency format 'multi-'.
Fixed capitalization of 'latin', 'boolean', 'unicode' to 'Latin', 'Boolean', 'Unicode'.

Clarified contradictions regarding comment formatting. NB Complete sentences should have a period.
Changed to remove reference to short comments without terminating period.
Further refined, corrected incorrect, fixed  singular/plural.
Edit for consistency use 'multi-' in 'multi-line` 'multi-statement` 'multi-sentence` etc with dash rather than space or no splitter (eg 'multi line`, 'multiline`). If a different convention is preferred, do that.
'Latin' is a proper noun, and thus should be capitalized.
Capitalization should be consisted. I propose 'boolean' should always be capitalized, as per https://english.stackexchange.com/questions/4481/should-the-word-boolean-be-capitalized

At present it is inconsistently capitalised.
Changed 'boolean' to 'Boolean'
Update pep-0008.txt - as a proper noun, 'unicode' should be capitalized. Fixed 3 instances.
@brettcannon
Copy link
Member

"boolean" and "unicode" are lowercase on purpose to reference the types in Python and not the concepts.

Removed revisions to 'boolean', 'unicode'. 

Unicode capitalization [issue](https://bugs.python.org/issue31873) under discussion.
@toonarmycaptain
Copy link
Contributor Author

Removed revisions to 'boolean', 'unicode'.

pep-0008.txt Outdated
@@ -1441,3 +1441,4 @@ This document has been placed in the public domain.
fill-column: 70
coding: utf-8
End:

Copy link
Member

Choose a reason for hiding this comment

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

Why the extra blank line?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Error. Fixed.

pep-0008.txt Outdated
@@ -121,13 +121,13 @@ Optional::
var_one, var_two,
var_three, var_four)

.. _`multiline if-statements`:
.. _`multi-line if-statements`:
Copy link
Member

Choose a reason for hiding this comment

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

I guess, but "multiline" is in pretty common usage, especially when talking about Python's multiline strings. Chrome's spellchecker doesn't even complain about it.

-0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Would you prefer to remove the hyphens in the instances that have them at present? I figure, provided there's not a specific reason for variance, that the form should be consistent within the document.

Copy link
Member

Choose a reason for hiding this comment

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

I would prefer it to be "multiline" everywhere. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

pep-0008.txt Outdated
@@ -333,7 +333,7 @@ technical terms are used which aren't English). In addition, string
literals and comments must also be in ASCII. The only exceptions are
(a) test cases testing the non-ASCII features, and
(b) names of authors. Authors whose names are not based on the
latin alphabet MUST provide a latin transliteration of their
Latin alphabet MUST provide a Latin transliteration of their
Copy link
Member

Choose a reason for hiding this comment

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

I think this is really talking about the latin-1 or iso-8859-1 encoding so maybe those terms should be used instead.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fair enough. I'll change it, do you have a preference?
For clarity/readability I'd probably preference latin-1 or iso-8859-1 (latin-1) over just the iso-8859 reference.

Copy link
Contributor

Choose a reason for hiding this comment

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

Personally, I prefer to talk about the alphabet, not the encoding. So "Latin" would be correct (capitalized IMO), but "latin-1" would not be.

Copy link
Member

Choose a reason for hiding this comment

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

Possibly so, but concretely, it's non-latin-1 characters that do (or did) cause technical problems, so it's better to specific and discuss the encoding.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

How about Latin alphabet (specifically the latin-1/ISO-8859-1 encoding) - or something similar? Best of both worlds? Also - ISO or iso`?

Removed trailing newlines
Made the form 'multiline' uniform throughout the document. 

Clarified references to Latin alphabet to direct to the specific character set desired.
pep-0008.txt Outdated
@@ -333,8 +333,8 @@ technical terms are used which aren't English). In addition, string
literals and comments must also be in ASCII. The only exceptions are
(a) test cases testing the non-ASCII features, and
(b) names of authors. Authors whose names are not based on the
latin alphabet MUST provide a latin transliteration of their
names.
Latin (latin-1, ISO/IEC 8859-1 character set) alphabet MUST provide
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this fine, or move 'alphabet' before the brackets:
"names of authors. Authors whose names are not based on the
Latin alphabet (latin-1, ISO/IEC 8859-1 character set) MUST provide
a transliteration of their names in this character set."

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I think the parenthetical comment should come after "alphabet".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Moved  'alphabet' before encoding parenthesis line 336.
Copy link
Member

@warsaw warsaw 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, thanks for your contribution!

@warsaw warsaw merged commit ef1eb2f into python:master Nov 6, 2017
warsaw added a commit that referenced this pull request Nov 6, 2017
@toonarmycaptain toonarmycaptain deleted the pep8-patch-2 branch November 6, 2017 14:06
@gvanrossum
Copy link
Member

@warsaw Why do you have a branch reverting this PR?

@warsaw
Copy link
Member

warsaw commented Nov 11, 2017

Do I? That's weird. Maybe a GitHub pebkac.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants