Skip to content
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

crash while using search #923

Closed
klfoulk16 opened this issue Feb 14, 2021 · 5 comments
Closed

crash while using search #923

klfoulk16 opened this issue Feb 14, 2021 · 5 comments
Milestone

Comments

@klfoulk16
Copy link
Collaborator

I was using the zulip search box [q] and I went to edit a former stream search. I deleted the entire search and pressed enter. Then the app crashed. I was unable to replicate this incident so I'm not sure if I should be creating an issue for it?

I am running Zulip Terminal 0.6.0+git on mac in a Z shell....The following was in my zulip-terminal-tracebacks.log:

{e}

'<' not supported between instances of 'NoneType' and 'int'
Traceback (most recent call last):
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zulipterminal/cli/run.py", line 434, in main
Controller(zuliprc_path,
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zulipterminal/core.py", line 438, in main
self.loop.run()
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zt_venv/lib/python3.9/site-packages/urwid/main_loop.py", line 287, in run
self._run()
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zt_venv/lib/python3.9/site-packages/urwid/main_loop.py", line 385, in _run
self.event_loop.run()
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zt_venv/lib/python3.9/site-packages/urwid/main_loop.py", line 790, in run
self._loop()
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zt_venv/lib/python3.9/site-packages/urwid/main_loop.py", line 827, in _loop
self._watch_filesfd
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zt_venv/lib/python3.9/site-packages/urwid/raw_display.py", line 416, in
wrapper = lambda: self.parse_input(
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zt_venv/lib/python3.9/site-packages/urwid/raw_display.py", line 515, in parse_input
callback(processed, processed_codes)
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zt_venv/lib/python3.9/site-packages/urwid/main_loop.py", line 412, in _update
self.process_input(keys)
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zt_venv/lib/python3.9/site-packages/urwid/main_loop.py", line 513, in process_input
k = self._topmost_widget.keypress(self.screen_size, k)
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zulipterminal/ui.py", line 181, in keypress
return self.controller.current_editor().keypress((size[1],), key)
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zulipterminal/ui_tools/boxes.py", line 1592, in keypress
self.panel_view.keypress(size, 'esc')
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zulipterminal/ui_tools/views.py", line 369, in keypress
self.log.set_focus(self.focus_index_before_search)
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zt_venv/lib/python3.9/site-packages/urwid/listbox.py", line 203, in set_focus
self.focus = position
File "/Users/kellyfoulk/Documents/code/zulip/zulip-terminal/zt_venv/lib/python3.9/site-packages/urwid/monitored_list.py", line 164, in _set_focus
if index < 0 or index >= len(self):
TypeError: '<' not supported between instances of 'NoneType' and 'int'

@klfoulk16 klfoulk16 changed the title zulip terminal crash while using search crash while using search Feb 14, 2021
@Rohitth007
Copy link
Collaborator

Rohitth007 commented Apr 1, 2021

@neiljp
This happened with me too...
Finally figured out what caused it!!
Steps to reproduce bug:

  • Search for something which you know for sure doesn't exist, eg aaaaaa
  • Press Enter
  • Press any key other than q, esc or any other shortcut key-binding that triggers an event. Eg: down-arrow
  • Now press esc

Zulip crashes

Related issue: #975

@Rohitth007
Copy link
Collaborator

@zulipbot claim

Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 1, 2021
This commit fixes a bug which causes Zulip to crash when searching
Streams or Topics. This happens because when a search result is
empty, log is empty and because an empty ListWalker returns None,
it leads to a Type error when setting focus.

This is fixed by adding a boolean `in_search_mode` which does not
call `get_focus` untill the search is over. Slight reordering was
done in `__init__` to fit the boolean in an ideal place w.r.t.
readability.

Incidentally, the same commit also corrects what the code for
`focus_index_before_search` was intended to do.

Fixes zulip#923 and Fixes zulip#975
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 1, 2021
This commit fixes a bug which causes Zulip to crash when searching
Streams or Topics. This happens because when a search result is
empty, log is empty and because an empty ListWalker returns None,
it leads to a Type error when setting focus.

This is fixed by adding a boolean `in_search_mode` which does not
call `get_focus` untill the search is over. Slight reordering was
done in `__init__` to fit the boolean in an ideal place w.r.t.
readability.

Incidentally, the same commit also corrects what the code for
`focus_index_before_search` was intended to do.

Fixes zulip#923 and Fixes zulip#975
@neiljp
Copy link
Collaborator

neiljp commented Apr 1, 2021

@Rohitth007 Thanks for reproducing this 👍

Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 2, 2021
This commit fixes a bug which causes Zulip to crash when searching
Streams or Topics. This happens because when a search result is
empty, log is empty and because an empty ListWalker returns None,
it leads to a Type error when setting focus.

This is fixed by making sure the user does not enter the empty list
when the search result is empty. The boolean `empty_search` is used
to achieve this.

This commit also improves the UI/UX when search result is empty by
displaying caption `Not Found` when a user tries to enter an empty
list. The user can then correct their mistake in spelling if this
is the case without retyping the whole word or `esc` out of the
search.

Captions are removed each time `q` is pressed using
`set_caption('')` and appropriate colors are set for `Not Found`.

Fixes zulip#923
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 2, 2021
This commit fixes a bug which causes Zulip to crash when searching
Streams or Topics. This happens because when a search result is
empty, log is empty and because an empty ListWalker returns None,
it leads to a Type error when setting focus.

This is fixed by making sure the user does not enter the empty list
when the search result is empty. The boolean `empty_search` is used
to achieve this.

This commit also improves the UI/UX when search result is empty by
displaying caption `Not Found` when a user tries to enter an empty
list. The user can then correct their mistake in spelling if this
is the case without retyping the whole word or `esc` out of the
search.

Captions are removed each time `q` is pressed using
`set_caption('')` and appropriate colors are set for `Not Found`.

Fixes zulip#923

Tests amended.
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 6, 2021
This commit fixes a bug which causes Zulip to crash when searching
Streams or Topics. This happens because when a search result is
empty, log is empty and because an empty ListWalker returns None,
it leads to a Type error when setting focus.

This is fixed by making sure the user does not enter the empty list
when the search result is empty. The boolean `empty_search` is used
to achieve this.

This commit also improves the UI/UX when search result is empty by
displaying Text `Not Found` in place of the empty list.
The user can then correct their search and `Not Found` disappears.

A `search_error` attribute is introducted in the pallete which
colors `Not Found` in `light red`. The `cross_mark` symbol -
`STREAM_MARKER_INVALID` is also added for effect.

`set_content('')` is used so that when a user presses `q` for a
second search the caption becomes empty.

Fixes zulip#923

Tests amended.
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 7, 2021
This commit fixes a bug which causes Zulip to crash when searching
Streams or Topics. This happens because when a search result is
empty, log is empty and because an empty ListWalker returns None,
it leads to a Type error when setting focus.

This is fixed by making sure the user does not enter the empty list
when the search result is empty. The boolean `empty_search` is used
to achieve this.

This commit also improves the UI/UX when search result is empty by
displaying Text `Not Found` in place of the empty list.
The user can then correct their search and `Not Found` disappears.

* A `search_error` attribute is introducted in the pallete which
colors `Not Found` in `light red`. The `cross_mark` symbol -
`STREAM_MARKER_INVALID` is also added for effect.

* `set_content('')` is used so that when a user presses `q` for a
second search the caption becomes empty.

The `set_count` function is modified to use `btn_list` instead
of `log` so that it doesn't assume the `Not Found` Text element is
a button.

Fixes zulip#923

Tests amended.
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 12, 2021
This commit fixes a bug which causes Zulip to crash when searching
Streams or Topics. This happens because when a search result is
empty, log is empty and because an empty ListWalker returns None,
it leads to a Type error when setting focus.

This is fixed by making sure the user does not enter the empty list
when the search result is empty. The boolean `empty_search` is used
to achieve this.

This commit also improves the UI/UX when search result is empty by
displaying Text `Not Found` in place of the empty list.
The user can then correct their search and `Not Found` disappears.

A `search_error` attribute is introducted in the pallete which
colors `Not Found` in `light red`. The `cross_mark` symbol -
`STREAM_MARKER_INVALID` is also added for effect.

Fixes zulip#923

Tests amended.
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 12, 2021
The tests for `PanelSearchBox` introduced when fixing bug zulip#923 is
split into two tests for better readability.

Both cases `test_keypress_ENTER_focus_set` and
`test_keypress_ENTER_focus_not_set` have to be checked carefully to
avoid the bug.
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 12, 2021
This is caused when ZT recieves an update event while searching and
`set_count` assumes that the Text widget is a Button since the log
is updated while searching.

This also means that message updates are only sent to a subset of
streams, topics or users.

Since, we only need to update the count variable in the button
object this is solved by using `btn_list` instead of `log`.
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 18, 2021
This commit fixes a bug which causes Zulip to crash when searching
Streams or Topics. This happens because when a search result is
empty, log is empty and because an empty ListWalker returns None,
it leads to a Type error when setting focus.

This is fixed by making sure the user does not enter the empty list
when the search result is empty. The boolean `empty_search` is used
to achieve this.

This commit also improves the UI/UX when search result is empty by
displaying Text `Not Found` in place of the empty list.
The user can then correct their search and `Not Found` disappears.

A `search_error` attribute is introducted in the pallete which
colors `Not Found` in `light red`. The `cross_mark` symbol -
`STREAM_MARKER_INVALID` is also added for effect.

Fixes zulip#923

Tests amended.
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 18, 2021
The tests for `PanelSearchBox` introduced when fixing bug zulip#923 is
split into two tests for better readability.

Both cases `test_keypress_ENTER_focus_set` and
`test_keypress_ENTER_focus_not_set` have to be checked carefully to
avoid the bug.
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 21, 2021
This commit fixes a bug which causes Zulip to crash when searching
Streams or Topics. This happens because when a search result is
empty, log is empty and because an empty ListWalker returns None,
it leads to a Type error when setting focus.

This is fixed by making sure the user does not enter the empty list
when the search result is empty. The boolean `empty_search` is used
to achieve this.

This commit also improves the UI/UX when search result is empty by
displaying Text `Not Found` in place of the empty list.
The user can then correct their search and `Not Found` disappears.

A `search_error` attribute is introducted in the pallete which
colors `Not Found` in `light red`. The `cross_mark` symbol -
`STREAM_MARKER_INVALID` is also added for effect.

Fixes zulip#923

Tests amended.
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 21, 2021
The tests for `PanelSearchBox` introduced when fixing bug zulip#923 is
split into two tests for better readability.

Both cases `test_keypress_ENTER_focus_set` and
`test_keypress_ENTER_focus_not_set` have to be checked carefully to
avoid the bug.
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 21, 2021
This commit fixes a bug which causes Zulip to crash when searching
Streams or Topics. This happens because when a search result is
empty, log is empty and because an empty ListWalker returns None,
it leads to a Type error when setting focus.

This is fixed by making sure the user does not enter the empty list
when the search result is empty. The boolean `empty_search` is used
to achieve this.

This commit also improves the UI/UX when search result is empty by
displaying Text `Not Found` in place of the empty list.
The user can then correct their search and `Not Found` disappears.

A `search_error` attribute is introducted in the pallete which
colors `Not Found` in `light red`. The `cross_mark` symbol -
`STREAM_MARKER_INVALID` is also added for effect.

Fixes zulip#923

Tests amended.
Rohitth007 added a commit to Rohitth007/zulip-terminal that referenced this issue Apr 21, 2021
The tests for `PanelSearchBox` introduced when fixing bug zulip#923 is
split into two tests for better readability.

Both cases `test_keypress_ENTER_focus_set` and
`test_keypress_ENTER_focus_not_set` have to be checked carefully to
avoid the bug.
@zulipbot
Copy link
Member

zulipbot commented May 2, 2021

Hello @Rohitth007, you claimed this issue to work on it, but this issue and any referenced pull requests haven't been updated for 10 days. Are you still working on this issue?

If so, please update this issue by leaving a comment on this issue to let me know that you're still working on it. Otherwise, I'll automatically remove you from this issue in 4 days.

If you've decided to work on something else, simply comment @zulipbot abandon so that someone else can claim it and continue from where you left off.

Thank you for your valuable contributions to Zulip!

@Rohitth007
Copy link
Collaborator

@zulipbot Yes, it's still in progress.

@neiljp neiljp closed this as completed in 92aaa8a May 17, 2021
@neiljp neiljp added this to the Next Release milestone May 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants