From de8d7a65f7b1956bd3d0acc8c3a1188c229f26df Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Thu, 11 Jul 2024 04:32:50 +0530 Subject: [PATCH 01/34] ui: Make the Help hotkey suggestion in the footer more conventional. Changed the brackets used for the hotkey. Removed the colon. --- zulipterminal/ui.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zulipterminal/ui.py b/zulipterminal/ui.py index d0785bd928..8b55469978 100644 --- a/zulipterminal/ui.py +++ b/zulipterminal/ui.py @@ -104,13 +104,13 @@ def get_random_help(self) -> List[Any]: # Get random allowed hotkey (ie. eligible for being displayed as a tip) allowed_commands = commands_for_random_tips() if not allowed_commands: - return ["Help(?): "] + return ["Help[?] "] random_command = random.choice(allowed_commands) random_command_display_keys = ", ".join( [display_key_for_urwid_key(key) for key in random_command["keys"]] ) return [ - "Help(?): ", + "Help[?] ", ("footer_contrast", f" {random_command_display_keys} "), f" {random_command['help_text']}", ] From 9fccce31d4478f905a7c08250f2b68518a7d2fbb Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Mon, 15 Jul 2024 10:25:42 +0530 Subject: [PATCH 02/34] refactor: ui/core/boxes: Separate out function to reset footer text. Added function reset_footer_text() to reduce overloading of set_footer_text(), and updated its every usage. Updated tests. --- tests/core/test_core.py | 5 +++-- tests/ui/test_ui.py | 8 ++++---- tests/ui_tools/test_boxes.py | 4 ++-- zulipterminal/core.py | 2 +- zulipterminal/ui.py | 18 +++++++++--------- zulipterminal/ui_tools/boxes.py | 2 +- 6 files changed, 20 insertions(+), 19 deletions(-) diff --git a/tests/core/test_core.py b/tests/core/test_core.py index ff6701a41d..1c9f49a926 100644 --- a/tests/core/test_core.py +++ b/tests/core/test_core.py @@ -590,6 +590,7 @@ def test_show_typing_notification( active_conversation_info: Dict[str, str], ) -> None: set_footer_text = mocker.patch(VIEW + ".set_footer_text") + reset_footer_text = mocker.patch(VIEW + ".reset_footer_text") mocker.patch(MODULE + ".time.sleep") controller.active_conversation_info = active_conversation_info @@ -608,8 +609,8 @@ def mock_typing() -> None: mocker.call([("footer_contrast", " hamlet "), " is typing..."]), ] ) - set_footer_text.assert_called_with() + reset_footer_text.assert_called_with() else: - set_footer_text.assert_called_once_with() + reset_footer_text.assert_called_once_with() assert controller.is_typing_notification_in_progress is False assert controller.active_conversation_info == {} diff --git a/tests/ui/test_ui.py b/tests/ui/test_ui.py index d017f4697a..9de74a2468 100644 --- a/tests/ui/test_ui.py +++ b/tests/ui/test_ui.py @@ -85,10 +85,10 @@ def test_set_footer_text_same_test( view._w.footer.set_text.assert_not_called() - def test_set_footer_text_default(self, view: View, mocker: MockerFixture) -> None: + def test_reset_footer_text(self, view: View, mocker: MockerFixture) -> None: mocker.patch(VIEW + ".get_random_help", return_value=["some help text"]) - view.set_footer_text() + view.reset_footer_text() view.frame.footer.set_text.assert_called_once_with(["some help text"]) view.controller.update_screen.assert_called_once_with() @@ -350,12 +350,12 @@ def test_keypress_NEW_HINT( widget_size: Callable[[Widget], urwid_Box], ) -> None: size = widget_size(view) - set_footer_text = mocker.patch(VIEW + ".set_footer_text") + reset_footer_text = mocker.patch(VIEW + ".reset_footer_text") mocker.patch(CONTROLLER + ".is_in_editor_mode", return_value=False) returned_key = view.keypress(size, key) - set_footer_text.assert_called_once_with() + reset_footer_text.assert_called_once_with() assert returned_key == key @pytest.mark.parametrize("key", keys_for_command("SEARCH_PEOPLE")) diff --git a/tests/ui_tools/test_boxes.py b/tests/ui_tools/test_boxes.py index c4e89a2b58..c24c263d2c 100644 --- a/tests/ui_tools/test_boxes.py +++ b/tests/ui_tools/test_boxes.py @@ -1583,7 +1583,7 @@ def test__keypress_typeahead_mode_autocomplete_key_footer_no_reset( write_box.keypress(size, key) assert write_box.is_in_typeahead_mode == expected_typeahead_mode - assert not self.view.set_footer_text.called + assert not self.view.reset_footer_text.called @pytest.mark.parametrize( "key, current_typeahead_mode, expected_typeahead_mode", @@ -1611,7 +1611,7 @@ def test__keypress_typeahead_mode_autocomplete_key_footer_reset( assert write_box.is_in_typeahead_mode == expected_typeahead_mode # We may prefer called-once in future, but the key part is that we do reset - assert self.view.set_footer_text.called + assert self.view.reset_footer_text.called @pytest.mark.parametrize( [ diff --git a/zulipterminal/core.py b/zulipterminal/core.py index a23b1596f1..4d70921b3d 100644 --- a/zulipterminal/core.py +++ b/zulipterminal/core.py @@ -454,7 +454,7 @@ def show_typing_notification(self) -> None: time.sleep(0.45) self.is_typing_notification_in_progress = False - self.view.set_footer_text() + self.view.reset_footer_text() def report_error( self, diff --git a/zulipterminal/ui.py b/zulipterminal/ui.py index 8b55469978..ef12371690 100644 --- a/zulipterminal/ui.py +++ b/zulipterminal/ui.py @@ -118,26 +118,26 @@ def get_random_help(self) -> List[Any]: @asynch def set_footer_text( self, - text_list: Optional[List[Any]] = None, + text: List[Any], style: str = "footer", duration: Optional[float] = None, ) -> None: # Avoid updating repeatedly (then pausing and showing default text) # This is simple, though doesn't avoid starting one thread for each call - if text_list == self._w.footer.text: + if text == self._w.footer.text: return - if text_list is None: - text = self.get_random_help() - else: - text = text_list self.frame.footer.set_text(text) self.frame.footer.set_attr_map({None: style}) self.controller.update_screen() if duration is not None: assert duration > 0 time.sleep(duration) - self.set_footer_text() + self.reset_footer_text() + + def reset_footer_text(self) -> None: + text = self.get_random_help() + self.set_footer_text(text, "footer") @asynch def set_typeahead_footer( @@ -329,7 +329,7 @@ def keypress(self, size: urwid_Box, key: str) -> Optional[str]: self.controller.show_markdown_help() return key elif is_command_key("NEW_HINT", key): - self.set_footer_text() + self.reset_footer_text() return key return super().keypress(size, key) @@ -347,7 +347,7 @@ def mouse_event( ) self.displaying_selection_hint = True elif event == "mouse release" and self.displaying_selection_hint: - self.model.controller.view.set_footer_text() + self.model.controller.view.reset_footer_text() self.displaying_selection_hint = False return super().mouse_event(size, event, button, col, row, focus) diff --git a/zulipterminal/ui_tools/boxes.py b/zulipterminal/ui_tools/boxes.py index 1a479cadad..937ed735ae 100644 --- a/zulipterminal/ui_tools/boxes.py +++ b/zulipterminal/ui_tools/boxes.py @@ -727,7 +727,7 @@ def exit_compose_box(self) -> None: def _set_default_footer_after_autocomplete(self) -> None: self.is_in_typeahead_mode = False - self.view.set_footer_text() + self.view.reset_footer_text() def keypress(self, size: urwid_Size, key: str) -> Optional[str]: if self.is_in_typeahead_mode and not ( From 13410a25db4794ed8967c4e5bedf91b195e38b0b Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Mon, 15 Jul 2024 10:40:35 +0530 Subject: [PATCH 03/34] refactor: ui/core: Separate footer events from set_footer_text(). By adding functions - set_footer_text_for_event() - set_footer_text_for_event_duration() and updating their usage. Updated tests. --- tests/core/test_core.py | 4 ++-- tests/ui/test_ui.py | 6 +++--- zulipterminal/core.py | 8 ++++---- zulipterminal/ui.py | 23 ++++++++++++----------- 4 files changed, 21 insertions(+), 20 deletions(-) diff --git a/tests/core/test_core.py b/tests/core/test_core.py index 1c9f49a926..c320def285 100644 --- a/tests/core/test_core.py +++ b/tests/core/test_core.py @@ -589,7 +589,7 @@ def test_show_typing_notification( controller: Controller, active_conversation_info: Dict[str, str], ) -> None: - set_footer_text = mocker.patch(VIEW + ".set_footer_text") + set_footer_text_for_event = mocker.patch(VIEW + ".set_footer_text_for_event") reset_footer_text = mocker.patch(VIEW + ".reset_footer_text") mocker.patch(MODULE + ".time.sleep") controller.active_conversation_info = active_conversation_info @@ -601,7 +601,7 @@ def mock_typing() -> None: Thread(controller.show_typing_notification()).start() if active_conversation_info: - set_footer_text.assert_has_calls( + set_footer_text_for_event.assert_has_calls( [ mocker.call([("footer_contrast", " hamlet "), " is typing"]), mocker.call([("footer_contrast", " hamlet "), " is typing."]), diff --git a/tests/ui/test_ui.py b/tests/ui/test_ui.py index 9de74a2468..7b9d4cbbd8 100644 --- a/tests/ui/test_ui.py +++ b/tests/ui/test_ui.py @@ -96,7 +96,7 @@ def test_reset_footer_text(self, view: View, mocker: MockerFixture) -> None: def test_set_footer_text_specific_text( self, view: View, text: str = "blah" ) -> None: - view.set_footer_text([text]) + view.set_footer_text_for_event([text]) view.frame.footer.set_text.assert_called_once_with([text]) view.controller.update_screen.assert_called_once_with() @@ -106,12 +106,12 @@ def test_set_footer_text_with_duration( view: View, mocker: MockerFixture, custom_text: str = "custom", - duration: Optional[float] = 5.3, + duration: float = 5.3, ) -> None: mocker.patch(VIEW + ".get_random_help", return_value=["some help text"]) mock_sleep = mocker.patch("time.sleep") - view.set_footer_text([custom_text], duration=duration) + view.set_footer_text_for_event_duration([custom_text], duration=duration) view.frame.footer.set_text.assert_has_calls( [mocker.call([custom_text]), mocker.call(["some help text"])] diff --git a/zulipterminal/core.py b/zulipterminal/core.py index 4d70921b3d..efe5e41c3f 100644 --- a/zulipterminal/core.py +++ b/zulipterminal/core.py @@ -445,7 +445,7 @@ def show_typing_notification(self) -> None: # Until conversation becomes "inactive" like when a `stop` event is sent while self.active_conversation_info: sender_name = self.active_conversation_info["sender_name"] - self.view.set_footer_text( + self.view.set_footer_text_for_event( [ ("footer_contrast", " " + sender_name + " "), " is typing" + next(dots), @@ -464,7 +464,7 @@ def report_error( """ Helper to show an error message in footer """ - self.view.set_footer_text(text, "task:error", duration) + self.view.set_footer_text_for_event_duration(text, duration, "task:error") def report_success( self, @@ -474,7 +474,7 @@ def report_success( """ Helper to show a success message in footer """ - self.view.set_footer_text(text, "task:success", duration) + self.view.set_footer_text_for_event_duration(text, duration, "task:success") def report_warning( self, @@ -484,7 +484,7 @@ def report_warning( """ Helper to show a warning message in footer """ - self.view.set_footer_text(text, "task:warning", duration) + self.view.set_footer_text_for_event_duration(text, duration, "task:warning") def show_media_confirmation_popup( self, func: Any, tool: str, media_path: str diff --git a/zulipterminal/ui.py b/zulipterminal/ui.py index ef12371690..560d2641c5 100644 --- a/zulipterminal/ui.py +++ b/zulipterminal/ui.py @@ -116,12 +116,7 @@ def get_random_help(self) -> List[Any]: ] @asynch - def set_footer_text( - self, - text: List[Any], - style: str = "footer", - duration: Optional[float] = None, - ) -> None: + def set_footer_text(self, text: List[Any], style: str = "footer") -> None: # Avoid updating repeatedly (then pausing and showing default text) # This is simple, though doesn't avoid starting one thread for each call if text == self._w.footer.text: @@ -130,15 +125,21 @@ def set_footer_text( self.frame.footer.set_text(text) self.frame.footer.set_attr_map({None: style}) self.controller.update_screen() - if duration is not None: - assert duration > 0 - time.sleep(duration) - self.reset_footer_text() def reset_footer_text(self) -> None: text = self.get_random_help() self.set_footer_text(text, "footer") + def set_footer_text_for_event(self, text: List[Any], style: str = "footer") -> None: + self.set_footer_text(text, style) + + def set_footer_text_for_event_duration( + self, text: List[Any], duration: float, style: str = "footer" + ) -> None: + self.set_footer_text_for_event(text, style) + time.sleep(duration) + self.reset_footer_text() + @asynch def set_typeahead_footer( self, suggestions: List[str], state: Optional[int], is_truncated: bool @@ -337,7 +338,7 @@ def mouse_event( self, size: urwid_Box, event: str, button: int, col: int, row: int, focus: bool ) -> bool: if event == "mouse drag": - self.model.controller.view.set_footer_text( + self.model.controller.view.set_footer_text_for_event( [ "Try pressing ", ("footer_contrast", f" {MOUSE_SELECTION_KEY} "), From b9b5ee8c02175b09758f8196cbc8fc5a4bb5f2e8 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Mon, 15 Jul 2024 10:46:01 +0530 Subject: [PATCH 04/34] ui: Add state variable to track live footer events. To allow later commits to update the footer only when there are no live footer events running. Tests updated. --- tests/ui/test_ui.py | 3 +++ zulipterminal/ui.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/tests/ui/test_ui.py b/tests/ui/test_ui.py index 7b9d4cbbd8..c3b67ea13d 100644 --- a/tests/ui/test_ui.py +++ b/tests/ui/test_ui.py @@ -92,6 +92,7 @@ def test_reset_footer_text(self, view: View, mocker: MockerFixture) -> None: view.frame.footer.set_text.assert_called_once_with(["some help text"]) view.controller.update_screen.assert_called_once_with() + assert view._is_footer_event_running is False def test_set_footer_text_specific_text( self, view: View, text: str = "blah" @@ -100,6 +101,7 @@ def test_set_footer_text_specific_text( view.frame.footer.set_text.assert_called_once_with([text]) view.controller.update_screen.assert_called_once_with() + assert view._is_footer_event_running is True def test_set_footer_text_with_duration( self, @@ -118,6 +120,7 @@ def test_set_footer_text_with_duration( ) mock_sleep.assert_called_once_with(duration) assert view.controller.update_screen.call_count == 2 + assert view._is_footer_event_running is False @pytest.mark.parametrize( "suggestions, state, truncated, footer_text", diff --git a/zulipterminal/ui.py b/zulipterminal/ui.py index 560d2641c5..92dce7bf5f 100644 --- a/zulipterminal/ui.py +++ b/zulipterminal/ui.py @@ -49,6 +49,7 @@ def __init__(self, controller: Any) -> None: self.write_box = WriteBox(self) self.search_box = MessageSearchBox(self.controller) self.stream_topic_map: Dict[int, str] = {} + self._is_footer_event_running: bool = False self.message_view: Any = None self.displaying_selection_hint = False @@ -128,9 +129,11 @@ def set_footer_text(self, text: List[Any], style: str = "footer") -> None: def reset_footer_text(self) -> None: text = self.get_random_help() + self._is_footer_event_running = False self.set_footer_text(text, "footer") def set_footer_text_for_event(self, text: List[Any], style: str = "footer") -> None: + self._is_footer_event_running = True self.set_footer_text(text, style) def set_footer_text_for_event_duration( From af00b47e6e98f4ca23ff5a93285184530bdef0fa Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Mon, 15 Jul 2024 10:47:34 +0530 Subject: [PATCH 05/34] lint-hotkeys: Handle the doc file does not exist exception. And add a separate error message. --- tools/lint-hotkeys | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 83a7db068e..92d113ca63 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -72,6 +72,10 @@ def lint_hotkeys_file() -> None: print(f"Rerun this command after resolving errors in config/{KEYS_FILE_NAME}") else: print("No hotkeys linting errors") + if not OUTPUT_FILE.exists(): + raise SystemExit( + f"Run './tools/{SCRIPT_NAME} --fix' to generate {OUTPUT_FILE_NAME} file" + ) if not output_file_matches_string(hotkeys_file_string): print( f"Run './tools/{SCRIPT_NAME} --fix' to update {OUTPUT_FILE_NAME} file" @@ -120,13 +124,19 @@ def get_hotkeys_file_string() -> str: def output_file_matches_string(hotkeys_file_string: str) -> bool: - with open(OUTPUT_FILE) as output_file: - content_is_identical = hotkeys_file_string == output_file.read() - if content_is_identical: - print(f"{OUTPUT_FILE_NAME} file already in sync with config/{KEYS_FILE_NAME}") - return True - else: - print(f"{OUTPUT_FILE_NAME} file not in sync with config/{KEYS_FILE_NAME}") + try: + with open(OUTPUT_FILE) as output_file: + content_is_identical = hotkeys_file_string == output_file.read() + if content_is_identical: + print( + f"{OUTPUT_FILE_NAME} file already in sync with config/{KEYS_FILE_NAME}" + ) + return True + else: + print(f"{OUTPUT_FILE_NAME} file not in sync with config/{KEYS_FILE_NAME}") + return False + except FileNotFoundError: + print(f"{OUTPUT_FILE_NAME} does not exist") return False From d2e0c1757f82086c625f4c78ffc880c24a843b17 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Mon, 15 Jul 2024 06:50:23 +0530 Subject: [PATCH 06/34] refactor: lint-hotkeys: Remove the write_hotkeys_file() function. As it is used only by one function, the code was moved inside that function. --- tools/lint-hotkeys | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 92d113ca63..2b885ea9c5 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -91,7 +91,8 @@ def generate_hotkeys_file() -> None: """ hotkeys_file_string = get_hotkeys_file_string() output_file_matches_string(hotkeys_file_string) - write_hotkeys_file(hotkeys_file_string) + with open(OUTPUT_FILE, "w") as hotkeys_file: + hotkeys_file.write(hotkeys_file_string) print(f"Hot Keys list saved in {OUTPUT_FILE}") @@ -152,14 +153,6 @@ def read_help_categories() -> Dict[str, List[Tuple[str, List[str]]]]: return categories -def write_hotkeys_file(hotkeys_file_string: str) -> None: - """ - Write hotkeys_file_string variable once to OUTPUT_FILE - """ - with open(OUTPUT_FILE, "w") as hotkeys_file: - hotkeys_file.write(hotkeys_file_string) - - if __name__ == "__main__": parser = argparse.ArgumentParser( description=f"Lint hotkeys by checking extracted key description style and key " From 4e8783cb8255e2447e87cdca88ab419f8d2d523b Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Mon, 15 Jul 2024 07:05:07 +0530 Subject: [PATCH 07/34] refactor: lint-hotkeys: Make int error flag boolean & use SystemExit. Replaced sys.exit() + print() with SystemExit(). --- tools/lint-hotkeys | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 2b885ea9c5..0d4ebb3bf1 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -1,7 +1,6 @@ #!/usr/bin/env python3 import argparse import re -import sys from collections import defaultdict from pathlib import Path, PurePath from typing import Dict, List, Tuple @@ -39,8 +38,8 @@ def lint_hotkeys_file() -> None: existing OUTPUT_FILE """ hotkeys_file_string = get_hotkeys_file_string() + error_flag = False # To lint keys description - error_flag = 0 categories = read_help_categories() for action in HELP_CATEGORIES: check_duplicate_keys_list: List[str] = [] @@ -53,7 +52,7 @@ def lint_hotkeys_file() -> None: f"Description - ({help_text}) for key combination - [{various_key_combinations}]\n" "It should contain only alphabets, spaces and special characters except ." ) - error_flag = 1 + error_flag = True # Check key combination duplication check_duplicate_keys_list = [ key for key in check_duplicate_keys_list if key not in KEYS_TO_EXCLUDE @@ -67,9 +66,11 @@ def lint_hotkeys_file() -> None: print( f"Duplicate key combination for keys {duplicate_keys} for category ({HELP_CATEGORIES[action]}) detected" ) - error_flag = 1 - if error_flag == 1: - print(f"Rerun this command after resolving errors in config/{KEYS_FILE_NAME}") + error_flag = True + if error_flag: + raise SystemExit( + f"Rerun this command after resolving errors in config/{KEYS_FILE_NAME}" + ) else: print("No hotkeys linting errors") if not OUTPUT_FILE.exists(): @@ -77,11 +78,9 @@ def lint_hotkeys_file() -> None: f"Run './tools/{SCRIPT_NAME} --fix' to generate {OUTPUT_FILE_NAME} file" ) if not output_file_matches_string(hotkeys_file_string): - print( + raise SystemExit( f"Run './tools/{SCRIPT_NAME} --fix' to update {OUTPUT_FILE_NAME} file" ) - error_flag = 1 - sys.exit(error_flag) def generate_hotkeys_file() -> None: From e954870418efb8c5341ac1f7e789402a0efb8454 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Mon, 15 Jul 2024 10:53:16 +0530 Subject: [PATCH 08/34] lint-hotkeys: Rewrite function docstrings and comments. --- tools/lint-hotkeys | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 0d4ebb3bf1..4f25522680 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -34,13 +34,15 @@ def main(fix: bool) -> None: def lint_hotkeys_file() -> None: """ - Lint KEYS_FILE for key description, then compare if in sync with - existing OUTPUT_FILE + Lint KEYS_FILE for valid key descriptions (help texts) and key categories, + check for duplicate key combinations, + and compare with existing OUTPUT_FILE. """ hotkeys_file_string = get_hotkeys_file_string() error_flag = False - # To lint keys description categories = read_help_categories() + + # Lint each key combination and description for action in HELP_CATEGORIES: check_duplicate_keys_list: List[str] = [] for help_text, key_combinations_list in categories[action]: @@ -86,7 +88,7 @@ def lint_hotkeys_file() -> None: def generate_hotkeys_file() -> None: """ Generate OUTPUT_FILE based on help text description and - shortcut key combinations in KEYS_FILE + shortcut key combinations in KEYS_FILE, grouped by categories """ hotkeys_file_string = get_hotkeys_file_string() output_file_matches_string(hotkeys_file_string) @@ -124,6 +126,9 @@ def get_hotkeys_file_string() -> str: def output_file_matches_string(hotkeys_file_string: str) -> bool: + """ + Check if the OUTPUT_FILE exists and matches the generated hotkeys_file_string + """ try: with open(OUTPUT_FILE) as output_file: content_is_identical = hotkeys_file_string == output_file.read() @@ -142,7 +147,9 @@ def output_file_matches_string(hotkeys_file_string: str) -> bool: def read_help_categories() -> Dict[str, List[Tuple[str, List[str]]]]: """ - Get all help categories from KEYS_FILE + Generate a dict from KEYS_FILE + key: help category + value: a list of help texts and key combinations, for each key binding in that help category """ categories = defaultdict(list) for cmd, item in KEY_BINDINGS.items(): From 4ed23c79e5e24e26391183bbcf5cab90b579df37 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Mon, 15 Jul 2024 11:01:47 +0530 Subject: [PATCH 09/34] refactor: lint-hotkeys: Rename get_hotkeys_file_string(). to generate_hotkeys_file_string() to better express its purpose. --- tools/lint-hotkeys | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 4f25522680..5ba24fa9f5 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -38,7 +38,7 @@ def lint_hotkeys_file() -> None: check for duplicate key combinations, and compare with existing OUTPUT_FILE. """ - hotkeys_file_string = get_hotkeys_file_string() + hotkeys_file_string = generate_hotkeys_file_string() error_flag = False categories = read_help_categories() @@ -90,14 +90,14 @@ def generate_hotkeys_file() -> None: Generate OUTPUT_FILE based on help text description and shortcut key combinations in KEYS_FILE, grouped by categories """ - hotkeys_file_string = get_hotkeys_file_string() + hotkeys_file_string = generate_hotkeys_file_string() output_file_matches_string(hotkeys_file_string) with open(OUTPUT_FILE, "w") as hotkeys_file: hotkeys_file.write(hotkeys_file_string) print(f"Hot Keys list saved in {OUTPUT_FILE}") -def get_hotkeys_file_string() -> str: +def generate_hotkeys_file_string() -> str: """ Construct string in form for output to OUTPUT_FILE based on help text description and shortcut key combinations in KEYS_FILE From 9717776207d268e890a4d13d14980511c0aed84c Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Mon, 15 Jul 2024 15:56:03 +0530 Subject: [PATCH 10/34] refactor: lint-hotkeys: Rename ambiguous keyword "action" to "batch". --- tools/lint-hotkeys | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 5ba24fa9f5..13b5dc50ea 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -43,9 +43,9 @@ def lint_hotkeys_file() -> None: categories = read_help_categories() # Lint each key combination and description - for action in HELP_CATEGORIES: + for batch in HELP_CATEGORIES: check_duplicate_keys_list: List[str] = [] - for help_text, key_combinations_list in categories[action]: + for help_text, key_combinations_list in categories[batch]: check_duplicate_keys_list.extend(key_combinations_list) various_key_combinations = " / ".join(key_combinations_list) # Check description style @@ -66,7 +66,7 @@ def lint_hotkeys_file() -> None: ] if len(duplicate_keys) != 0: print( - f"Duplicate key combination for keys {duplicate_keys} for category ({HELP_CATEGORIES[action]}) detected" + f"Duplicate key combination for keys {duplicate_keys} for category ({HELP_CATEGORIES[batch]}) detected" ) error_flag = True if error_flag: @@ -107,13 +107,13 @@ def generate_hotkeys_file_string() -> str: f"\n" "\n\n# Hot Keys\n" ) - for action in HELP_CATEGORIES: + for batch in HELP_CATEGORIES: hotkeys_file_string += ( - f"## {HELP_CATEGORIES[action]}\n" + f"## {HELP_CATEGORIES[batch]}\n" "|Command|Key Combination|\n" "| :--- | :---: |\n" ) - for help_text, key_combinations_list in categories[action]: + for help_text, key_combinations_list in categories[batch]: various_key_combinations = " / ".join( [ " + ".join([f"{key}" for key in key_combination.split()]) From 11a7ead5c7dc7bde87a78e54f89947137cbd20a3 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Wed, 17 Jul 2024 17:46:37 +0530 Subject: [PATCH 11/34] refactor: lint-hotkeys: Improve variable naming of generated dict. categories -> entries_by_category --- tools/lint-hotkeys | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 13b5dc50ea..1673322eb4 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -151,12 +151,12 @@ def read_help_categories() -> Dict[str, List[Tuple[str, List[str]]]]: key: help category value: a list of help texts and key combinations, for each key binding in that help category """ - categories = defaultdict(list) + entries_by_category = defaultdict(list) for cmd, item in KEY_BINDINGS.items(): - categories[item["key_category"]].append( + entries_by_category[item["key_category"]].append( (item["help_text"], display_keys_for_command(cmd)) ) - return categories + return entries_by_category if __name__ == "__main__": From 32372510c050a63e4199288bf07887c31b86e30a Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Mon, 15 Jul 2024 11:29:43 +0530 Subject: [PATCH 12/34] lint-hotkeys: Refactor the flow by creating new ENTRY_BY_CATEGORIES. read_help_categories() was being called twice when linting. - By generate_hotkeys_file_string() - and separately by read_help_categories() again. Since both the linting and document generation need to call read_help_categories(), the function will always be called on running main(). Instead of moving its call into main(), and passing the same argument through several deep function calls, it was made a module level constant, that is initialized whenever the file is run. --- tools/lint-hotkeys | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 1673322eb4..ecda18be1c 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -25,6 +25,23 @@ HELP_TEXT_STYLE = re.compile(r"^[a-zA-Z /()',&@#:_-]*$") KEYS_TO_EXCLUDE = ["q", "e", "m", "r", "Esc"] +def read_help_categories() -> Dict[str, List[Tuple[str, List[str]]]]: + """ + Generate a dict from KEYS_FILE + key: help category + value: a list of help texts and key combinations, for each key binding in that help category + """ + entries_by_category = defaultdict(list) + for cmd, item in KEY_BINDINGS.items(): + entries_by_category[item["key_category"]].append( + (item["help_text"], display_keys_for_command(cmd)) + ) + return entries_by_category + + +ENTRIES_BY_CATEGORY: Dict[str, List[Tuple[str, List[str]]]] = read_help_categories() + + def main(fix: bool) -> None: if fix: generate_hotkeys_file() @@ -40,12 +57,11 @@ def lint_hotkeys_file() -> None: """ hotkeys_file_string = generate_hotkeys_file_string() error_flag = False - categories = read_help_categories() # Lint each key combination and description for batch in HELP_CATEGORIES: check_duplicate_keys_list: List[str] = [] - for help_text, key_combinations_list in categories[batch]: + for help_text, key_combinations_list in ENTRIES_BY_CATEGORY[batch]: check_duplicate_keys_list.extend(key_combinations_list) various_key_combinations = " / ".join(key_combinations_list) # Check description style @@ -102,7 +118,6 @@ def generate_hotkeys_file_string() -> str: Construct string in form for output to OUTPUT_FILE based on help text description and shortcut key combinations in KEYS_FILE """ - categories = read_help_categories() hotkeys_file_string = ( f"\n" "\n\n# Hot Keys\n" @@ -113,7 +128,7 @@ def generate_hotkeys_file_string() -> str: "|Command|Key Combination|\n" "| :--- | :---: |\n" ) - for help_text, key_combinations_list in categories[batch]: + for help_text, key_combinations_list in ENTRIES_BY_CATEGORY[batch]: various_key_combinations = " / ".join( [ " + ".join([f"{key}" for key in key_combination.split()]) @@ -145,20 +160,6 @@ def output_file_matches_string(hotkeys_file_string: str) -> bool: return False -def read_help_categories() -> Dict[str, List[Tuple[str, List[str]]]]: - """ - Generate a dict from KEYS_FILE - key: help category - value: a list of help texts and key combinations, for each key binding in that help category - """ - entries_by_category = defaultdict(list) - for cmd, item in KEY_BINDINGS.items(): - entries_by_category[item["key_category"]].append( - (item["help_text"], display_keys_for_command(cmd)) - ) - return entries_by_category - - if __name__ == "__main__": parser = argparse.ArgumentParser( description=f"Lint hotkeys by checking extracted key description style and key " From 188bd7f382af2e795a72de1eb1956161414a9731 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Wed, 17 Jul 2024 14:54:40 +0530 Subject: [PATCH 13/34] lint-hotkeys: Delay generation of file string when linting. Only if the linting of keys.py is successful, do we lint hotkeys.md by comparing it with keys.py. So, moved that into the else block for when there are no errors. --- tools/lint-hotkeys | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index ecda18be1c..3639d4a34e 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -55,7 +55,6 @@ def lint_hotkeys_file() -> None: check for duplicate key combinations, and compare with existing OUTPUT_FILE. """ - hotkeys_file_string = generate_hotkeys_file_string() error_flag = False # Lint each key combination and description @@ -95,6 +94,7 @@ def lint_hotkeys_file() -> None: raise SystemExit( f"Run './tools/{SCRIPT_NAME} --fix' to generate {OUTPUT_FILE_NAME} file" ) + hotkeys_file_string = generate_hotkeys_file_string() if not output_file_matches_string(hotkeys_file_string): raise SystemExit( f"Run './tools/{SCRIPT_NAME} --fix' to update {OUTPUT_FILE_NAME} file" From 0dd8aa4da2a9be559d6f3e61398acf97230bf14c Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Wed, 17 Jul 2024 15:20:15 +0530 Subject: [PATCH 14/34] refactor: lint-hotkeys: Split out the help text linting function. To prepare for contexts, when we will lint all help groups one by one, but only want to lint help texts once. --- tools/lint-hotkeys | 34 +++++++++++++++++++++++----------- 1 file changed, 23 insertions(+), 11 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 3639d4a34e..56083e2d22 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -57,20 +57,11 @@ def lint_hotkeys_file() -> None: """ error_flag = False - # Lint each key combination and description + # Lint each key combination for duplicates for batch in HELP_CATEGORIES: check_duplicate_keys_list: List[str] = [] - for help_text, key_combinations_list in ENTRIES_BY_CATEGORY[batch]: + for _, key_combinations_list in ENTRIES_BY_CATEGORY[batch]: check_duplicate_keys_list.extend(key_combinations_list) - various_key_combinations = " / ".join(key_combinations_list) - # Check description style - if not re.match(HELP_TEXT_STYLE, help_text): - print( - f"Description - ({help_text}) for key combination - [{various_key_combinations}]\n" - "It should contain only alphabets, spaces and special characters except ." - ) - error_flag = True - # Check key combination duplication check_duplicate_keys_list = [ key for key in check_duplicate_keys_list if key not in KEYS_TO_EXCLUDE ] @@ -84,6 +75,10 @@ def lint_hotkeys_file() -> None: f"Duplicate key combination for keys {duplicate_keys} for category ({HELP_CATEGORIES[batch]}) detected" ) error_flag = True + + # Lint help text + error_flag |= lint_help_text() + if error_flag: raise SystemExit( f"Rerun this command after resolving errors in config/{KEYS_FILE_NAME}" @@ -101,6 +96,23 @@ def lint_hotkeys_file() -> None: ) +def lint_help_text() -> bool: + """ + Lint each keybinding's help text / description for invalid characters + """ + error_flag = False + for keybinding in KEY_BINDINGS.values(): + help_text = keybinding["help_text"] + if not re.match(HELP_TEXT_STYLE, help_text): + key_combinations = " / ".join(keybinding["keys"]) + print( + f"Description - ({help_text}) for key combination - [{key_combinations}]\n" + "It should contain only alphabets, spaces and special characters except ." + ) + error_flag = True + return error_flag + + def generate_hotkeys_file() -> None: """ Generate OUTPUT_FILE based on help text description and From 5286488fd94996985b4f45772e88e9800e4fdb4e Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Wed, 17 Jul 2024 15:28:19 +0530 Subject: [PATCH 15/34] refactor: lint-hotkeys: Split out the function that lints categories. This is to allow linting all the groups before handling the errors. The new purposes of functions - lint_hotkeys_file() for running linting on all groups (categories, and contexts later), and handling error messages. - lint_help_categories() for linting each group (category/context). - lint_help_texts() for linting the help texts. --- tools/lint-hotkeys | 47 ++++++++++++++++++++++++++-------------------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 56083e2d22..13b4c02d18 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -57,26 +57,7 @@ def lint_hotkeys_file() -> None: """ error_flag = False - # Lint each key combination for duplicates - for batch in HELP_CATEGORIES: - check_duplicate_keys_list: List[str] = [] - for _, key_combinations_list in ENTRIES_BY_CATEGORY[batch]: - check_duplicate_keys_list.extend(key_combinations_list) - check_duplicate_keys_list = [ - key for key in check_duplicate_keys_list if key not in KEYS_TO_EXCLUDE - ] - duplicate_keys = [ - key - for key in check_duplicate_keys_list - if check_duplicate_keys_list.count(key) > 1 - ] - if len(duplicate_keys) != 0: - print( - f"Duplicate key combination for keys {duplicate_keys} for category ({HELP_CATEGORIES[batch]}) detected" - ) - error_flag = True - - # Lint help text + error_flag |= lint_help_categories() error_flag |= lint_help_text() if error_flag: @@ -113,6 +94,32 @@ def lint_help_text() -> bool: return error_flag +def lint_help_categories() -> bool: + """ + Lint help categories by checking each key combination for duplicates + within the same category + """ + error_flag = False + for batch in HELP_CATEGORIES: + check_duplicate_keys_list: List[str] = [] + for _, key_combinations_list in ENTRIES_BY_CATEGORY[batch]: + check_duplicate_keys_list.extend(key_combinations_list) + check_duplicate_keys_list = [ + key for key in check_duplicate_keys_list if key not in KEYS_TO_EXCLUDE + ] + duplicate_keys = [ + key + for key in check_duplicate_keys_list + if check_duplicate_keys_list.count(key) > 1 + ] + if len(duplicate_keys) != 0: + print( + f"Duplicate key combination for keys {duplicate_keys} for category ({HELP_CATEGORIES[batch]}) detected" + ) + error_flag = True + return error_flag + + def generate_hotkeys_file() -> None: """ Generate OUTPUT_FILE based on help text description and From 4e7a4f2d319a433e6a2932f45eb43ea9a61b2a95 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Wed, 17 Jul 2024 15:42:25 +0530 Subject: [PATCH 16/34] lint-hotkeys: Restructure the error messages of lint_help_text(). To reduce redundancy in error messages for scenarios with several linting errors, collect all errors before printing them with the hint to resolve the errors. Also add a \n at the end of error messages. --- tools/lint-hotkeys | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 13b4c02d18..1700db06aa 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -82,15 +82,20 @@ def lint_help_text() -> bool: Lint each keybinding's help text / description for invalid characters """ error_flag = False + error_message = "" for keybinding in KEY_BINDINGS.values(): help_text = keybinding["help_text"] if not re.match(HELP_TEXT_STYLE, help_text): key_combinations = " / ".join(keybinding["keys"]) - print( - f"Description - ({help_text}) for key combination - [{key_combinations}]\n" - "It should contain only alphabets, spaces and special characters except ." + error_message += ( + f" ({help_text}) for key combination - [{key_combinations}]\n" ) error_flag = True + if error_flag: + print( + "Help text descriptions should contain only alphabets, spaces and special characters except .\n" + + error_message + ) return error_flag @@ -114,7 +119,7 @@ def lint_help_categories() -> bool: ] if len(duplicate_keys) != 0: print( - f"Duplicate key combination for keys {duplicate_keys} for category ({HELP_CATEGORIES[batch]}) detected" + f"Duplicate key combination for keys {duplicate_keys} for category ({HELP_CATEGORIES[batch]}) detected\n" ) error_flag = True return error_flag From efc8f827213c74036066dda1fe3fcdec0c988bf3 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Wed, 17 Jul 2024 15:54:02 +0530 Subject: [PATCH 17/34] lint-hotkeys: Lint for typos in key_category values. --- tools/lint-hotkeys | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 1700db06aa..f247692d18 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -102,8 +102,9 @@ def lint_help_text() -> bool: def lint_help_categories() -> bool: """ Lint help categories by checking each key combination for duplicates - within the same category + within the same category and the validity of each category (typo-checking) """ + # Lint for duplicate key combinations within the same category error_flag = False for batch in HELP_CATEGORIES: check_duplicate_keys_list: List[str] = [] @@ -122,6 +123,20 @@ def lint_help_categories() -> bool: f"Duplicate key combination for keys {duplicate_keys} for category ({HELP_CATEGORIES[batch]}) detected\n" ) error_flag = True + + # Lint for typos in key categories + error_message = "" + for key, binding in KEY_BINDINGS.items(): + category_value = binding["key_category"] + if category_value not in HELP_CATEGORIES: + error_message += f" Invalid category '{category_value}' for key '{key}'.\n" + error_flag = True + if error_message: + print( + f"Choose a valid category value from:\n{', '.join(HELP_CATEGORIES.keys())}\n" + + error_message + ) + return error_flag From ac8e715fabba8fd3609822061cd27b7c4aeea0e1 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Wed, 17 Jul 2024 16:21:05 +0530 Subject: [PATCH 18/34] refactor: lint-hotkeys: Use help_group instead of HELP_CATEGORIES. Part of adding generic variables in preparation for contexts. --- tools/lint-hotkeys | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index f247692d18..7898715032 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -104,9 +104,11 @@ def lint_help_categories() -> bool: Lint help categories by checking each key combination for duplicates within the same category and the validity of each category (typo-checking) """ + help_group = HELP_CATEGORIES + # Lint for duplicate key combinations within the same category error_flag = False - for batch in HELP_CATEGORIES: + for batch in help_group: check_duplicate_keys_list: List[str] = [] for _, key_combinations_list in ENTRIES_BY_CATEGORY[batch]: check_duplicate_keys_list.extend(key_combinations_list) @@ -120,7 +122,7 @@ def lint_help_categories() -> bool: ] if len(duplicate_keys) != 0: print( - f"Duplicate key combination for keys {duplicate_keys} for category ({HELP_CATEGORIES[batch]}) detected\n" + f"Duplicate key combination for keys {duplicate_keys} for category ({help_group[batch]}) detected\n" ) error_flag = True @@ -128,12 +130,12 @@ def lint_help_categories() -> bool: error_message = "" for key, binding in KEY_BINDINGS.items(): category_value = binding["key_category"] - if category_value not in HELP_CATEGORIES: + if category_value not in help_group: error_message += f" Invalid category '{category_value}' for key '{key}'.\n" error_flag = True if error_message: print( - f"Choose a valid category value from:\n{', '.join(HELP_CATEGORIES.keys())}\n" + f"Choose a valid category value from:\n{', '.join(help_group.keys())}\n" + error_message ) @@ -157,13 +159,15 @@ def generate_hotkeys_file_string() -> str: Construct string in form for output to OUTPUT_FILE based on help text description and shortcut key combinations in KEYS_FILE """ + help_group = HELP_CATEGORIES + hotkeys_file_string = ( f"\n" "\n\n# Hot Keys\n" ) - for batch in HELP_CATEGORIES: + for batch in help_group: hotkeys_file_string += ( - f"## {HELP_CATEGORIES[batch]}\n" + f"## {help_group[batch]}\n" "|Command|Key Combination|\n" "| :--- | :---: |\n" ) From 58ec337b6f6ba671e87281aa01bd7d9623a70a14 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Wed, 17 Jul 2024 16:25:30 +0530 Subject: [PATCH 19/34] refactor: lint-hotkeys: Create generic variable entries_by_group. Part of adding generic variables in preparation for contexts. --- tools/lint-hotkeys | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 7898715032..a82401d53b 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -105,12 +105,13 @@ def lint_help_categories() -> bool: within the same category and the validity of each category (typo-checking) """ help_group = HELP_CATEGORIES + entries_by_group = ENTRIES_BY_CATEGORY # Lint for duplicate key combinations within the same category error_flag = False for batch in help_group: check_duplicate_keys_list: List[str] = [] - for _, key_combinations_list in ENTRIES_BY_CATEGORY[batch]: + for _, key_combinations_list in entries_by_group[batch]: check_duplicate_keys_list.extend(key_combinations_list) check_duplicate_keys_list = [ key for key in check_duplicate_keys_list if key not in KEYS_TO_EXCLUDE @@ -160,6 +161,7 @@ def generate_hotkeys_file_string() -> str: description and shortcut key combinations in KEYS_FILE """ help_group = HELP_CATEGORIES + entries_by_group = ENTRIES_BY_CATEGORY hotkeys_file_string = ( f"\n" @@ -171,7 +173,7 @@ def generate_hotkeys_file_string() -> str: "|Command|Key Combination|\n" "| :--- | :---: |\n" ) - for help_text, key_combinations_list in ENTRIES_BY_CATEGORY[batch]: + for help_text, key_combinations_list in entries_by_group[batch]: various_key_combinations = " / ".join( [ " + ".join([f"{key}" for key in key_combination.split()]) From 3ce52db6fc6186168d17963947a17eb7da123181 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Wed, 17 Jul 2024 16:34:07 +0530 Subject: [PATCH 20/34] refactor: lint-hotkeys: Create generic variables for key_category. Replace 'key_category' with 'key_group' and 'key_group_value'. Add type KeyGroup to enforce typing. Part of adding generic variables in preparation for contexts. --- tools/lint-hotkeys | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index a82401d53b..8601ad0ec6 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -5,6 +5,8 @@ from collections import defaultdict from pathlib import Path, PurePath from typing import Dict, List, Tuple +from typing_extensions import Literal + from zulipterminal.config.keys import ( HELP_CATEGORIES, KEY_BINDINGS, @@ -25,15 +27,19 @@ HELP_TEXT_STYLE = re.compile(r"^[a-zA-Z /()',&@#:_-]*$") KEYS_TO_EXCLUDE = ["q", "e", "m", "r", "Esc"] +KeyGroup = Literal["key_category"] + + def read_help_categories() -> Dict[str, List[Tuple[str, List[str]]]]: """ Generate a dict from KEYS_FILE key: help category value: a list of help texts and key combinations, for each key binding in that help category """ + key_group: KeyGroup = "key_category" entries_by_category = defaultdict(list) for cmd, item in KEY_BINDINGS.items(): - entries_by_category[item["key_category"]].append( + entries_by_category[item[key_group]].append( (item["help_text"], display_keys_for_command(cmd)) ) return entries_by_category @@ -106,6 +112,7 @@ def lint_help_categories() -> bool: """ help_group = HELP_CATEGORIES entries_by_group = ENTRIES_BY_CATEGORY + key_group: KeyGroup = "key_category" # Lint for duplicate key combinations within the same category error_flag = False @@ -130,7 +137,7 @@ def lint_help_categories() -> bool: # Lint for typos in key categories error_message = "" for key, binding in KEY_BINDINGS.items(): - category_value = binding["key_category"] + category_value = binding[key_group] if category_value not in help_group: error_message += f" Invalid category '{category_value}' for key '{key}'.\n" error_flag = True From 4911685172a3cb14865d5fee029982bd1791998e Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Wed, 17 Jul 2024 16:40:37 +0530 Subject: [PATCH 21/34] refactor: lint-hotkeys: Replace 'category(ies)' with 'group(s)'. This commit only updates the user-facing strings and variables, it does not update the occurrences in the comments or docstrings. Part of adding generic variables in preparation for contexts. --- tools/lint-hotkeys | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 8601ad0ec6..2cf693b402 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -30,22 +30,22 @@ KEYS_TO_EXCLUDE = ["q", "e", "m", "r", "Esc"] KeyGroup = Literal["key_category"] -def read_help_categories() -> Dict[str, List[Tuple[str, List[str]]]]: +def read_help_groups() -> Dict[str, List[Tuple[str, List[str]]]]: """ Generate a dict from KEYS_FILE key: help category value: a list of help texts and key combinations, for each key binding in that help category """ key_group: KeyGroup = "key_category" - entries_by_category = defaultdict(list) + entries_by_group = defaultdict(list) for cmd, item in KEY_BINDINGS.items(): - entries_by_category[item[key_group]].append( + entries_by_group[item[key_group]].append( (item["help_text"], display_keys_for_command(cmd)) ) - return entries_by_category + return entries_by_group -ENTRIES_BY_CATEGORY: Dict[str, List[Tuple[str, List[str]]]] = read_help_categories() +ENTRIES_BY_CATEGORY: Dict[str, List[Tuple[str, List[str]]]] = read_help_groups() def main(fix: bool) -> None: @@ -63,7 +63,7 @@ def lint_hotkeys_file() -> None: """ error_flag = False - error_flag |= lint_help_categories() + error_flag |= lint_help_groups() error_flag |= lint_help_text() if error_flag: @@ -105,11 +105,12 @@ def lint_help_text() -> bool: return error_flag -def lint_help_categories() -> bool: +def lint_help_groups() -> bool: """ - Lint help categories by checking each key combination for duplicates - within the same category and the validity of each category (typo-checking) + Lint help groups by checking each key combination for duplicates + within the same group and the validity of each category (typo-checking) """ + group = "category" help_group = HELP_CATEGORIES entries_by_group = ENTRIES_BY_CATEGORY key_group: KeyGroup = "key_category" @@ -130,20 +131,20 @@ def lint_help_categories() -> bool: ] if len(duplicate_keys) != 0: print( - f"Duplicate key combination for keys {duplicate_keys} for category ({help_group[batch]}) detected\n" + f"Duplicate key combination for keys {duplicate_keys} for {group} ({help_group[batch]}) detected\n" ) error_flag = True # Lint for typos in key categories error_message = "" for key, binding in KEY_BINDINGS.items(): - category_value = binding[key_group] - if category_value not in help_group: - error_message += f" Invalid category '{category_value}' for key '{key}'.\n" + group_value = binding[key_group] + if group_value not in help_group: + error_message += f" Invalid {group} '{group_value}' for key '{key}'.\n" error_flag = True if error_message: print( - f"Choose a valid category value from:\n{', '.join(help_group.keys())}\n" + f"Choose a valid {group} value from:\n{', '.join(help_group.keys())}\n" + error_message ) From b35e0cec13bfef34df0589b785425c9a45d5bb1a Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Wed, 17 Jul 2024 17:31:15 +0530 Subject: [PATCH 22/34] refactor: lint-hotkeys: Delete constant OUTPUT_FILE_NAME, compute it. Part of adding generic variables in preparation for contexts. --- tools/lint-hotkeys | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 2cf693b402..5670129205 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -19,7 +19,6 @@ KEYS_FILE = ( ) KEYS_FILE_NAME = KEYS_FILE.name OUTPUT_FILE = Path(__file__).resolve().parent.parent / "docs" / "hotkeys.md" -OUTPUT_FILE_NAME = OUTPUT_FILE.name SCRIPT_NAME = PurePath(__file__).name HELP_TEXT_STYLE = re.compile(r"^[a-zA-Z /()',&@#:_-]*$") @@ -74,12 +73,12 @@ def lint_hotkeys_file() -> None: print("No hotkeys linting errors") if not OUTPUT_FILE.exists(): raise SystemExit( - f"Run './tools/{SCRIPT_NAME} --fix' to generate {OUTPUT_FILE_NAME} file" + f"Run './tools/{SCRIPT_NAME} --fix' to generate {OUTPUT_FILE.name} file" ) hotkeys_file_string = generate_hotkeys_file_string() if not output_file_matches_string(hotkeys_file_string): raise SystemExit( - f"Run './tools/{SCRIPT_NAME} --fix' to update {OUTPUT_FILE_NAME} file" + f"Run './tools/{SCRIPT_NAME} --fix' to update {OUTPUT_FILE.name} file" ) @@ -197,19 +196,20 @@ def output_file_matches_string(hotkeys_file_string: str) -> bool: """ Check if the OUTPUT_FILE exists and matches the generated hotkeys_file_string """ + output_file_name = OUTPUT_FILE.name try: with open(OUTPUT_FILE) as output_file: content_is_identical = hotkeys_file_string == output_file.read() if content_is_identical: print( - f"{OUTPUT_FILE_NAME} file already in sync with config/{KEYS_FILE_NAME}" + f"{output_file_name} file already in sync with config/{KEYS_FILE_NAME}" ) return True else: - print(f"{OUTPUT_FILE_NAME} file not in sync with config/{KEYS_FILE_NAME}") + print(f"{output_file_name} file not in sync with config/{KEYS_FILE_NAME}") return False except FileNotFoundError: - print(f"{OUTPUT_FILE_NAME} does not exist") + print(f"{output_file_name} does not exist") return False @@ -221,7 +221,7 @@ if __name__ == "__main__": parser.add_argument( "--fix", action="store_true", - help=f"Generate {OUTPUT_FILE_NAME} file by extracting key description and key " + help=f"Generate {OUTPUT_FILE.name} file by extracting key description and key " f"combination from config/{KEYS_FILE_NAME} file", ) args = parser.parse_args() From 001814baeb0f4af0895275f61c10defbba25d7e1 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Wed, 17 Jul 2024 17:57:55 +0530 Subject: [PATCH 23/34] refactor: lint-hotkeys: Replace usage of OUTPUT_FILE. Use CATEGORY_OUTPUT_FILE instead of the module constant OUTPUT_FILE. Use the smaller case 'output file' instead, in docstrings. Use existing_file as the name of the file object. Part of adding generic variables in preparation for contexts. --- tools/lint-hotkeys | 31 +++++++++++++++++-------------- 1 file changed, 17 insertions(+), 14 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 5670129205..5e7ca9dac0 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -18,7 +18,7 @@ KEYS_FILE = ( Path(__file__).resolve().parent.parent / "zulipterminal" / "config" / "keys.py" ) KEYS_FILE_NAME = KEYS_FILE.name -OUTPUT_FILE = Path(__file__).resolve().parent.parent / "docs" / "hotkeys.md" +CATEGORY_OUTPUT_FILE = Path(__file__).resolve().parent.parent / "docs" / "hotkeys.md" SCRIPT_NAME = PurePath(__file__).name HELP_TEXT_STYLE = re.compile(r"^[a-zA-Z /()',&@#:_-]*$") @@ -58,7 +58,7 @@ def lint_hotkeys_file() -> None: """ Lint KEYS_FILE for valid key descriptions (help texts) and key categories, check for duplicate key combinations, - and compare with existing OUTPUT_FILE. + and compare with existing output file. """ error_flag = False @@ -71,14 +71,15 @@ def lint_hotkeys_file() -> None: ) else: print("No hotkeys linting errors") - if not OUTPUT_FILE.exists(): + output_file = CATEGORY_OUTPUT_FILE + if not output_file.exists(): raise SystemExit( - f"Run './tools/{SCRIPT_NAME} --fix' to generate {OUTPUT_FILE.name} file" + f"Run './tools/{SCRIPT_NAME} --fix' to generate {output_file.name} file" ) hotkeys_file_string = generate_hotkeys_file_string() if not output_file_matches_string(hotkeys_file_string): raise SystemExit( - f"Run './tools/{SCRIPT_NAME} --fix' to update {OUTPUT_FILE.name} file" + f"Run './tools/{SCRIPT_NAME} --fix' to update {output_file.name} file" ) @@ -152,19 +153,20 @@ def lint_help_groups() -> bool: def generate_hotkeys_file() -> None: """ - Generate OUTPUT_FILE based on help text description and + Generate output file based on help text description and shortcut key combinations in KEYS_FILE, grouped by categories """ + output_file = CATEGORY_OUTPUT_FILE hotkeys_file_string = generate_hotkeys_file_string() output_file_matches_string(hotkeys_file_string) - with open(OUTPUT_FILE, "w") as hotkeys_file: + with open(output_file, "w") as hotkeys_file: hotkeys_file.write(hotkeys_file_string) - print(f"Hot Keys list saved in {OUTPUT_FILE}") + print(f"Hot Keys list saved in {output_file.name}") def generate_hotkeys_file_string() -> str: """ - Construct string in form for output to OUTPUT_FILE based on help text + Construct string in form for output to output file based on help text description and shortcut key combinations in KEYS_FILE """ help_group = HELP_CATEGORIES @@ -194,12 +196,13 @@ def generate_hotkeys_file_string() -> str: def output_file_matches_string(hotkeys_file_string: str) -> bool: """ - Check if the OUTPUT_FILE exists and matches the generated hotkeys_file_string + Check if the output file exists and matches the generated hotkeys_file_string """ - output_file_name = OUTPUT_FILE.name + output_file = CATEGORY_OUTPUT_FILE + output_file_name = output_file.name try: - with open(OUTPUT_FILE) as output_file: - content_is_identical = hotkeys_file_string == output_file.read() + with open(output_file) as existing_file: + content_is_identical = hotkeys_file_string == existing_file.read() if content_is_identical: print( f"{output_file_name} file already in sync with config/{KEYS_FILE_NAME}" @@ -221,7 +224,7 @@ if __name__ == "__main__": parser.add_argument( "--fix", action="store_true", - help=f"Generate {OUTPUT_FILE.name} file by extracting key description and key " + help=f"Generate {CATEGORY_OUTPUT_FILE.name} file by extracting key description and key " f"combination from config/{KEYS_FILE_NAME} file", ) args = parser.parse_args() From e2c7e570dbe980bb3486e4aa8fcf6b55528cf176 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Wed, 17 Jul 2024 18:17:04 +0530 Subject: [PATCH 24/34] refactor: lint-hotkeys: Generalize the traversal of key group values. Convert string entries of `key_group` into lists, to later support the list of strings `key_contexts` with the same code. --- tools/lint-hotkeys | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 5e7ca9dac0..65b0b7b248 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -3,7 +3,7 @@ import argparse import re from collections import defaultdict from pathlib import Path, PurePath -from typing import Dict, List, Tuple +from typing import Dict, List, Tuple, Union from typing_extensions import Literal @@ -38,9 +38,11 @@ def read_help_groups() -> Dict[str, List[Tuple[str, List[str]]]]: key_group: KeyGroup = "key_category" entries_by_group = defaultdict(list) for cmd, item in KEY_BINDINGS.items(): - entries_by_group[item[key_group]].append( - (item["help_text"], display_keys_for_command(cmd)) - ) + groups = [item[key_group]] + for group in groups: + entries_by_group[group].append( + (item["help_text"], display_keys_for_command(cmd)) + ) return entries_by_group @@ -138,10 +140,13 @@ def lint_help_groups() -> bool: # Lint for typos in key categories error_message = "" for key, binding in KEY_BINDINGS.items(): - group_value = binding[key_group] - if group_value not in help_group: - error_message += f" Invalid {group} '{group_value}' for key '{key}'.\n" - error_flag = True + group_values: Union[str, List[str]] = binding[key_group] + if isinstance(group_values, str): + group_values = [group_values] + for group_value in group_values: + if group_value not in help_group: + error_message += f" Invalid {group} '{group_value}' for key '{key}'.\n" + error_flag = True if error_message: print( f"Choose a valid {group} value from:\n{', '.join(help_group.keys())}\n" From f908dc6539c58db567510905db8361d0a50c9c12 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Thu, 18 Jul 2024 06:16:35 +0530 Subject: [PATCH 25/34] refactor: lint-hotkeys: Use bundled group values. Add a TypedDict bundling the values associated with category/context, and a dictionary mapping the group to its bundle. Pass group argument to all functions, to use the respective 'bundled' values associated with that group. --- tools/lint-hotkeys | 59 +++++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 22 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 65b0b7b248..6bef778f4e 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -5,7 +5,7 @@ from collections import defaultdict from pathlib import Path, PurePath from typing import Dict, List, Tuple, Union -from typing_extensions import Literal +from typing_extensions import Literal, TypedDict from zulipterminal.config.keys import ( HELP_CATEGORIES, @@ -26,16 +26,17 @@ HELP_TEXT_STYLE = re.compile(r"^[a-zA-Z /()',&@#:_-]*$") KEYS_TO_EXCLUDE = ["q", "e", "m", "r", "Esc"] +Group = Literal["category"] KeyGroup = Literal["key_category"] +GroupedHelpEntries = Dict[str, List[Tuple[str, List[str]]]] -def read_help_groups() -> Dict[str, List[Tuple[str, List[str]]]]: +def read_help_groups(key_group: KeyGroup) -> GroupedHelpEntries: """ Generate a dict from KEYS_FILE key: help category value: a list of help texts and key combinations, for each key binding in that help category """ - key_group: KeyGroup = "key_category" entries_by_group = defaultdict(list) for cmd, item in KEY_BINDINGS.items(): groups = [item[key_group]] @@ -46,12 +47,26 @@ def read_help_groups() -> Dict[str, List[Tuple[str, List[str]]]]: return entries_by_group -ENTRIES_BY_CATEGORY: Dict[str, List[Tuple[str, List[str]]]] = read_help_groups() +class HelpGroupBundle(TypedDict): + help_group: Dict[str, str] + entries_by_group: GroupedHelpEntries + key_group: KeyGroup + output_file: Path + + +GROUP_MAPPING: Dict[Group, HelpGroupBundle] = { + "category": { + "help_group": HELP_CATEGORIES, + "entries_by_group": read_help_groups("key_category"), + "key_group": "key_category", + "output_file": CATEGORY_OUTPUT_FILE, + }, +} def main(fix: bool) -> None: if fix: - generate_hotkeys_file() + generate_hotkeys_file("category") else: lint_hotkeys_file() @@ -64,7 +79,7 @@ def lint_hotkeys_file() -> None: """ error_flag = False - error_flag |= lint_help_groups() + error_flag |= lint_help_groups("category") error_flag |= lint_help_text() if error_flag: @@ -78,8 +93,8 @@ def lint_hotkeys_file() -> None: raise SystemExit( f"Run './tools/{SCRIPT_NAME} --fix' to generate {output_file.name} file" ) - hotkeys_file_string = generate_hotkeys_file_string() - if not output_file_matches_string(hotkeys_file_string): + hotkeys_file_string = generate_hotkeys_file_string("category") + if not output_file_matches_string(hotkeys_file_string, "category"): raise SystemExit( f"Run './tools/{SCRIPT_NAME} --fix' to update {output_file.name} file" ) @@ -107,15 +122,15 @@ def lint_help_text() -> bool: return error_flag -def lint_help_groups() -> bool: +def lint_help_groups(group: Group) -> bool: """ Lint help groups by checking each key combination for duplicates within the same group and the validity of each category (typo-checking) """ - group = "category" - help_group = HELP_CATEGORIES - entries_by_group = ENTRIES_BY_CATEGORY - key_group: KeyGroup = "key_category" + bundle = GROUP_MAPPING[group] + help_group = bundle["help_group"] + entries_by_group = bundle["entries_by_group"] + key_group: KeyGroup = bundle["key_group"] # Lint for duplicate key combinations within the same category error_flag = False @@ -156,26 +171,26 @@ def lint_help_groups() -> bool: return error_flag -def generate_hotkeys_file() -> None: +def generate_hotkeys_file(group: Group) -> None: """ Generate output file based on help text description and shortcut key combinations in KEYS_FILE, grouped by categories """ - output_file = CATEGORY_OUTPUT_FILE - hotkeys_file_string = generate_hotkeys_file_string() - output_file_matches_string(hotkeys_file_string) + output_file = GROUP_MAPPING[group]["output_file"] + hotkeys_file_string = generate_hotkeys_file_string(group) + output_file_matches_string(hotkeys_file_string, group) with open(output_file, "w") as hotkeys_file: hotkeys_file.write(hotkeys_file_string) print(f"Hot Keys list saved in {output_file.name}") -def generate_hotkeys_file_string() -> str: +def generate_hotkeys_file_string(group: Group) -> str: """ Construct string in form for output to output file based on help text description and shortcut key combinations in KEYS_FILE """ - help_group = HELP_CATEGORIES - entries_by_group = ENTRIES_BY_CATEGORY + help_group = GROUP_MAPPING[group]["help_group"] + entries_by_group = GROUP_MAPPING[group]["entries_by_group"] hotkeys_file_string = ( f"\n" @@ -199,11 +214,11 @@ def generate_hotkeys_file_string() -> str: return hotkeys_file_string -def output_file_matches_string(hotkeys_file_string: str) -> bool: +def output_file_matches_string(hotkeys_file_string: str, group: Group) -> bool: """ Check if the output file exists and matches the generated hotkeys_file_string """ - output_file = CATEGORY_OUTPUT_FILE + output_file = GROUP_MAPPING[group]["output_file"] output_file_name = output_file.name try: with open(output_file) as existing_file: From f5d0bc4c4f72feadfd2a25280a691e21a70a70ac Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Tue, 9 Jul 2024 01:27:34 +0530 Subject: [PATCH 26/34] keys: Add a contexts field to Key Binding. Add a dict of supported hotkey contexts for key bindings. Used a dict, instead of a list to facilitate the mapping of internal names to user-friendly names. Added TODO comments marking key bindings that require condition checks. Tests updated. Linting added. --- tests/config/test_keys.py | 4 ++ tools/lint-hotkeys | 33 +++++++---- zulipterminal/config/keys.py | 108 ++++++++++++++++++++++++++++++++++- 3 files changed, 132 insertions(+), 13 deletions(-) diff --git a/tests/config/test_keys.py b/tests/config/test_keys.py index 752e69af29..de9e127be1 100644 --- a/tests/config/test_keys.py +++ b/tests/config/test_keys.py @@ -76,23 +76,27 @@ def test_commands_for_random_tips(mocker: MockerFixture) -> None: "keys": ["a"], "help_text": "alpha", "key_category": "category 1", + "key_contexts": ["context 1", "context 2"], "excluded_from_random_tips": True, }, "BETA": { "keys": ["b"], "help_text": "beta", "key_category": "category 1", + "key_contexts": ["context 1"], "excluded_from_random_tips": False, }, "GAMMA": { "keys": ["g"], "help_text": "gamma", "key_category": "category 1", + "key_contexts": ["global"], }, "DELTA": { "keys": ["d"], "help_text": "delta", "key_category": "category 2", + "key_contexts": ["context 2"], "excluded_from_random_tips": True, }, } diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 6bef778f4e..83ac46f976 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -9,6 +9,7 @@ from typing_extensions import Literal, TypedDict from zulipterminal.config.keys import ( HELP_CATEGORIES, + HELP_CONTEXTS, KEY_BINDINGS, display_keys_for_command, ) @@ -19,6 +20,7 @@ KEYS_FILE = ( ) KEYS_FILE_NAME = KEYS_FILE.name CATEGORY_OUTPUT_FILE = Path(__file__).resolve().parent.parent / "docs" / "hotkeys.md" +CONTEXT_OUTPUT_FILE = Path(__file__).resolve().parent.parent / "docs" / "contexts.md" SCRIPT_NAME = PurePath(__file__).name HELP_TEXT_STYLE = re.compile(r"^[a-zA-Z /()',&@#:_-]*$") @@ -26,20 +28,20 @@ HELP_TEXT_STYLE = re.compile(r"^[a-zA-Z /()',&@#:_-]*$") KEYS_TO_EXCLUDE = ["q", "e", "m", "r", "Esc"] -Group = Literal["category"] -KeyGroup = Literal["key_category"] +Group = Literal["category", "context"] +KeyGroup = Literal["key_category", "key_contexts"] GroupedHelpEntries = Dict[str, List[Tuple[str, List[str]]]] def read_help_groups(key_group: KeyGroup) -> GroupedHelpEntries: """ Generate a dict from KEYS_FILE - key: help category - value: a list of help texts and key combinations, for each key binding in that help category + key: help category/context + value: a list of help texts and key combinations, for each key binding in that help category/context """ entries_by_group = defaultdict(list) for cmd, item in KEY_BINDINGS.items(): - groups = [item[key_group]] + groups = [item[key_group]] if key_group == "key_category" else item[key_group] for group in groups: entries_by_group[group].append( (item["help_text"], display_keys_for_command(cmd)) @@ -61,6 +63,12 @@ GROUP_MAPPING: Dict[Group, HelpGroupBundle] = { "key_group": "key_category", "output_file": CATEGORY_OUTPUT_FILE, }, + "context": { + "help_group": HELP_CONTEXTS, + "entries_by_group": read_help_groups("key_contexts"), + "key_group": "key_contexts", + "output_file": CONTEXT_OUTPUT_FILE, + }, } @@ -73,14 +81,15 @@ def main(fix: bool) -> None: def lint_hotkeys_file() -> None: """ - Lint KEYS_FILE for valid key descriptions (help texts) and key categories, + Lint KEYS_FILE for valid key descriptions (help texts) and key categories/contexts, check for duplicate key combinations, and compare with existing output file. """ error_flag = False - error_flag |= lint_help_groups("category") - error_flag |= lint_help_text() + error_flag |= ( + lint_help_groups("category") | lint_help_groups("context") | lint_help_text() + ) if error_flag: raise SystemExit( @@ -125,14 +134,14 @@ def lint_help_text() -> bool: def lint_help_groups(group: Group) -> bool: """ Lint help groups by checking each key combination for duplicates - within the same group and the validity of each category (typo-checking) + within the same group and the validity of each category/context (typo-checking) """ bundle = GROUP_MAPPING[group] help_group = bundle["help_group"] entries_by_group = bundle["entries_by_group"] key_group: KeyGroup = bundle["key_group"] - # Lint for duplicate key combinations within the same category + # Lint for duplicate key combinations within the same category/context error_flag = False for batch in help_group: check_duplicate_keys_list: List[str] = [] @@ -152,7 +161,7 @@ def lint_help_groups(group: Group) -> bool: ) error_flag = True - # Lint for typos in key categories + # Lint for typos in key categories/contexts error_message = "" for key, binding in KEY_BINDINGS.items(): group_values: Union[str, List[str]] = binding[key_group] @@ -174,7 +183,7 @@ def lint_help_groups(group: Group) -> bool: def generate_hotkeys_file(group: Group) -> None: """ Generate output file based on help text description and - shortcut key combinations in KEYS_FILE, grouped by categories + shortcut key combinations in KEYS_FILE, grouped by categories/contexts """ output_file = GROUP_MAPPING[group]["output_file"] hotkeys_file_string = generate_hotkeys_file_string(group) diff --git a/zulipterminal/config/keys.py b/zulipterminal/config/keys.py index 2e9fa40b79..5d347f0c5d 100644 --- a/zulipterminal/config/keys.py +++ b/zulipterminal/config/keys.py @@ -23,6 +23,7 @@ class KeyBinding(TypedDict): help_text: str excluded_from_random_tips: NotRequired[bool] key_category: str + key_contexts: List[str] # fmt: off @@ -35,270 +36,329 @@ class KeyBinding(TypedDict): 'help_text': 'Show/hide Help Menu', 'excluded_from_random_tips': True, 'key_category': 'general', + 'key_contexts': ['general'], }, 'MARKDOWN_HELP': { 'keys': ['meta m'], 'help_text': 'Show/hide Markdown Help Menu', 'key_category': 'general', + 'key_contexts': ['general'], }, 'ABOUT': { 'keys': ['meta ?'], 'help_text': 'Show/hide About Menu', 'key_category': 'general', + 'key_contexts': ['general'], }, 'OPEN_DRAFT': { 'keys': ['d'], 'help_text': 'Open draft message saved in this session', 'key_category': 'open_compose', + 'key_contexts': ['general'], }, 'COPY_ABOUT_INFO': { 'keys': ['c'], 'help_text': 'Copy information from About Menu to clipboard', 'key_category': 'general', + 'key_contexts': ['about'], }, 'EXIT_POPUP': { 'keys': ['esc'], 'help_text': 'Close popup', 'key_category': 'navigation', + 'key_contexts': ['popup'], }, 'GO_UP': { 'keys': ['up', 'k'], 'help_text': 'Go up / Previous message', 'key_category': 'navigation', + 'key_contexts': ['general'], }, 'GO_DOWN': { 'keys': ['down', 'j'], 'help_text': 'Go down / Next message', 'key_category': 'navigation', + 'key_contexts': ['general'], }, 'GO_LEFT': { 'keys': ['left', 'h'], 'help_text': 'Go left', 'key_category': 'navigation', + 'key_contexts': ['general'], }, 'GO_RIGHT': { 'keys': ['right', 'l'], 'help_text': 'Go right', 'key_category': 'navigation', + 'key_contexts': ['general'], }, 'SCROLL_UP': { 'keys': ['page up', 'K'], 'help_text': 'Scroll up', 'key_category': 'navigation', + 'key_contexts': ['general'], }, 'SCROLL_DOWN': { 'keys': ['page down', 'J'], 'help_text': 'Scroll down', 'key_category': 'navigation', + 'key_contexts': ['general'], }, 'GO_TO_BOTTOM': { 'keys': ['end', 'G'], 'help_text': 'Go to bottom / Last message', 'key_category': 'navigation', + 'key_contexts': ['general'], }, 'ACTIVATE_BUTTON': { 'keys': ['enter', ' '], 'help_text': 'Trigger the selected entry', 'key_category': 'navigation', + 'key_contexts': ['button'], }, 'REPLY_MESSAGE': { 'keys': ['r', 'enter'], 'help_text': 'Reply to the current message', 'key_category': 'open_compose', + 'key_contexts': ['message'], }, 'MENTION_REPLY': { 'keys': ['@'], 'help_text': 'Reply mentioning the sender of the current message', 'key_category': 'open_compose', + 'key_contexts': ['message'], }, 'QUOTE_REPLY': { 'keys': ['>'], 'help_text': 'Reply quoting the current message text', 'key_category': 'open_compose', + 'key_contexts': ['message'], }, 'REPLY_AUTHOR': { 'keys': ['R'], 'help_text': 'Reply directly to the sender of the current message', 'key_category': 'open_compose', + 'key_contexts': ['message'], }, 'EDIT_MESSAGE': { 'keys': ['e'], 'help_text': "Edit message's content or topic", - 'key_category': 'msg_actions' + 'key_category': 'msg_actions', + 'key_contexts': ['message'], }, 'STREAM_MESSAGE': { 'keys': ['c'], 'help_text': 'New message to a stream', 'key_category': 'open_compose', + 'key_contexts': ['general'], }, 'PRIVATE_MESSAGE': { 'keys': ['x'], 'help_text': 'New message to a person or group of people', 'key_category': 'open_compose', + 'key_contexts': ['general'], }, 'CYCLE_COMPOSE_FOCUS': { 'keys': ['tab'], 'help_text': 'Cycle through recipient and content boxes', 'key_category': 'compose_box', + 'key_contexts': ['compose_box'], }, 'SEND_MESSAGE': { 'keys': ['ctrl d', 'meta enter'], 'help_text': 'Send a message', 'key_category': 'compose_box', + 'key_contexts': ['compose_box'], }, 'SAVE_AS_DRAFT': { 'keys': ['meta s'], 'help_text': 'Save current message as a draft', 'key_category': 'compose_box', + 'key_contexts': ['compose_box'], }, 'AUTOCOMPLETE': { 'keys': ['ctrl f'], 'help_text': ('Autocomplete @mentions, #stream_names, :emoji:' ' and topics'), 'key_category': 'compose_box', + 'key_contexts': ['compose_box'], }, 'AUTOCOMPLETE_REVERSE': { 'keys': ['ctrl r'], 'help_text': 'Cycle through autocomplete suggestions in reverse', 'key_category': 'compose_box', + 'key_contexts': ['compose_box'], }, 'ADD_REACTION': { 'keys': [':'], 'help_text': 'Show/hide emoji picker for current message', 'key_category': 'msg_actions', + 'key_contexts': ['message'], }, 'STREAM_NARROW': { 'keys': ['s'], 'help_text': 'View the stream of the current message', 'key_category': 'narrowing', + 'key_contexts': ['message'], }, 'TOPIC_NARROW': { 'keys': ['S'], 'help_text': 'View the topic of the current message', 'key_category': 'narrowing', + 'key_contexts': ['message'], }, 'TOGGLE_NARROW': { 'keys': ['z'], 'help_text': "Zoom in/out the message's conversation context", 'key_category': 'narrowing', + 'key_contexts': ['message'], }, 'NARROW_MESSAGE_RECIPIENT': { 'keys': ['meta .'], 'help_text': 'Switch message view to the compose box target', 'key_category': 'narrowing', + 'key_contexts': ['compose_box'], }, 'EXIT_COMPOSE': { 'keys': ['esc'], 'help_text': 'Exit message compose box', 'key_category': 'compose_box', + 'key_contexts': ['compose_box'], }, 'REACTION_AGREEMENT': { 'keys': ['='], 'help_text': 'Toggle first emoji reaction on selected message', 'key_category': 'msg_actions', + 'key_contexts': ['message'], }, 'TOGGLE_TOPIC': { 'keys': ['t'], 'help_text': 'Toggle topics in a stream', 'key_category': 'stream_list', + 'key_contexts': ['stream', 'topic'], }, 'ALL_MESSAGES': { 'keys': ['a', 'esc'], 'help_text': 'View all messages', 'key_category': 'narrowing', + 'key_contexts': ['message'], }, 'ALL_PM': { 'keys': ['P'], 'help_text': 'View all direct messages', 'key_category': 'narrowing', + 'key_contexts': ['general'], }, 'ALL_STARRED': { 'keys': ['f'], 'help_text': 'View all starred messages', 'key_category': 'narrowing', + 'key_contexts': ['general'], }, 'ALL_MENTIONS': { 'keys': ['#'], 'help_text': "View all messages in which you're mentioned", 'key_category': 'narrowing', + 'key_contexts': ['general'], }, 'NEXT_UNREAD_TOPIC': { 'keys': ['n'], 'help_text': 'Next unread topic', 'key_category': 'narrowing', + 'key_contexts': ['general'], }, 'NEXT_UNREAD_PM': { 'keys': ['p'], 'help_text': 'Next unread direct message', 'key_category': 'narrowing', + 'key_contexts': ['general'], }, 'SEARCH_PEOPLE': { 'keys': ['w'], 'help_text': 'Search users', 'key_category': 'searching', + 'key_contexts': ['general'], }, 'SEARCH_MESSAGES': { 'keys': ['/'], 'help_text': 'Search messages', 'key_category': 'searching', + 'key_contexts': ['general'], }, 'SEARCH_STREAMS': { 'keys': ['q'], 'help_text': 'Search streams', 'key_category': 'searching', + 'key_contexts': ['general'], + 'excluded_from_random_tips': True, + # TODO: condition check required }, 'SEARCH_TOPICS': { 'keys': ['q'], 'help_text': 'Search topics in a stream', 'key_category': 'searching', + 'key_contexts': ['general'], + 'excluded_from_random_tips': True, + # TODO: condition check required }, 'SEARCH_EMOJIS': { 'keys': ['p'], 'help_text': 'Search emojis from emoji picker', 'excluded_from_random_tips': True, 'key_category': 'searching', + 'key_contexts': ['emoji_list'], }, 'EXECUTE_SEARCH': { 'keys': ['enter'], 'help_text': 'Submit search and browse results', 'key_category': 'searching', + 'key_contexts': ['search_box'], }, 'CLEAR_SEARCH': { 'keys': ['esc'], 'help_text': 'Clear search in current panel', 'key_category': 'searching', + 'key_contexts': ['message', 'stream', 'topic', 'user'], + 'excluded_from_random_tips': True, + # TODO: condition check required }, 'TOGGLE_MUTE_STREAM': { 'keys': ['m'], 'help_text': 'Mute/unmute streams', 'key_category': 'stream_list', + 'key_contexts': ['stream'], }, 'THUMBS_UP': { 'keys': ['+'], 'help_text': 'Toggle thumbs-up reaction to the current message', 'key_category': 'msg_actions', + 'key_contexts': ['message'], }, 'TOGGLE_STAR_STATUS': { 'keys': ['ctrl s', '*'], 'help_text': 'Toggle star status of the current message', 'key_category': 'msg_actions', + 'key_contexts': ['message'], }, 'MSG_INFO': { 'keys': ['i'], 'help_text': 'Show/hide message information', 'key_category': 'msg_actions', + 'key_contexts': ['message', 'msg_info'], }, 'MSG_SENDER_INFO': { 'keys': ['u'], 'help_text': 'Show/hide message sender information', 'key_category': 'msg_actions', + 'key_contexts': ['msg_info'], }, 'EDIT_HISTORY': { 'keys': ['e'], 'help_text': 'Show/hide edit history (from message information)', 'excluded_from_random_tips': True, 'key_category': 'msg_actions', + 'key_contexts': ['msg_info'], }, 'VIEW_IN_BROWSER': { 'keys': ['v'], @@ -306,17 +366,20 @@ class KeyBinding(TypedDict): 'View current message in browser (from message information)', 'excluded_from_random_tips': True, 'key_category': 'msg_actions', + 'key_contexts': ['msg_info'], }, 'STREAM_INFO': { 'keys': ['i'], 'help_text': 'Show/hide stream information & modify settings', 'key_category': 'stream_list', + 'key_contexts': ['stream', 'stream_info'], }, 'STREAM_MEMBERS': { 'keys': ['m'], 'help_text': 'Show/hide stream members (from stream information)', 'excluded_from_random_tips': True, 'key_category': 'stream_list', + 'key_contexts': ['stream_info'], }, 'COPY_STREAM_EMAIL': { 'keys': ['c'], @@ -324,101 +387,121 @@ class KeyBinding(TypedDict): 'Copy stream email to clipboard (from stream information)', 'excluded_from_random_tips': True, 'key_category': 'stream_list', + 'key_contexts': ['stream_info'], }, 'REDRAW': { 'keys': ['ctrl l'], 'help_text': 'Redraw screen', 'key_category': 'general', + 'key_contexts': ['global'], }, 'QUIT': { 'keys': ['ctrl c'], 'help_text': 'Quit', 'key_category': 'general', + 'key_contexts': ['global'], }, 'USER_INFO': { 'keys': ['i'], 'help_text': 'Show/hide user information (from users list)', 'key_category': 'general', + 'key_contexts': ['user'], }, 'BEGINNING_OF_LINE': { 'keys': ['ctrl a', 'home'], 'help_text': 'Start of line', 'key_category': 'editor_navigation', + 'key_contexts': ['editor'], }, 'END_OF_LINE': { 'keys': ['ctrl e', 'end'], 'help_text': 'End of line', 'key_category': 'editor_navigation', + 'key_contexts': ['editor'], }, 'ONE_WORD_BACKWARD': { 'keys': ['meta b', 'shift left'], 'help_text': 'Start of current or previous word', 'key_category': 'editor_navigation', + 'key_contexts': ['editor'], }, 'ONE_WORD_FORWARD': { 'keys': ['meta f', 'shift right'], 'help_text': 'Start of next word', 'key_category': 'editor_navigation', + 'key_contexts': ['editor'], }, 'PREV_LINE': { 'keys': ['up', 'ctrl p'], 'help_text': 'Previous line', 'key_category': 'editor_navigation', + 'key_contexts': ['editor'], }, 'NEXT_LINE': { 'keys': ['down', 'ctrl n'], 'help_text': 'Next line', 'key_category': 'editor_navigation', + 'key_contexts': ['editor'], }, 'UNDO_LAST_ACTION': { 'keys': ['ctrl _'], 'help_text': 'Undo last action', 'key_category': 'editor_text_manipulation', + 'key_contexts': ['editor'], }, 'CLEAR_MESSAGE': { 'keys': ['ctrl l'], 'help_text': 'Clear text box', 'key_category': 'editor_text_manipulation', + 'key_contexts': ['editor'], }, 'CUT_TO_END_OF_LINE': { 'keys': ['ctrl k'], 'help_text': 'Cut forwards to the end of the line', 'key_category': 'editor_text_manipulation', + 'key_contexts': ['editor'], }, 'CUT_TO_START_OF_LINE': { 'keys': ['ctrl u'], 'help_text': 'Cut backwards to the start of the line', 'key_category': 'editor_text_manipulation', + 'key_contexts': ['editor'], }, 'CUT_TO_END_OF_WORD': { 'keys': ['meta d'], 'help_text': 'Cut forwards to the end of the current word', 'key_category': 'editor_text_manipulation', + 'key_contexts': ['editor'], }, 'CUT_TO_START_OF_WORD': { 'keys': ['ctrl w', 'meta backspace'], 'help_text': 'Cut backwards to the start of the current word', 'key_category': 'editor_text_manipulation', + 'key_contexts': ['editor'], }, 'CUT_WHOLE_LINE': { 'keys': ['meta x'], 'help_text': 'Cut the current line', 'key_category': 'editor_text_manipulation', + 'key_contexts': ['editor'], }, 'PASTE_LAST_CUT': { 'keys': ['ctrl y'], 'help_text': 'Paste last cut section', 'key_category': 'editor_text_manipulation', + 'key_contexts': ['editor'], }, 'DELETE_LAST_CHARACTER': { 'keys': ['ctrl h'], 'help_text': 'Delete previous character', 'key_category': 'editor_text_manipulation', + 'key_contexts': ['editor'], }, 'TRANSPOSE_CHARACTERS': { 'keys': ['ctrl t'], 'help_text': 'Swap with previous character', 'key_category': 'editor_text_manipulation', + 'key_contexts': ['editor'], }, 'NEW_LINE': { # urwid_readline's command @@ -427,26 +510,31 @@ class KeyBinding(TypedDict): 'keys': ['enter'], 'help_text': 'Insert new line', 'key_category': 'compose_box', + 'key_contexts': ['compose_box'], }, 'OPEN_EXTERNAL_EDITOR': { 'keys': ['ctrl o'], 'help_text': 'Open an external editor to edit the message content', 'key_category': 'compose_box', + 'key_contexts': ['compose_box'], }, 'FULL_RENDERED_MESSAGE': { 'keys': ['f'], 'help_text': 'Show/hide full rendered message (from message information)', 'key_category': 'msg_actions', + 'key_contexts': ['msg_info'], }, 'FULL_RAW_MESSAGE': { 'keys': ['r'], 'help_text': 'Show/hide full raw message (from message information)', 'key_category': 'msg_actions', + 'key_contexts': ['msg_info'], }, 'NEW_HINT': { 'keys': ['tab'], 'help_text': 'New footer hotkey hint', 'key_category': 'general', + 'key_contexts': ['general'], }, } # fmt: on @@ -464,6 +552,24 @@ class KeyBinding(TypedDict): "editor_text_manipulation": "Editor: Text Manipulation", } +HELP_CONTEXTS: Dict[str, str] = { + "global": "Global", + "general": "General", # not in an editor or a popup + "editor": "Editor", + "compose_box": "Compose box", + "stream": "Stream list", + "topic": "Topic list", + "user": "User list", + "message": "Message", + "stream_info": "Stream information", + "msg_info": "Message information", + "emoji_list": "Emoji list", + "about": "About information", + "popup": "Popup", + "button": "Button", + "search_box": "Search box", +} + ZT_TO_URWID_CMD_MAPPING = { "GO_UP": CURSOR_UP, "GO_DOWN": CURSOR_DOWN, From a8dd4de6f8b262a20b7f99be3355e4603cf48b84 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Thu, 18 Jul 2024 06:33:55 +0530 Subject: [PATCH 27/34] lint-hotkeys: Add argument to generate a keys file grouped by contexts. --- tools/lint-hotkeys | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index 83ac46f976..d7b7914b64 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -72,10 +72,12 @@ GROUP_MAPPING: Dict[Group, HelpGroupBundle] = { } -def main(fix: bool) -> None: +def main(fix: bool, generate_context_file: bool) -> None: if fix: generate_hotkeys_file("category") - else: + if generate_context_file: + generate_hotkeys_file("context") + if not fix and not generate_context_file: lint_hotkeys_file() @@ -256,5 +258,11 @@ if __name__ == "__main__": help=f"Generate {CATEGORY_OUTPUT_FILE.name} file by extracting key description and key " f"combination from config/{KEYS_FILE_NAME} file", ) + parser.add_argument( + "--generate-context-file", + action="store_true", + help=f"Generate {CONTEXT_OUTPUT_FILE.name} file by extracting key description and key " + f"combination from config/{KEYS_FILE_NAME} file", + ) args = parser.parse_args() - main(args.fix) + main(args.fix, args.generate_context_file) From a76da7fe84d3655ff1857feb80788e7609d56db5 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Tue, 9 Jul 2024 01:59:40 +0530 Subject: [PATCH 28/34] keys: Use the help contexts in generating random help tips. Tests added and updated. --- tests/config/test_keys.py | 31 +++++++++++++++++++++++-------- zulipterminal/config/keys.py | 10 ++++++++-- 2 files changed, 31 insertions(+), 10 deletions(-) diff --git a/tests/config/test_keys.py b/tests/config/test_keys.py index de9e127be1..813aaac993 100644 --- a/tests/config/test_keys.py +++ b/tests/config/test_keys.py @@ -70,20 +70,30 @@ def test_HELP_is_not_allowed_as_tip() -> None: assert keys.KEY_BINDINGS["HELP"] not in keys.commands_for_random_tips() -def test_commands_for_random_tips(mocker: MockerFixture) -> None: +@pytest.mark.parametrize( + "context, expected_command", + [ + (None, "GAMMA"), + ("context_1", "BETA"), + ("context_2", "GAMMA"), + ], +) +def test_commands_for_random_tips( + context: str, expected_command: str, mocker: MockerFixture +) -> None: new_key_bindings: Dict[str, keys.KeyBinding] = { "ALPHA": { "keys": ["a"], "help_text": "alpha", "key_category": "category 1", - "key_contexts": ["context 1", "context 2"], + "key_contexts": ["context_1", "context_2"], "excluded_from_random_tips": True, }, "BETA": { "keys": ["b"], "help_text": "beta", "key_category": "category 1", - "key_contexts": ["context 1"], + "key_contexts": ["context_1"], "excluded_from_random_tips": False, }, "GAMMA": { @@ -96,15 +106,20 @@ def test_commands_for_random_tips(mocker: MockerFixture) -> None: "keys": ["d"], "help_text": "delta", "key_category": "category 2", - "key_contexts": ["context 2"], + "key_contexts": ["context_2"], "excluded_from_random_tips": True, }, } + new_help_contexts: Dict[str, str] = { + "global": "Global", + "context_1": "Context 1", + "context_2": "Context 2", + } mocker.patch.dict(keys.KEY_BINDINGS, new_key_bindings, clear=True) - result = keys.commands_for_random_tips() - assert len(result) == 2 - assert new_key_bindings["BETA"] in result - assert new_key_bindings["GAMMA"] in result + mocker.patch.object(keys, "HELP_CONTEXTS", new_help_contexts) + result = keys.commands_for_random_tips(context) + assert len(result) == 1 + assert new_key_bindings[expected_command] in result def test_updated_urwid_command_map() -> None: diff --git a/zulipterminal/config/keys.py b/zulipterminal/config/keys.py index 5d347f0c5d..e6205e7a57 100644 --- a/zulipterminal/config/keys.py +++ b/zulipterminal/config/keys.py @@ -655,15 +655,21 @@ def primary_display_key_for_command(command: str) -> str: return display_key_for_urwid_key(primary_key_for_command(command)) -def commands_for_random_tips() -> List[KeyBinding]: +def commands_for_random_tips(context: str = "") -> List[KeyBinding]: """ Return list of commands which may be displayed as a random tip """ - return [ + if not context or context not in HELP_CONTEXTS: + context = "global" + random_tips: List[KeyBinding] = [ key_binding for key_binding in KEY_BINDINGS.values() if not key_binding.get("excluded_from_random_tips", False) + and context in key_binding["key_contexts"] ] + if len(random_tips) == 0: + return commands_for_random_tips("global") + return random_tips # Refer urwid/command_map.py From 9c48eb7341b1208be5dd5cef4d38557264ed6d8f Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Tue, 16 Apr 2024 16:19:44 +0530 Subject: [PATCH 29/34] keys: Include previously excluded random help hints. Hotkeys that used to be excluded because they were too specific, can now be useful as hints in particular contexts. --- zulipterminal/config/keys.py | 5 ----- 1 file changed, 5 deletions(-) diff --git a/zulipterminal/config/keys.py b/zulipterminal/config/keys.py index e6205e7a57..c946a97361 100644 --- a/zulipterminal/config/keys.py +++ b/zulipterminal/config/keys.py @@ -305,7 +305,6 @@ class KeyBinding(TypedDict): 'SEARCH_EMOJIS': { 'keys': ['p'], 'help_text': 'Search emojis from emoji picker', - 'excluded_from_random_tips': True, 'key_category': 'searching', 'key_contexts': ['emoji_list'], }, @@ -356,7 +355,6 @@ class KeyBinding(TypedDict): 'EDIT_HISTORY': { 'keys': ['e'], 'help_text': 'Show/hide edit history (from message information)', - 'excluded_from_random_tips': True, 'key_category': 'msg_actions', 'key_contexts': ['msg_info'], }, @@ -364,7 +362,6 @@ class KeyBinding(TypedDict): 'keys': ['v'], 'help_text': 'View current message in browser (from message information)', - 'excluded_from_random_tips': True, 'key_category': 'msg_actions', 'key_contexts': ['msg_info'], }, @@ -377,7 +374,6 @@ class KeyBinding(TypedDict): 'STREAM_MEMBERS': { 'keys': ['m'], 'help_text': 'Show/hide stream members (from stream information)', - 'excluded_from_random_tips': True, 'key_category': 'stream_list', 'key_contexts': ['stream_info'], }, @@ -385,7 +381,6 @@ class KeyBinding(TypedDict): 'keys': ['c'], 'help_text': 'Copy stream email to clipboard (from stream information)', - 'excluded_from_random_tips': True, 'key_category': 'stream_list', 'key_contexts': ['stream_info'], }, From 2e1b0f03de98403802f1e06af61377d831df9821 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Tue, 16 Jul 2024 10:30:48 +0530 Subject: [PATCH 30/34] ui: Enable footer texts to display contextual help tips. - Add a context property. - Update footer text on context changes, if a footer event is not already in progress. Tests added. --- tests/ui/test_ui.py | 18 ++++++++++++++++++ zulipterminal/ui.py | 16 +++++++++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/tests/ui/test_ui.py b/tests/ui/test_ui.py index c3b67ea13d..80adcfca3d 100644 --- a/tests/ui/test_ui.py +++ b/tests/ui/test_ui.py @@ -122,6 +122,24 @@ def test_set_footer_text_with_duration( assert view.controller.update_screen.call_count == 2 assert view._is_footer_event_running is False + @pytest.mark.parametrize( + "event_running, expected_call_count", [(True, 0), (False, 1)] + ) + def test_set_footer_text_on_context_change( + self, + view: View, + mocker: MockerFixture, + event_running: bool, + expected_call_count: int, + ) -> None: + mocker.patch(VIEW + ".get_random_help", return_value=["some help text"]) + view._is_footer_event_running = event_running + + view.set_footer_text_on_context_change() + + assert view.frame.footer.set_text.call_count == expected_call_count + assert view.controller.update_screen.call_count == expected_call_count + @pytest.mark.parametrize( "suggestions, state, truncated, footer_text", [ diff --git a/zulipterminal/ui.py b/zulipterminal/ui.py index 92dce7bf5f..3b3b6aa5e1 100644 --- a/zulipterminal/ui.py +++ b/zulipterminal/ui.py @@ -49,6 +49,7 @@ def __init__(self, controller: Any) -> None: self.write_box = WriteBox(self) self.search_box = MessageSearchBox(self.controller) self.stream_topic_map: Dict[int, str] = {} + self._context: str = "global" self._is_footer_event_running: bool = False self.message_view: Any = None @@ -103,7 +104,7 @@ def right_column_view(self) -> Any: def get_random_help(self) -> List[Any]: # Get random allowed hotkey (ie. eligible for being displayed as a tip) - allowed_commands = commands_for_random_tips() + allowed_commands = commands_for_random_tips(self.context) if not allowed_commands: return ["Help[?] "] random_command = random.choice(allowed_commands) @@ -143,6 +144,19 @@ def set_footer_text_for_event_duration( time.sleep(duration) self.reset_footer_text() + def set_footer_text_on_context_change(self) -> None: + if self._is_footer_event_running: + return + self.reset_footer_text() + + def _update_context(self, context_value: str = "") -> None: + if self._context == context_value: + return + self._context = context_value + self.set_footer_text_on_context_change() + + context = property(lambda self: self._context, _update_context) + @asynch def set_typeahead_footer( self, suggestions: List[str], state: Optional[int], is_truncated: bool From 1b6ad6bf125a69eca65af44bb97d22ad7ffd5b7d Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Thu, 11 Jul 2024 04:36:11 +0530 Subject: [PATCH 31/34] ui/keys: Display the context of the footer hint. Return a valid display context name from commands_for_random_tips(), and use in the footer. Tests added and updated. --- tests/config/test_keys.py | 20 +++++++++++--------- zulipterminal/config/keys.py | 10 ++++++---- zulipterminal/ui.py | 8 ++++++-- 3 files changed, 23 insertions(+), 15 deletions(-) diff --git a/tests/config/test_keys.py b/tests/config/test_keys.py index 813aaac993..534c349cef 100644 --- a/tests/config/test_keys.py +++ b/tests/config/test_keys.py @@ -67,19 +67,20 @@ def test_is_command_key_invalid_command(invalid_command: str) -> None: def test_HELP_is_not_allowed_as_tip() -> None: assert keys.KEY_BINDINGS["HELP"]["excluded_from_random_tips"] is True - assert keys.KEY_BINDINGS["HELP"] not in keys.commands_for_random_tips() + commands, _ = keys.commands_for_random_tips() + assert keys.KEY_BINDINGS["HELP"] not in commands @pytest.mark.parametrize( - "context, expected_command", + "context, expected_command, expected_context", [ - (None, "GAMMA"), - ("context_1", "BETA"), - ("context_2", "GAMMA"), + (None, "GAMMA", "Global"), + ("context_1", "BETA", "Context 1"), + ("context_2", "GAMMA", "Global"), ], ) def test_commands_for_random_tips( - context: str, expected_command: str, mocker: MockerFixture + context: str, expected_command: str, expected_context: str, mocker: MockerFixture ) -> None: new_key_bindings: Dict[str, keys.KeyBinding] = { "ALPHA": { @@ -117,9 +118,10 @@ def test_commands_for_random_tips( } mocker.patch.dict(keys.KEY_BINDINGS, new_key_bindings, clear=True) mocker.patch.object(keys, "HELP_CONTEXTS", new_help_contexts) - result = keys.commands_for_random_tips(context) - assert len(result) == 1 - assert new_key_bindings[expected_command] in result + commands, context_display_name = keys.commands_for_random_tips(context) + assert len(commands) == 1 + assert new_key_bindings[expected_command] in commands + assert context_display_name == expected_context def test_updated_urwid_command_map() -> None: diff --git a/zulipterminal/config/keys.py b/zulipterminal/config/keys.py index c946a97361..c1adb414e7 100644 --- a/zulipterminal/config/keys.py +++ b/zulipterminal/config/keys.py @@ -2,7 +2,7 @@ Keybindings and their helper functions """ -from typing import Dict, List +from typing import Dict, List, Tuple from typing_extensions import NotRequired, TypedDict from urwid.command_map import ( @@ -650,9 +650,10 @@ def primary_display_key_for_command(command: str) -> str: return display_key_for_urwid_key(primary_key_for_command(command)) -def commands_for_random_tips(context: str = "") -> List[KeyBinding]: +def commands_for_random_tips(context: str = "") -> Tuple[List[KeyBinding], str]: """ - Return list of commands which may be displayed as a random tip + Return list of commands which may be displayed as a random tip, + and their associated user-facing context name """ if not context or context not in HELP_CONTEXTS: context = "global" @@ -664,7 +665,8 @@ def commands_for_random_tips(context: str = "") -> List[KeyBinding]: ] if len(random_tips) == 0: return commands_for_random_tips("global") - return random_tips + context_display_name = HELP_CONTEXTS.get(context, "Global") + return random_tips, context_display_name # Refer urwid/command_map.py diff --git a/zulipterminal/ui.py b/zulipterminal/ui.py index 3b3b6aa5e1..2a89ad53a3 100644 --- a/zulipterminal/ui.py +++ b/zulipterminal/ui.py @@ -104,7 +104,7 @@ def right_column_view(self) -> Any: def get_random_help(self) -> List[Any]: # Get random allowed hotkey (ie. eligible for being displayed as a tip) - allowed_commands = commands_for_random_tips(self.context) + allowed_commands, tip_context = commands_for_random_tips(self.context) if not allowed_commands: return ["Help[?] "] random_command = random.choice(allowed_commands) @@ -114,7 +114,11 @@ def get_random_help(self) -> List[Any]: return [ "Help[?] ", ("footer_contrast", f" {random_command_display_keys} "), - f" {random_command['help_text']}", + f" ({tip_context}) ", + ( + "footer_contrast", + f" {random_command['help_text']} ", + ), ] @asynch From f75a309f4eaa41061eafae68f764ccda6fa6fe1e Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Thu, 11 Jul 2024 06:39:49 +0530 Subject: [PATCH 32/34] contexts/core/ui: Track the currently focused widget in the UI. using a customized urwid.MainLoop. Added a new file contexts.py that implements the MainLoop that will track the context through the focus path. Developer overview document regenerated. Test updated. --- docs/developer-file-overview.md | 1 + tests/core/test_core.py | 2 +- zulipterminal/contexts.py | 125 ++++++++++++++++++++++++++++++++ zulipterminal/core.py | 3 +- zulipterminal/ui.py | 5 +- 5 files changed, 133 insertions(+), 3 deletions(-) create mode 100644 zulipterminal/contexts.py diff --git a/docs/developer-file-overview.md b/docs/developer-file-overview.md index ef21a7c9fb..89887591c1 100644 --- a/docs/developer-file-overview.md +++ b/docs/developer-file-overview.md @@ -8,6 +8,7 @@ Zulip Terminal uses [Zulip's API](https://zulip.com/api/) to store and retrieve | Folder | File | Description | | ---------------------- | ------------------- | ----------------------------------------------------------------------------------------| | zulipterminal | api_types.py | Types from the Zulip API, translated into python, to improve type checking | +| | contexts.py | Tracks the currently focused widget in the UI | | | core.py | Defines the `Controller`, which sets up the `Model`, `View`, and how they interact | | | helper.py | Helper functions used in multiple places | | | model.py | Defines the `Model`, fetching and storing data retrieved from the Zulip server | diff --git a/tests/core/test_core.py b/tests/core/test_core.py index c320def285..896a264ee6 100644 --- a/tests/core/test_core.py +++ b/tests/core/test_core.py @@ -39,7 +39,7 @@ def controller(self, mocker: MockerFixture) -> Controller: self.poll_for_events = mocker.patch(MODEL + ".poll_for_events") mocker.patch(MODULE + ".Controller.show_loading") self.main_loop = mocker.patch( - MODULE + ".urwid.MainLoop", return_value=mocker.Mock() + MODULE + ".FocusTrackingMainLoop", return_value=mocker.Mock() ) self.config_file = "path/to/zuliprc" diff --git a/zulipterminal/contexts.py b/zulipterminal/contexts.py new file mode 100644 index 0000000000..449ae23d35 --- /dev/null +++ b/zulipterminal/contexts.py @@ -0,0 +1,125 @@ +""" +Tracks the currently focused widget in the UI +""" +import re +from typing import Dict, List, Optional, Tuple, Union + +import urwid + +from zulipterminal.config.themes import ThemeSpec + + +# These include contexts that do not have any help hints as well, +# for the sake of completeness +AUTOHIDE_PREFIXES: Dict[Tuple[Union[int, str], ...], str] = { + (1, 0, 0): "menu_button", + (1, 0, 1, "body"): "stream_topic_button", + (1, 0, 1, "header"): "left_panel_search_box", + (1, "body"): "message_box", + (1, "header"): "message_search_box", + (1, "footer"): "compose_box", + (1, 1, "header"): "user_search_box", + (1, 1, "body"): "user_button", +} + +NON_AUTOHIDE_PREFIXES: Dict[Tuple[Union[int, str], ...], str] = { + (0, 0): "menu_button", + (0, 1, "body"): "stream_topic_button", + (0, 1, "header"): "left_panel_search_box", + (1, "body"): "message_box", + (1, "header"): "message_search_box", + (1, "footer"): "compose_box", + (2, "header"): "user_search_box", + (2, "body"): "user_button", +} + + +class FocusTrackingMainLoop(urwid.MainLoop): + def __init__( + self, + widget: urwid.Widget, + palette: ThemeSpec, + screen: Optional[urwid.BaseScreen], + ) -> None: + super().__init__(widget, palette, screen) + self.previous_focus_path = None + self.view = widget + + def process_input(self, input: List[str]) -> None: + super().process_input(input) + self.track_focus_change() + + def track_focus_change(self) -> None: + focus_path = self.widget.get_focus_path() + if focus_path != self.previous_focus_path: + # Update view's context irrespective of the focused widget + self.view.context = self.get_context_name(focus_path) + + self.previous_focus_path = focus_path + + def get_context_name(self, focus_path: Tuple[Union[int, str]]) -> str: + widget_in_focus = self.get_widget_in_focus(focus_path) + + if self.widget != self.view: + overlay_widget_to_context_map = { + "msg_info_popup": "msg_info", + "stream_info_popup": "stream_info", + "emoji_list_popup": "emoji_list", + "about_popup": "about", + } + return overlay_widget_to_context_map.get(widget_in_focus, "popup") + + widget_suffix_to_context_map = { + "user_button": "user", + "message_box": "message", + "stream_topic_button": ( + "topic" if self.widget.left_panel.is_in_topic_view else "stream" + ), + "topic": "topic", + "compose_box": "compose_box", + "search_box": "editor", + "button": "button", + } + for suffix, context in widget_suffix_to_context_map.items(): + if widget_in_focus.endswith(suffix): + return context + + return "general" + + def get_widget_in_focus(self, focus_path: Tuple[Union[int, str], ...]) -> str: + if isinstance(self.widget, urwid.Overlay): + # NoticeView and PopUpConfirmationView do not shift focus path on opening, + # until the user presses a recognized key that doesn't close the popup. + if len(focus_path) > 2: + # View -> AttrMap -> Frame -> LineBox -> the named Popup + popup_widget = ( + self.widget.contents[1][0].original_widget + ).body.original_widget + popup_widget_class = popup_widget.__class__.__name__ + + if popup_widget_class == "EmojiPickerView": + popup_widget_class = ( + "EmojiSearchView" + if focus_path[2] == "header" + else "EmojiListView" + ) + + # PascalCase to snake_case + return re.sub( + r"view", + r"popup", + re.sub(r"(? Any: text_header = self.get_random_help() return urwid.AttrWrap(urwid.Text(text_header), "footer") + def get_focus_path(self) -> Tuple[Union[int, str], ...]: + return self.frame.get_focus_path() + def main_window(self) -> Any: self.left_panel, self.left_tab = self.left_column_view() self.center_panel = self.middle_column_view() From 58b1a965f69d8e80ccd0e779f76b103f3caa4244 Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Thu, 11 Jul 2024 02:13:28 +0530 Subject: [PATCH 33/34] core/ui/views/keys: Add a Contextual Help Menu. Assigned an arbitrary hotkey. Will be overloaded onto the HELP command. Help Menus do not currently work from within editors or popups. Hotkeys document regenerated. --- docs/hotkeys.md | 1 + zulipterminal/config/keys.py | 7 +++++++ zulipterminal/core.py | 4 ++-- zulipterminal/ui.py | 3 +++ zulipterminal/ui_tools/views.py | 7 ++++++- 5 files changed, 19 insertions(+), 3 deletions(-) diff --git a/docs/hotkeys.md b/docs/hotkeys.md index 86162271fd..056450a0ae 100644 --- a/docs/hotkeys.md +++ b/docs/hotkeys.md @@ -6,6 +6,7 @@ |Command|Key Combination| | :--- | :---: | |Show/hide Help Menu|?| +|Show/hide Contextual Help Menu|Meta + c| |Show/hide Markdown Help Menu|Meta + m| |Show/hide About Menu|Meta + ?| |Copy information from About Menu to clipboard|c| diff --git a/zulipterminal/config/keys.py b/zulipterminal/config/keys.py index c1adb414e7..15b0f6498b 100644 --- a/zulipterminal/config/keys.py +++ b/zulipterminal/config/keys.py @@ -38,6 +38,13 @@ class KeyBinding(TypedDict): 'key_category': 'general', 'key_contexts': ['general'], }, + 'CONTEXTUAL_HELP': { + 'keys': ['meta c'], + 'help_text': 'Show/hide Contextual Help Menu', + 'excluded_from_random_tips': True, + 'key_category': 'general', + 'key_contexts': ['general'], + }, 'MARKDOWN_HELP': { 'keys': ['meta m'], 'help_text': 'Show/hide Markdown Help Menu', diff --git a/zulipterminal/core.py b/zulipterminal/core.py index 9cd829676a..5649d33e19 100644 --- a/zulipterminal/core.py +++ b/zulipterminal/core.py @@ -252,8 +252,8 @@ def is_any_popup_open(self) -> bool: def exit_popup(self) -> None: self.loop.widget = self.view - def show_help(self) -> None: - help_view = HelpView(self, f"Help Menu {SCROLL_PROMPT}") + def show_help(self, context: Optional[str] = None) -> None: + help_view = HelpView(self, f"Help Menu {SCROLL_PROMPT}", context) self.show_pop_up(help_view, "area:help") def show_markdown_help(self) -> None: diff --git a/zulipterminal/ui.py b/zulipterminal/ui.py index c4a6ef2ee9..03277b15de 100644 --- a/zulipterminal/ui.py +++ b/zulipterminal/ui.py @@ -350,6 +350,9 @@ def keypress(self, size: urwid_Box, key: str) -> Optional[str]: # Show help menu self.controller.show_help() return key + elif is_command_key("CONTEXTUAL_HELP", key): + self.controller.show_help(self.context) + return key elif is_command_key("MARKDOWN_HELP", key): self.controller.show_markdown_help() return key diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index 6d01a82566..36091378ff 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -1238,13 +1238,16 @@ def _fetch_user_data( class HelpView(PopUpView): - def __init__(self, controller: Any, title: str) -> None: + def __init__( + self, controller: Any, title: str, context: Optional[str] = None + ) -> None: help_menu_content = [] for category in HELP_CATEGORIES: keys_in_category = ( binding for binding in KEY_BINDINGS.values() if binding["key_category"] == category + and (not context or context in binding["key_contexts"]) ) key_bindings = [ ( @@ -1254,6 +1257,8 @@ def __init__(self, controller: Any, title: str) -> None: for binding in keys_in_category ] + if not key_bindings: + continue help_menu_content.append((HELP_CATEGORIES[category], key_bindings)) popup_width, column_widths = self.calculate_table_widths( From 3513d73107a90f177bfd5a6ea5932b0815414bba Mon Sep 17 00:00:00 2001 From: Niloth-p <20315308+Niloth-p@users.noreply.github.com> Date: Tue, 16 Jul 2024 10:43:29 +0530 Subject: [PATCH 34/34] keys/views: Add mapping of contexts to broader contexts they belong to. Update the contextual help menu to use the parent contexts as well. Add linting for parent contexts mapping. --- tools/lint-hotkeys | 38 ++++++++++++++++++++++++++++++++- zulipterminal/config/keys.py | 18 ++++++++++++++++ zulipterminal/ui_tools/views.py | 8 ++++++- 3 files changed, 62 insertions(+), 2 deletions(-) diff --git a/tools/lint-hotkeys b/tools/lint-hotkeys index d7b7914b64..1005f33421 100755 --- a/tools/lint-hotkeys +++ b/tools/lint-hotkeys @@ -11,6 +11,7 @@ from zulipterminal.config.keys import ( HELP_CATEGORIES, HELP_CONTEXTS, KEY_BINDINGS, + PARENT_CONTEXTS, display_keys_for_command, ) @@ -90,7 +91,10 @@ def lint_hotkeys_file() -> None: error_flag = False error_flag |= ( - lint_help_groups("category") | lint_help_groups("context") | lint_help_text() + lint_help_groups("category") + | lint_help_groups("context") + | lint_help_text() + | lint_parent_contexts() ) if error_flag: @@ -182,6 +186,38 @@ def lint_help_groups(group: Group) -> bool: return error_flag +def lint_parent_contexts() -> bool: + """ + Lint for any typos in the PARENT_CONTEXTS dict + """ + key_typos = [] + value_typos = [] + + for key, value_list in PARENT_CONTEXTS.items(): + if key not in HELP_CONTEXTS: + key_typos.append(key) + for value in value_list: + if value not in HELP_CONTEXTS: + value_typos.append(value) + + error_message = "" + if key_typos: + error_message += ( + f"Invalid contexts in parent context keys: {', '.join(key_typos)}.\n" + ) + if value_typos: + error_message += ( + f"Invalid contexts in parent context values: {', '.join(value_typos)}.\n" + ) + + if error_message: + error_message += f" Choose a context from:\n{', '.join(HELP_CONTEXTS.keys())}\n" + print(error_message) + return True + + return False + + def generate_hotkeys_file(group: Group) -> None: """ Generate output file based on help text description and diff --git a/zulipterminal/config/keys.py b/zulipterminal/config/keys.py index 15b0f6498b..65cceb62fc 100644 --- a/zulipterminal/config/keys.py +++ b/zulipterminal/config/keys.py @@ -572,6 +572,24 @@ class KeyBinding(TypedDict): "search_box": "Search box", } +PARENT_CONTEXTS: Dict[str, List[str]] = { + "global": [], + "general": ["global"], + "editor": ["global"], + "compose_box": ["editor", "global"], + "stream": ["general", "global", "button"], + "topic": ["general", "global", "button"], + "user": ["general", "global", "button"], + "message": ["general", "global"], + "stream_info": ["global", "popup"], + "msg_info": ["global", "popup"], + "emoji_list": ["global", "popup"], + "about": ["global", "popup"], + "search_box": ["global", "editor"], + "popup": ["global"], + "button": ["global"], +} + ZT_TO_URWID_CMD_MAPPING = { "GO_UP": CURSOR_UP, "GO_DOWN": CURSOR_DOWN, diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py index 36091378ff..a15a4c8fd5 100644 --- a/zulipterminal/ui_tools/views.py +++ b/zulipterminal/ui_tools/views.py @@ -14,6 +14,7 @@ from zulipterminal.config.keys import ( HELP_CATEGORIES, KEY_BINDINGS, + PARENT_CONTEXTS, display_key_for_urwid_key, display_keys_for_command, is_command_key, @@ -1242,12 +1243,17 @@ def __init__( self, controller: Any, title: str, context: Optional[str] = None ) -> None: help_menu_content = [] + if context: + valid_contexts = PARENT_CONTEXTS[context] + [context] for category in HELP_CATEGORIES: keys_in_category = ( binding for binding in KEY_BINDINGS.values() if binding["key_category"] == category - and (not context or context in binding["key_contexts"]) + and ( + not context + or bool(set(binding["key_contexts"]) & set(valid_contexts)) + ) ) key_bindings = [ (