bugfix: view: Fix Zulip crash during search.

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.

tests/ui/test_ui_tools.py

@@ -498,6 +498,7 @@ def test_init(self, mocker, stream_view):
         ('FOO', ['foo', '', 'FOO', 'FOOBAR'], ['foo', ]),
         # With 'bar' pinned.
         ('bar', ['bar'], ['bar', ]),
+        ('baar', 'search error', []),
     ])
     def test_update_streams(self, mocker, stream_view, new_text, expected_log,
                             to_pin):
@@ -510,14 +511,17 @@ def test_update_streams(self, mocker, stream_view, new_text, expected_log,
         stream_names.sort(key=lambda stream_name: stream_name in [
             stream for stream in to_pin], reverse=True)
         self.view.controller.is_in_editor_mode = lambda: True
-        search_box = "SEARCH_BOX"
+        search_box = stream_view.stream_search_box
         stream_view.streams_btn_list = [
             mocker.Mock(stream_name=stream_name)
             for stream_name in stream_names
         ]
         stream_view.update_streams(search_box, new_text)
-        assert [stream.stream_name for stream in stream_view.log
-                ] == expected_log
+        if expected_log != 'search error':
+            assert [stream.stream_name for stream in stream_view.log
+                    ] == expected_log
+        else:
+            assert hasattr(stream_view.log[0].original_widget, 'text')
         self.view.controller.update_screen.assert_called_once_with()
 
     def test_mouse_event(self, mocker, stream_view, widget_size):
@@ -624,6 +628,7 @@ def test_init(self, mocker, topic_view):
         ('FOO', ['FOO', 'FOOBAR', 'foo']),
         ('(no', ['(no topic)']),
         ('topic', ['(no topic)']),
+        ('c', 'search error'),
     ])
     def test_update_topics(self, mocker, topic_view, new_text, expected_log):
         topic_names = [
@@ -638,8 +643,11 @@ def test_update_topics(self, mocker, topic_view, new_text, expected_log):
             for topic_name in topic_names
         ]
         topic_view.update_topics(search_box, new_text)
-        assert [topic.topic_name for topic in topic_view.log
-                ] == expected_log
+        if expected_log != 'search error':
+            assert [topic.topic_name for topic in topic_view.log
+                    ] == expected_log
+        else:
+            assert hasattr(topic_view.log[0].original_widget, 'text')
         self.view.controller.update_screen.assert_called_once_with()
 
     @pytest.mark.parametrize(['topic_name', 'topic_initial_log',
@@ -1097,8 +1105,8 @@ def test_update_user_list(self, right_col_view, mocker,
         set_body = mocker.patch(VIEWS + ".urwid.Frame.set_body")
 
         right_col_view.update_user_list("SEARCH_BOX", search_string)
-
-        right_col_view.users_view.assert_called_with(assert_list)
+        if assert_list:
+            right_col_view.users_view.assert_called_with(assert_list)
         set_body.assert_called_once_with(right_col_view.body)
 
     def test_update_user_presence(self, right_col_view, mocker,

tests/ui_tools/test_boxes.py

@@ -814,6 +814,8 @@ def test_keypress_ENTER(self, panel_search_box, widget_size,
             lambda: True
         )
         panel_search_box.panel_view.log = log
+        empty_search = False if log else True
+        panel_search_box.panel_view.empty_search = empty_search
         panel_search_box.set_caption("")
         panel_search_box.edit_text = "key words"
 
@@ -822,19 +824,24 @@ def test_keypress_ENTER(self, panel_search_box, widget_size,
         # Update this display
         # FIXME We can't test for the styled version?
         # We'd compare to [('filter_results', 'Search Results'), ' ']
-        assert panel_search_box.caption == self.search_caption
-        assert panel_search_box.edit_text == "key words"
 
-        # Leave editor mode
-        (panel_search_box.panel_view.view.controller.exit_editor_mode
-         .assert_called_once_with())
+        assert panel_search_box.edit_text == "key words"
 
-        # Switch focus to body; if have results, move to them
-        panel_search_box.panel_view.set_focus.assert_called_once_with("body")
         if expect_body_focus_set:
+            assert panel_search_box.caption == self.search_caption
+            # Leave editor mode
+            (panel_search_box.panel_view.view.controller.exit_editor_mode
+             .assert_called_once_with())
+            # Switch focus to body; if have results, move to them
+            panel_search_box.panel_view.set_focus.assert_called_once_with(
+                "body")
             (panel_search_box.panel_view.body.set_focus
              .assert_called_once_with(0))
         else:
+            assert panel_search_box.caption == ''
+            (panel_search_box.panel_view.view.controller.exit_editor_mode
+             .assert_not_called())
+            panel_search_box.panel_view.set_focus.assert_not_called()
             (panel_search_box.panel_view.body.set_focus
              .assert_not_called())
 

zulipterminal/config/themes.py

@@ -49,6 +49,7 @@
     'area:msg': 'standout',
     'area:stream': 'standout',
     'area:error': 'standout',
+    'search_error': 'standout'
 }
 
 
@@ -189,6 +190,8 @@
          None,             DEF['white'],              DEF['dark_cyan']),
         ('area:error',     'white',                   'dark red',
          None,             DEF['white'],               DEF['dark_red']),
+        ('search_error',   'light red',               'black',
+         None,             DEF['light_red'],           DEF['black']),
     ],
     'gruvbox_dark': [
         # default colorscheme on 16 colors, gruvbox colorscheme
@@ -281,6 +284,8 @@
          None,             BLACK,             LIGHTBLUE),
         ('area:error',     'white',           'dark red',
          None,             WHITE,             DARKRED),
+        ('search_error',   'light red',       'black',
+         None,             LIGHTRED,          BLACK),
     ],
     'zt_light': [
         (None,             'black',           'white'),
@@ -327,6 +332,7 @@
         ('area:stream',    'black',           'light blue'),
         ('area:msg',       'black',           'yellow'),
         ('area:error',     'black',           'light red'),
+        ('search_error',   'light red',       'white'),
     ],
     'zt_blue': [
         (None,             'black',           'light blue'),
@@ -373,6 +379,7 @@
         ('area:stream',    'white',           'dark cyan'),
         ('area:msg',       'white',           'brown'),
         ('area:error',     'white',           'dark red'),
+        ('search_error',   'light red',       'light blue'),
     ]
 }
 

zulipterminal/ui_tools/boxes.py

@@ -1609,6 +1609,10 @@ def __init__(self, panel_view: Any, search_command: str,
         self.search_text = (
             f"Search [{', '.join(keys_for_command(search_command))}]: "
         )
+        self.search_error = urwid.AttrMap(
+            urwid.Text([' ', STREAM_MARKER_INVALID, ' Not Found']),
+            'search_error'
+        )
         urwid.connect_signal(self, 'change', update_function)
         super().__init__(caption='', edit_text=self.search_text)
 
@@ -1637,10 +1641,10 @@ def keypress(self, size: urwid_Size, key: str) -> Optional[str]:
             self.reset_search_text()
             self.panel_view.set_focus("body")
             self.panel_view.keypress(size, 'esc')
-        elif is_command_key('ENTER', key):
+        elif is_command_key('ENTER', key) and not self.panel_view.empty_search:
             self.panel_view.view.controller.exit_editor_mode()
             self.set_caption([('filter_results', 'Search Results'), ' '])
             self.panel_view.set_focus("body")
-            if hasattr(self.panel_view, 'log') and len(self.panel_view.log):
+            if hasattr(self.panel_view, 'log'):
                 self.panel_view.body.set_focus(0)
         return super().keypress(size, key)

zulipterminal/ui_tools/views.py

@@ -306,6 +306,7 @@ def __init__(self, streams_btn_list: List[Any], view: Any) -> None:
             bline='─', brcorner='─'
         ))
         self.search_lock = threading.Lock()
+        self.empty_search = False
 
     @asynch
     def update_streams(self, search_box: Any, new_text: str) -> None:
@@ -321,17 +322,21 @@ def update_streams(self, search_box: Any, new_text: str) -> None:
             streams_display = match_stream(stream_buttons, new_text,
                                            self.view.pinned_streams)[0]
 
+            streams_display_num = len(streams_display)
+            if streams_display_num == 0:
+                self.empty_search = True
+
             # Add a divider to separate pinned streams from the rest.
             pinned_stream_names = [
                 stream['name']
                 for stream in self.view.pinned_streams
             ]
-            first_unpinned_index = len(streams_display)
+            first_unpinned_index = streams_display_num
             for index, stream in enumerate(streams_display):
                 if stream.stream_name not in pinned_stream_names:
                     first_unpinned_index = index
                     break
-            if first_unpinned_index not in [0, len(streams_display)]:
+            if first_unpinned_index not in [0, streams_display_num]:
                 # Do not add a divider when it is already present. This can
                 # happen when new_text=''.
                 if not isinstance(streams_display[first_unpinned_index],
@@ -340,7 +345,10 @@ def update_streams(self, search_box: Any, new_text: str) -> None:
                                            StreamsViewDivider())
 
             self.log.clear()
-            self.log.extend(streams_display)
+            if not self.empty_search:
+                self.log.extend(streams_display)
+            else:
+                self.log.extend([self.stream_search_box.search_error])
             self.view.controller.update_screen()
 
     def mouse_event(self, size: urwid_Size, event: str, button: int, col: int,
@@ -392,6 +400,7 @@ def __init__(self, topics_btn_list: List[Any], view: Any,
             bline='─', brcorner='─'
         ))
         self.search_lock = threading.Lock()
+        self.empty_search = False
 
     @asynch
     def update_topics(self, search_box: Any, new_text: str) -> None:
@@ -401,13 +410,19 @@ def update_topics(self, search_box: Any, new_text: str) -> None:
         # displaying wrong topics list.
         with self.search_lock:
             new_text = new_text.lower()
-            topics_to_display = [
+            topics_display = [
                 topic
                 for topic in self.topics_btn_list.copy()
                 if new_text in topic.topic_name.lower()
             ]
+            if len(topics_display) == 0:
+                self.empty_search = True
+
             self.log.clear()
-            self.log.extend(topics_to_display)
+            if not self.empty_search:
+                self.log.extend(topics_display)
+            else:
+                self.log.extend([self.topic_search_box.search_error])
             self.view.controller.update_screen()
 
     def update_topics_list(self, stream_id: int, topic_name: str,
@@ -626,6 +641,7 @@ def __init__(self, width: int, view: Any) -> None:
             )
         self.allow_update_user_list = True
         self.search_lock = threading.Lock()
+        self.empty_search = False
         super().__init__(self.users_view(), header=search_box)
 
     @asynch
@@ -658,14 +674,25 @@ def update_user_list(self, search_box: Any=None,
         with self.search_lock:
             if user_list:
                 self.view.users = user_list
+
             users = self.view.users.copy()
             if new_text:
                 users_display = [
                     user for user in users if match_user(user, new_text)
                 ]
             else:
                 users_display = users
-            self.body = self.users_view(users_display)
+
+            if len(users_display) == 0:
+                self.empty_search = True
+
+            if not self.empty_search:
+                self.body = self.users_view(users_display)
+            else:
+                self.body = UsersView(
+                    self.view.controller,
+                    [self.user_search.search_error]
+                )
             self.set_body(self.body)
             self.view.controller.update_screen()