Remove assert from __eq__ for windows
m1guelperez opened this issue · 5 comments
for window in session.list_windows():
if window.name == "test_window":
target_window = window
break
if target_window == None:
target_window = session.new_window(attach=True, window_name="test_window")
target_window.select_window()
target_window.panes[0].send_keys("exec zsh")The code above will crash, when the window exists and we then check the if target_window == None because of:
def __eq__(self, other: object) -> bool:
assert isinstance(other, Window)
return self.window_id == other.window_idI guess it would be better to remove the assert and return false. Should I do a PR?
Can you summarize / explain the code snippet a bit more? I formatted the code
. Should I do a PR?
Yes. If you make a PR, I can examine it closer.
Consider you iterate through all existing windows, to see if a window with a particular name already exists.
We basically use target_window here as placeholder. If the window we search does not exist it will be None.
However, in the window.py file, the == is implemented as the code I provided. But I do not think that there should be an assert.
The method to check equal for windows should be implemented like that:
def __eq__(self, other: object) -> bool:
if not isinstance(other, Window):
return False
return self.window_id == other.window_idIs it clearer now?
I may not be able to look at the PR / related code until this weekend
Is it clearer now?
Yes
What happens if you replace if target_window == None with if target_window is None?
What happens if you replace
if target_window == Nonewithif target_window is None?
That should indeed work! However, I think we should still implement the __eq__ in a different way or?
Closed #505.