From a38e3fa1ee8c042a549fc060122c2427383e82fe Mon Sep 17 00:00:00 2001 From: Abhirup-99 Date: Sat, 5 Dec 2020 21:47:20 +0530 Subject: [PATCH] footlinks: Add threshold configuration for footlinks. This enables user to configure footlinks threshold settings using footlinks_threshold in zuliprc, retiring the previous footlinks. Tests have been updated to reflect the change. Fixes #773. --- tests/cli/test_run.py | 5 ++- tests/core/test_core.py | 7 ++-- tests/model/test_model.py | 30 +++++++++++++++-- tests/ui/test_ui_tools.py | 8 ++--- tests/ui_tools/test_popups.py | 4 +-- zulipterminal/cli/run.py | 12 ++++--- zulipterminal/core.py | 10 +++--- zulipterminal/model.py | 57 ++++++++++++++++++--------------- zulipterminal/ui_tools/boxes.py | 6 +++- zulipterminal/ui_tools/views.py | 4 +-- 10 files changed, 91 insertions(+), 52 deletions(-) diff --git a/tests/cli/test_run.py b/tests/cli/test_run.py index 30a03694a88..0f3fbf7a9db 100644 --- a/tests/cli/test_run.py +++ b/tests/cli/test_run.py @@ -101,7 +101,7 @@ def test_valid_zuliprc_but_no_connection(capsys, mocker, minimal_zuliprc, "Loading with:", " theme 'zt_dark' specified with no config.", " autohide setting 'no_autohide' specified with no config.", - " footlinks setting 'enabled' specified with no config.", + " footlinks_threshold setting '3' specified with no config.", " color depth setting '256' specified with no config.", "\x1b[91m", ("Error connecting to Zulip server: {}.\x1b[0m". @@ -128,7 +128,6 @@ def test_warning_regarding_incomplete_theme(capsys, mocker, monkeypatch, with pytest.raises(SystemExit) as e: main(["-c", minimal_zuliprc, "-t", bad_theme]) assert str(e.value) == '1' - captured = capsys.readouterr() lines = captured.out.strip().split("\n") @@ -140,7 +139,7 @@ def test_warning_regarding_incomplete_theme(capsys, mocker, monkeypatch, " (you could try: {}, {})" "\x1b[0m".format('a', 'b'), " autohide setting 'no_autohide' specified with no config.", - " footlinks setting 'enabled' specified with no config.", + " footlinks_threshold setting '3' specified with no config.", " color depth setting '256' specified with no config.", "\x1b[91m", ("Error connecting to Zulip server: {}.\x1b[0m". diff --git a/tests/core/test_core.py b/tests/core/test_core.py index 6798799727e..1cb47cb6da1 100644 --- a/tests/core/test_core.py +++ b/tests/core/test_core.py @@ -35,10 +35,11 @@ def controller(self, mocker) -> None: self.in_explore_mode = False self.autohide = True # FIXME Add tests for no-autohide self.notify_enabled = False - self.footlinks_enabled = True - result = Controller(self.config_file, self.theme_name, self.theme, 256, + self.footlinks_threshold = '3' + result = Controller(self.config_file, self.footlinks_threshold, + self.theme_name, self.theme, 256, self.in_explore_mode, self.autohide, - self.notify_enabled, self.footlinks_enabled) + self.notify_enabled) result.view.message_view = mocker.Mock() # set in View.__init__ return result diff --git a/tests/model/test_model.py b/tests/model/test_model.py index 3ff9442765b..00ac1492ec4 100644 --- a/tests/model/test_model.py +++ b/tests/model/test_model.py @@ -1079,7 +1079,7 @@ def test_notify_users_enabled(self, mocker, model, message_fixture, 'edited_messages': {1}, 'topics': {10: ['old subject']}, }, False), - ({ # message_id not present in index. + ({ # message_id not present in index, topic view closed. 'message_id': 3, 'rendered_content': '

new content

', 'subject': 'new subject', @@ -1100,8 +1100,31 @@ def test_notify_users_enabled(self, mocker, model, message_fixture, 'subject': 'old subject' }}, 'edited_messages': set(), - 'topics': {10: ['old subject']}, + 'topics': {10: []}, # This resets the cache }, False), + ({ # message_id not present in index, topic view is enabled. + 'message_id': 3, + 'rendered_content': '

new content

', + 'subject': 'new subject', + 'stream_id': 10, + 'message_ids': [3], + }, 0, { + 'messages': { + 1: { + 'id': 1, + 'stream_id': 10, + 'content': 'old content', + 'subject': 'old subject' + }, + 2: { + 'id': 2, + 'stream_id': 10, + 'content': 'old content', + 'subject': 'old subject' + }}, + 'edited_messages': set(), + 'topics': {10: ['new subject', 'old subject']}, + }, True), ({ # Message content is updated and topic view is enabled. 'message_id': 1, 'rendered_content': '

new content

', @@ -1131,7 +1154,8 @@ def test_notify_users_enabled(self, mocker, model, message_fixture, "Message content is updated", "Both message content and subject is updated", "Some new type of update which we don't handle yet", - "message_id not present in index", + "message_id not present in index, topic view closed", + "message_id not present in index, topic view is enabled", "Message content is updated and topic view is enabled", ]) def test__handle_update_message_event(self, mocker, model, diff --git a/tests/ui/test_ui_tools.py b/tests/ui/test_ui_tools.py index e4a2a1b1da5..e4426e126d0 100644 --- a/tests/ui/test_ui_tools.py +++ b/tests/ui/test_ui_tools.py @@ -2067,7 +2067,7 @@ def test_reactions_view(self, message_fixture, to_vary_in_each_message): ) def test_footlinks_view(self, message_fixture, message_links, expected_text, expected_attrib): - self.model.controller.footlinks_enabled = True + self.model.controller.footlinks_threshold = 3 msg_box = MessageBox(message_fixture, self.model, None) footlinks = msg_box.footlinks_view(message_links) @@ -2079,16 +2079,16 @@ def test_footlinks_view(self, message_fixture, message_links, assert footlinks is None assert not hasattr(footlinks, 'original_widget') - @pytest.mark.parametrize('footlinks_enabled, expected_instance', [ + @pytest.mark.parametrize('footlinks_threshold, expected_instance', [ (False, type(None)), (True, Padding), ]) - def test_footlinks_enabled(self, message_fixture, footlinks_enabled, + def test_footlinks_enabled(self, message_fixture, footlinks_threshold, expected_instance): message_links = OrderedDict([ ('/~https://github.com/zulip/zulip-terminal', ('ZT', 1, True)), ]) - self.model.controller.footlinks_enabled = footlinks_enabled + self.model.controller.footlinks_threshold = footlinks_threshold msg_box = MessageBox(message_fixture, self.model, None) footlinks = msg_box.footlinks_view(message_links) diff --git a/tests/ui_tools/test_popups.py b/tests/ui_tools/test_popups.py index 864801d1611..8b5441b132e 100644 --- a/tests/ui_tools/test_popups.py +++ b/tests/ui_tools/test_popups.py @@ -134,7 +134,7 @@ def mock_external_classes(self, mocker): theme_name='zt_dark', color_depth=256, autohide_enabled=False, - footlink_enabled=True) + footlinks_threshold='3') @pytest.mark.parametrize('key', {*keys_for_command('GO_BACK'), *keys_for_command('ABOUT')}) @@ -170,7 +170,7 @@ def test_feature_level_content(self, mocker, zulip_version): theme_name='zt_dark', color_depth=256, autohide_enabled=False, - footlink_enabled=True) + footlinks_threshold='3') assert len(about_view.feature_level_content) == ( 1 if server_feature_level else 0 diff --git a/zulipterminal/cli/run.py b/zulipterminal/cli/run.py index de6f6a1226b..08d94938041 100755 --- a/zulipterminal/cli/run.py +++ b/zulipterminal/cli/run.py @@ -213,7 +213,7 @@ def parse_zuliprc(zuliprc_str: str) -> Dict[str, Any]: 'theme': ('zt_dark', NO_CONFIG), 'autohide': ('no_autohide', NO_CONFIG), 'notify': ('disabled', NO_CONFIG), - 'footlinks': ('enabled', NO_CONFIG), + 'footlinks_threshold': ('3', NO_CONFIG), 'color-depth': ('256', NO_CONFIG) } @@ -320,8 +320,8 @@ def main(options: Optional[List[str]]=None) -> None: format(", ".join(complete)))) print(" autohide setting '{}' specified {}." .format(*zterm['autohide'])) - print(" footlinks setting '{}' specified {}." - .format(*zterm['footlinks'])) + print(" footlinks_threshold setting '{}' specified {}." + .format(*zterm['footlinks_threshold'])) print(" color depth setting '{}' specified {}." .format(*zterm['color-depth'])) # For binary settings @@ -329,7 +329,6 @@ def main(options: Optional[List[str]]=None) -> None: valid_settings = { 'autohide': ['autohide', 'no_autohide'], 'notify': ['enabled', 'disabled'], - 'footlinks': ['enabled', 'disabled'], 'color-depth': ['1', '16', '256'] } boolean_settings = dict() # type: Dict[str, bool] @@ -351,7 +350,12 @@ def main(options: Optional[List[str]]=None) -> None: else: theme_data = THEMES[theme_to_use[0]] + ZULIPRC_CONFIG = 'in zuliprc file' + footlinks_threshold = 3 + if zterm['footlinks_threshold'][1] == ZULIPRC_CONFIG: + footlinks_threshold = int(zterm['footlinks_threshold'][0]) Controller(zuliprc_path, + footlinks_threshold, theme_to_use[0], theme_data, color_depth, diff --git a/zulipterminal/core.py b/zulipterminal/core.py index d9f74f7e5f8..bcdae92b5fa 100644 --- a/zulipterminal/core.py +++ b/zulipterminal/core.py @@ -33,16 +33,16 @@ class Controller: the application. """ - def __init__(self, config_file: str, theme_name: str, theme: ThemeSpec, - color_depth: int, in_explore_mode: bool, - autohide: bool, notify: bool, footlinks: bool) -> None: + def __init__(self, config_file: str, footlinks_threshold: int, + theme_name: str, theme: ThemeSpec, color_depth: int, + in_explore_mode: bool, autohide: bool, notify: bool) -> None: self.theme_name = theme_name self.theme = theme self.color_depth = color_depth self.in_explore_mode = in_explore_mode self.autohide = autohide self.notify_enabled = notify - self.footlinks_enabled = footlinks + self.footlinks_threshold = footlinks_threshold self._editor = None # type: Optional[Any] @@ -210,7 +210,7 @@ def show_about(self) -> None: server_feature_level=self.model.server_feature_level, theme_name=self.theme_name, color_depth=self.color_depth, autohide_enabled=self.autohide, - footlink_enabled=self.footlinks_enabled) + footlinks_threshold=self.footlinks_threshold) ) def show_edit_history( diff --git a/zulipterminal/model.py b/zulipterminal/model.py index 404ce2297a8..a4b1327b1c7 100644 --- a/zulipterminal/model.py +++ b/zulipterminal/model.py @@ -1006,32 +1006,39 @@ def _handle_update_message_event(self, event: Event) -> None: if indexed_message: self.index['edited_messages'].add(message_id) - if indexed_message: - if 'rendered_content' in event: - indexed_message['content'] = event['rendered_content'] - self.index['messages'][message_id] = indexed_message - self._update_rendered_view(message_id) - - # 'subject' is not present in update event if - # the event didn't have a 'subject' update. - if 'subject' in event: - new_subject = event['subject'] - stream_id = event['stream_id'] - for msg_id in event['message_ids']: - self.index['messages'][msg_id]['subject'] = new_subject + # Update the rendered content, if the message is indexed + if 'rendered_content' in event and indexed_message: + indexed_message['content'] = event['rendered_content'] + self.index['messages'][message_id] = indexed_message + self._update_rendered_view(message_id) + + # NOTE: This is independent of messages being indexed + # Previous assertion: + # * 'subject' is not present in update event if + # the event didn't have a 'subject' update. + if 'subject' in event: + new_subject = event['subject'] + stream_id = event['stream_id'] + + # Update any indexed messages & re-render them + for msg_id in event['message_ids']: + indexed_msg = self.index['messages'].get(msg_id) + if indexed_msg: + indexed_msg['subject'] = new_subject self._update_rendered_view(msg_id) - if stream_id in self.index['topics']: - # If topic view is open, reload list else reset cache. - if hasattr(self.controller, 'view'): - view = self.controller.view - if (view.left_panel.is_in_topic_view_with_stream_id( - indexed_message['stream_id'])): - self._fetch_topics_in_streams([stream_id]) - view.left_panel.show_topic_view( - view.topic_w.stream_button) - self.controller.update_screen() - else: - self.index['topics'][stream_id] = [] + + # If topic view is open, reload list else reset cache. + if stream_id in self.index['topics']: + if hasattr(self.controller, 'view'): + view = self.controller.view + if (view.left_panel.is_in_topic_view_with_stream_id( + stream_id)): + self._fetch_topics_in_streams([stream_id]) + view.left_panel.show_topic_view( + view.topic_w.stream_button) + self.controller.update_screen() + else: + self.index['topics'][stream_id] = [] def _handle_reaction_event(self, event: Event) -> None: """ diff --git a/zulipterminal/ui_tools/boxes.py b/zulipterminal/ui_tools/boxes.py index cc5dc60184d..dec5ec61188 100644 --- a/zulipterminal/ui_tools/boxes.py +++ b/zulipterminal/ui_tools/boxes.py @@ -675,14 +675,18 @@ def footlinks_view( self, message_links: 'OrderedDict[str, Tuple[str, int, bool]]', ) -> Any: # Return if footlinks are disabled by the user. - if not self.model.controller.footlinks_enabled: + if self.model.controller.footlinks_threshold == 0: return None footlinks = [] + counter = 0 for link, (text, index, show_footlink) in message_links.items(): + if counter == self.model.controller.footlinks_threshold: + break if not show_footlink: continue + counter += 1 footlinks.extend([ ('msg_link_index', '{}:'.format(index)), ' ', diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index 871de4fb03f..9db516dd55a 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -993,7 +993,7 @@ def __init__(self, controller: Any, title: str, *, zt_version: str, server_version: str, server_feature_level: Optional[int], theme_name: str, color_depth: int, - autohide_enabled: bool, footlink_enabled: bool) -> None: + autohide_enabled: bool, footlinks_threshold: int) -> None: self.feature_level_content = ( [('Feature level', str(server_feature_level))] if server_feature_level else [] @@ -1005,7 +1005,7 @@ def __init__(self, controller: Any, title: str, *, zt_version: str, ('Application Configuration', [ ('Theme', theme_name), ('Autohide', 'enabled' if autohide_enabled else 'disabled'), - ('Footlink', 'enabled' if footlink_enabled else 'disabled'), + ('Footlinks threshold', str(footlinks_threshold)), ('Color depth', str(color_depth))]) ]