tmux-python/libtmux

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_id

I guess it would be better to remove the assert and return false. Should I do a PR?

tony commented

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_id

Is it clearer now?

tony commented

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 == None with if target_window is None?

That should indeed work! However, I think we should still implement the __eq__ in a different way or?

Closed #505.