Skip to content

Commit

Permalink
footlinks: Add threshold configuration for footlinks.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
Abhirup-99 committed Dec 5, 2020
1 parent f9a46fa commit a38e3fa
Show file tree
Hide file tree
Showing 10 changed files with 91 additions and 52 deletions.
5 changes: 2 additions & 3 deletions tests/cli/test_run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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".
Expand All @@ -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")
Expand All @@ -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".
Expand Down
7 changes: 4 additions & 3 deletions tests/core/test_core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
30 changes: 27 additions & 3 deletions tests/model/test_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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': '<p>new content</p>',
'subject': 'new subject',
Expand All @@ -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': '<p>new content</p>',
'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': '<p>new content</p>',
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 4 additions & 4 deletions tests/ui/test_ui_tools.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand Down
4 changes: 2 additions & 2 deletions tests/ui_tools/test_popups.py
Original file line number Diff line number Diff line change
Expand Up @@ -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')})
Expand Down Expand Up @@ -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
Expand Down
12 changes: 8 additions & 4 deletions zulipterminal/cli/run.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand Down Expand Up @@ -320,16 +320,15 @@ 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
# Specify setting in order True, False
valid_settings = {
'autohide': ['autohide', 'no_autohide'],
'notify': ['enabled', 'disabled'],
'footlinks': ['enabled', 'disabled'],
'color-depth': ['1', '16', '256']
}
boolean_settings = dict() # type: Dict[str, bool]
Expand All @@ -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,
Expand Down
10 changes: 5 additions & 5 deletions zulipterminal/core.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]

Expand Down Expand Up @@ -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(
Expand Down
57 changes: 32 additions & 25 deletions zulipterminal/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
"""
Expand Down
6 changes: 5 additions & 1 deletion zulipterminal/ui_tools/boxes.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
' ',
Expand Down
4 changes: 2 additions & 2 deletions zulipterminal/ui_tools/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 []
Expand All @@ -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))])
]

Expand Down

0 comments on commit a38e3fa

Please sign in to comment.