-
-
Notifications
You must be signed in to change notification settings - Fork 32.9k
WIP: Use assign expr for "while True: ... break" #8095
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -237,16 +237,12 @@ def binhex(inp, out): | |
|
||
with io.open(inp, 'rb') as ifp: | ||
# XXXX Do textfile translation on non-mac systems | ||
while True: | ||
d = ifp.read(128000) | ||
if not d: break | ||
while (d := ifp.read(128000)): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a bigger win than it looks like. Note that the original "if not d: break" on a single line was itself arguably a style violation because the break doesn't start a line, even though it might change flow-control. That said, I would have done the same thing as the original author to keep it short; it feels like a guard instead of a real control "path". So this is an area where vertical space matters, the "while True: " is ugly, and the current spelling is already doing slightly ugly things to avoid drawing extra emphasis ... and shortening it with the inline assignment improves on all three. |
||
ofp.write(d) | ||
ofp.close_data() | ||
|
||
ifp = openrsrc(inp, 'rb') | ||
while True: | ||
d = ifp.read(128000) | ||
if not d: break | ||
while (d := ifp.read(128000)): | ||
ofp.write_rsrc(d) | ||
ofp.close() | ||
ifp.close() | ||
|
@@ -460,19 +456,15 @@ def hexbin(inp, out): | |
|
||
with io.open(out, 'wb') as ofp: | ||
# XXXX Do translation on non-mac systems | ||
while True: | ||
d = ifp.read(128000) | ||
if not d: break | ||
while (d := ifp.read(128000)): | ||
ofp.write(d) | ||
ifp.close_data() | ||
|
||
d = ifp.read_rsrc(128000) | ||
if d: | ||
ofp = openrsrc(out, 'wb') | ||
ofp.write(d) | ||
while True: | ||
d = ifp.read_rsrc(128000) | ||
if not d: break | ||
while (d := ifp.read_rsrc(128000)): | ||
ofp.write(d) | ||
ofp.close() | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -349,13 +349,7 @@ def scan(self, string): | |
append = result.append | ||
match = self.scanner.scanner(string).match | ||
i = 0 | ||
while True: | ||
m = match() | ||
if not m: | ||
break | ||
j = m.end() | ||
if i == j: | ||
break | ||
while (m := match()) and (j := m.end()) == i: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't that reverse the test on j? As in, shouldn't it be Though I'm not sure which way this argues; the original logic was at least as hard to follow as the new code. |
||
action = self.lexicon[m.lastindex-1][1] | ||
if callable(action): | ||
self.match = m | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are you using parentheses here? I don't think they are syntactically necessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm too lazy to read the latest PEP, and the current implementation of the PEP is outdated, so I added parenthesis even if they are not needed.