Issue 53094: Multi with statement

Keyboard Shortcuts

	File
u :	up to issue
m :	publish + mail comments
M :	edit review message
j / k :	jump to file after / before current file
	Side-by-side diff
i :	toggle intra-line diffs
e :	expand all comments
c :	collapse all comments
s :	toggle showing all comments
n / p :	next / previous diff chunk or comment
N / P :	next / previous comment
<Enter> :	respond to / edit current comment

	Issue
u :	up to list of issues
m :	publish + mail comments
j / k :	jump to patch after / before current patch
o / <Enter> :	open current patch in side-by-side view
i :	open current patch in unified diff view

	Issue List
j / k :	jump to issue after / before current issue
o / <Enter> :	open current issue
# :	close issue

	Comment/message editing
<Ctrl> + s :	save comment
<Esc> :	cancel edit

Issue 53094: Multi with statement

Can't Edit
Can't Publish+Mail
Start Review

Created:
6 months, 3 weeks ago by Georg

Modified:
3 months, 3 weeks ago

Reviewers:
Benjamin

CC:

SVN Base:
http://svn.python.org/view/*checkout*/python/trunk/

Visibility:
Public.

Patch Set 2 : Version after review by Benjamin

Created: 6 months, 3 weeks ago

Download raw patch set

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats			Patch
M	Grammar/Grammar	View		1 chunk	15 lines	0 comments	Download
M	Include/graminit.h	View		1 chunk	13 lines	0 comments	Download
M	Lib/compiler/transformer.py	View		1 chunk	38 lines	0 comments	Download
M	Lib/test/test_compiler.py	View		1 chunk	32 lines	0 comments	Download
M	Lib/test/test_parser.py	View		1 chunk	12 lines	0 comments	Download
M	Lib/test/test_with.py	View	1	1 chunk	94 lines	0 comments	Download
M	Modules/parsermodule.c	View		1 chunk	59 lines	0 comments	Download
M	Python/ast.c	View	1	2 chunks	89 lines	0 comments	Download
M	Python/graminit.c	View		2 chunks	78 lines	0 comments	Download

Messages

Total messages: 2

Expand All Messages | Collapse All Messages

Benjamin

Looks good except for nits. http://codereview.appspot.com/53094/diff/1/9 File Lib/test/test_compiler.py (right): http://codereview.appspot.com/53094/diff/1/9#newcode186 Line 186: self.assertEquals(dct.get('result'), 1) Why ...

6 months, 3 weeks ago

Georg

All done, I guess. http://codereview.appspot.com/53094/diff/1/9 File Lib/test/test_compiler.py (right): http://codereview.appspot.com/53094/diff/1/9#newcode186 Line 186: self.assertEquals(dct.get('result'), 1) On 2009/05/02 ...

6 months, 3 weeks ago

All done, I guess.

http://codereview.appspot.com/53094/diff/1/9
File Lib/test/test_compiler.py (right):

http://codereview.appspot.com/53094/diff/1/9#newcode186
Line 186: self.assertEquals(dct.get('result'), 1)
On 2009/05/02 18:22:06, Benjamin wrote:
> Why using .get() here?

Following the above example.  Probably to not get an error but a test failure if
it isn't there.

http://codereview.appspot.com/53094/diff/1/7
File Lib/test/test_with.py (right):

http://codereview.appspot.com/53094/diff/1/7#newcode657
Line 657: class NestedWith(unittest.TestCase):
On 2009/05/02 18:22:06, Benjamin wrote:
> You should test that the second's __exit__ is called even if the first one
> raises.

Done.

http://codereview.appspot.com/53094/diff/1/3
File Python/ast.c (right):

http://codereview.appspot.com/53094/diff/1/3#newcode3045
Line 3045: assert(TYPE(n) == with_stmt);
On 2009/05/02 18:22:06, Benjamin wrote:
> REQ() is better here.

It was already there, but Done.

Expand All Messages | Collapse All Messages