PEP 8: Add note regarding exception message string literals #2606
Conversation
pep-0008.txt
Outdated
|
|
||
| Strive to keep exception message string literals on one line, | ||
| in order to make them easily greppable. | ||
| If your exception message exceeds the maximum line length, |
There was a problem hiding this comment.
This could be a link, but equally it follows the line length section directly
Edit: non-commital suggestion as I wasn't very clear on what I meant:
| If your exception message exceeds the maximum line length, | |
| If your exception message exceeds the `maximum line length`_, |
There was a problem hiding this comment.
I'm not sure it's needed, but I'm also fine with a link. I'll leave it for you and Christopher to decide
(IIRC, both of you have the commit bit here, so feel free to apply the suggestion when you've decided.)
There was a problem hiding this comment.
I'm not sure it's needed, but I'm also fine with a link.
I'm not sure it is either, since it is a short section and immediately above. But I don't have a strong opinion.
I'll leave it for you and Christopher to decide slightly_smiling_face
You mean @Rosuav ? Or are you referring to me (CAM)?
There was a problem hiding this comment.
I was referring to you, CAM :) (Actually, I was unaware that there was more than two PEP editors)
There was a problem hiding this comment.
I was unaware that there was more than two PEP editors
Guido, Barry, Brett, ChrisA, Jelle, Adam, Cam, and Hugo are the current complement -- a few more than two!
A
There was a problem hiding this comment.
It's okay, most of us are invisible :) For what it's worth, I have no opinion on the specific wording, but I fully approve of the sentiment here.
There was a problem hiding this comment.
Can I mark this as resolved? Did you decide about the link?
pep-0008.txt
Outdated
|
|
||
| Strive to keep exception message string literals on one line, | ||
| in order to make them easily greppable. | ||
| If your exception message exceeds the maximum line length, |
There was a problem hiding this comment.
I'm not sure it's needed, but I'm also fine with a link.
I'm not sure it is either, since it is a short section and immediately above. But I don't have a strong opinion.
I'll leave it for you and Christopher to decide slightly_smiling_face
You mean @Rosuav ? Or are you referring to me (CAM)?
|
@warsaw What do you think? It’s a bit of a u-turn but I think grepping for errors would be more effective. |
Definitely wait for Barry to review.
I wonder if we should generalize to "error message"?
Sure. I should've done the same for PEP 7; sorry 'bout the rushed merge.
+1 |
Agreed
+1 I guess I'm +0 on this change for PEP 8. We have to be careful not to overload PEP 8 for every possible corner case, but I agree with @gvanrossum on greppability. Let me quibble just a bit with the wording. |
pep-0008.txt
Outdated
| @@ -241,6 +241,17 @@ Another such case is with ``assert`` statements. | |||
|
|
|||
| Make sure to indent the continued line appropriately. | |||
|
|
|||
|
|
|||
| Error Message String Literals | |||
There was a problem hiding this comment.
I think just saying "Error messages" would be sufficient. I have to pause to think about what "Error message string literals" are. Exception strings? Something else? Is there something special about string literals for errors?
There was a problem hiding this comment.
My one concern with that is that readers might think its talking about the wording/formatting of error messages themself, rather than concerned with how string literals for error messages are formatted in the code. But I can see both sides of this.
There was a problem hiding this comment.
How about just dropping "Literals"?
"Strive to keep error message strings on one line"
vs.
"Strive to keep error messages on one line"
There was a problem hiding this comment.
PTAL; I've removed "literals".
pep-0008.txt
Outdated
| Error Message String Literals | ||
| ----------------------------- | ||
|
|
||
| Strive to keep error message string literals on one line, |
There was a problem hiding this comment.
s/error message string literals/error messages/
| Strive to keep error message string literals on one line, | ||
| in order to make them easily greppable. | ||
| If your error message exceeds the maximum line length, | ||
| try to mitigate it by reducing indent, or just bend that rule. |
There was a problem hiding this comment.
Maybe we don't need the "or just bend that rule." and the invocation of practicality over purity? All of that should be well understood if you read PEP 8.
There was a problem hiding this comment.
I suppose the benefit of making it explicit is that it establishes that preserving greppability should generally take priority over following the line length restriction, which is otherwise left ambigious by the current wording and thus would seem to neuter much of the intent of the addition here, perhaps.
There was a problem hiding this comment.
I prefer the explicity of "or just bend that rule", but I will remove it if there is a strong opposition to it.
FWIW, I'm inclined to not applying this generalisation to the PEP 7 change (#2605). |
|
I've thought about this over the weekend. @gvanrossum I don't know, I kind of think maybe this and the PEP 7 changes shouldn't go in. It's seems vague and less focused on making code readable than the effects of that code. PEP 8 is already huge, so I think the bar has to be pretty high on the "this helps readability" scale. |
Ok. Again, sorry for not waiting on your review. I'll be more careful next time. I'll prepare a PR for reverting this. |
|
In the face of Barry's strong opinion I back out on updating the PEPs. I still think we should generally not be breaking up error messages across lines. |
Ditto |
)" Based on post-merge feedback, it was decided to revert this change. See the original PR (pythonGH-2605), the proposed PR for PEP 8 (pythonGH-2606), and the issue (pythonGH-2604) for the discussions. This reverts commit c787739.
Resolves #2604