Change iter.next() to next(iter) - #9535
Conversation
|
CC @LeonarddeR - let's defer this until speech refactor lands. |
| self._gen = self.gen(eventName, obj) | ||
| try: | ||
| self.next() | ||
| next(self) |
There was a problem hiding this comment.
I'm afraid this is subject to serious breakage as it is now. You're calling next(self) instead of self.next(). However, next is a method on self. Here, you're not dealing with a real iterator, but with a class that only inherits from object.
I'd like to suggest to revert these changes to eventHandler altogether.
| @@ -49,7 +49,7 @@ def readObjects(obj): | |||
| _startGenerator(readObjectsHelper_generator(obj)) | |||
There was a problem hiding this comment.
All changes to this file are going to conflict with #7599. Please revert these as well. They will be addressed during the Python 3 transition.
There was a problem hiding this comment.
I've reverted the changes.
LeonarddeR
left a comment
LeonarddeR
left a comment
There was a problem hiding this comment.
@feerrenrut: This is now a one liner, so this can be merged into master I think.
I'm sorry that I had to request to strip down the pull request this way.
|
Wouldn't be better to wait for merging speech refactor and then check if sayAllHandler still use iter.next()? |
Yes, I think this makes sense. You will need to change the target of this PR to be the threshold branch, which is where we will be doing the python 3 work. |
|
I believe that with dropping speechCompat, there is no longer a call of next like this in sayAllHandler. |
|
I changed target branch to threshold and i found that sayAllHandler uses "reader.next()" in line 40 (readObjects function). |
|
Nope, please leave this as is. next is a function on _ObjectsReader.
|
|
So i think it can be merged as it is. |
5 participants
Link to issue number:
closes #9086
Summary of the issue:
See issue description.
Description of how this pull request fixes the issue:
As suggested I've changed iter.next() to next(iter).
Testing performed:
Only ran NVDA from source, because I'm not sure whether it needs any special testing scenerios.
Known issues with pull request:
None
Change log entry:
None
cc @josephsl