Repository: zulip/zulip-terminal
Pull request #1398 (2023-05-10 00:53:21)

Document & extract additional application symbols
<!-- See README for documentation, or ask in #zulip-terminal if unclear -->
### What does this PR do, and why?

Currently symbols.py only shows the symbols as strings, which makes them challenging to look up via unicode code point or name; the first commit aims to resolve this.

Subsequent commits:
- extract other symbols which were not yet centralized in symbols.py (including some in dicts)
- adds support for symbols in dicts to render-symbols.py, ie. `zulip-term-check-symbols`
- converts code that uses LineBox to make a line underneath/above to use a Divider (some with a Pile)
- remove unnecessarily specified parameters to LineBox
- creates a TitledColumn class for the bold-titled columns which are used in multiple places

Other than centralizing more symbols, this reduces duplication, verbosity and number of lines quite substantially.

Tested visually, in regular and autohide mode. Most is fairly straightforward refactoring, so is low risk.

### Follow-up    <!-- DELETE SECTION IF EMPTY -->
<!-- In what ways is this not fully implemented/functioning? Compared to a discussion/issue? -->
<!-- Do you not understand something? Are you unsure about a certain approach? Want feedback? -->
- [ ] A follow-up commit (not in this branch) explores moving the column dividers into the Columns of the application, but this currently poses challenges with column focus, so not in this PR
- [ ] Updating the tests for this, again indicates how tightly coupled some UI tests are
- [ ] Find any other symbols which are not in symbols.py

### External discussion & connections
<!-- [x] all that apply, specifying topic and adding numbers after # for issues/PRs -->
- [x] Discussed in **#zulip-terminal** in `Having symbol issues?`
- [ ] Fully fixes #
- [ ] Partially fixes issue #
- [ ] Builds upon previous unmerged work in PR #
- [ ] Is a follow-up to work in PR #
- [ ] Requires merge of PR #
- [ ] Merge will enable work on #

### How did you test this?
<!-- [x] all that apply -->
- [ ] Manually - Behavioral changes
- [x] Manually - Visual changes
- [x] Adapting existing automated tests
- [ ] Adding automated tests for new behavior (or missing tests)
- [x] Existing automated tests should already cover this (*only a refactor of tested code*)

### Self-review checklist for each commit
- [x] It is a [minimal coherent idea](https://github.com/zulip/zulip-terminal#structuring-commits---speeding-up-reviews-merging--development)
- [x] It has a commit summary following the [documented style](https://github.com/zulip/zulip-terminal#structuring-commits---speeding-up-reviews-merging--development) (title & body)
- [x] It has a commit summary describing the  motivation and reasoning for the change
- [x] It individually passes linting and tests
- [ ] It contains test additions for any new behavior
- [x] It flows clearly from a previous branch commit, and/or prepares for the next commit

### Visual changes    <!-- DELETE SECTION IF NO VISUAL CHANGE -->
Ideally none!

zulipbot: Hello @zulip/server-refactoring members, this pull request was labeled with the "area: refactoring" label, so you may want to check it out!

<!-- areaLabelAddition -->

neiljp: Dropped previous last commit for now due to a potential focus issue; rest to merge shortly.

Patch:
diff --git a/zulipterminal/config/symbols.py b/zulipterminal/config/symbols.py
index 2b49e8e..4782f4c 100644
--- a/zulipterminal/config/symbols.py
+++ b/zulipterminal/config/symbols.py
@@ -2,27 +2,79 @@
 Terminal characters used to mark particular elements of the user interface
 """
 
-INVALID_MARKER = "✗"
+# Unless otherwise noted, all symbols are in:
+# - Basic Multilingual Plane (BMP)
+# - Unicode v1.1
+# Suffix comments indicate: unicode name, codepoint (unicode block, version if not v1.1)
+
+INVALID_MARKER = "✗"  # BALLOT X, U+2717 (Dingbats)
 STREAM_MARKER_PRIVATE = "P"
 STREAM_MARKER_PUBLIC = "#"
-STREAM_MARKER_WEB_PUBLIC = "⊚"
-STREAM_TOPIC_SEPARATOR = "▶"
-# Used as a separator between messages and 'EDITED'
-MESSAGE_CONTENT_MARKER = "▒"  # Options are '█', '▓', '▒', '░'
-QUOTED_TEXT_MARKER = "░"
-MESSAGE_HEADER_DIVIDER = "━"
-CHECK_MARK = "✓"
-APPLICATION_TITLE_BAR_LINE = "═"
-PINNED_STREAMS_DIVIDER = "-"
-COLUMN_TITLE_BAR_LINE = "━"
+STREAM_MARKER_WEB_PUBLIC = "⊚"  # CIRCLED RING OPERATOR, U+229A (Mathematical operators)
+STREAM_TOPIC_SEPARATOR = "▶"  # BLACK RIGHT-POINTING TRIANGLE, U+25B6 (Geometric shapes)
+
+# Range of block options for consideration: '█', '▓', '▒', '░'
+# FULL BLOCK U+2588, DARK SHADE U+2593, MEDIUM SHADE U+2592, LIGHT SHADE U+2591
+
+# Separator between messages and 'EDITED'
+MESSAGE_CONTENT_MARKER = "▒"  # MEDIUM SHADE, U+2592 (Block elements)
+
+QUOTED_TEXT_MARKER = "░"  # LIGHT SHADE, U+2591 (Block elements)
+
+# Extends from end of recipient details (above messages where recipients differ above)
+MESSAGE_HEADER_DIVIDER = "━"  # BOX DRAWINGS HEAVY HORIZONTAL, U+2501 (Box drawing)
+
+# NOTE: CHECK_MARK is not used for resolved topics (that is an API detail)
+CHECK_MARK = "✓"  # CHECK MARK, U+2713 (Dingbats)
+
+APPLICATION_TITLE_BAR_LINE = "═"  # BOX DRAWINGS DOUBLE HORIZONTAL, U+2550 (Box drawing)
+PINNED_STREAMS_DIVIDER = "-"  # HYPHEN-MINUS, U+002D (Basic latin)
+COLUMN_TITLE_BAR_LINE = "━"  # BOX DRAWINGS HEAVY HORIZONTAL, U+2501 (Box drawing)
+COLUMN_DIVIDER_LINE = "│"  # BOX DRAWINGS LIGHT VERTICAL, U+2502 (Box drawing)
+SECTION_DIVIDER_LINE = "─"  # BOX DRAWINGS LIGHT HORIZONTAL, U+2500 (Box drawing)
+
 # NOTE: The '⏱' emoji needs an extra space while rendering. Otherwise, it
 # appears to overlap its subsequent text.
-TIME_MENTION_MARKER = "⏱ "  # Other tested options are: '⧗' and '⧖'.
+# Other tested options are: '⧗' and '⧖'.
+# TODO: Try 25F7, WHITE CIRCLE WITH UPPER RIGHT QUADRANT?
+TIME_MENTION_MARKER = "⏱ "  # STOPWATCH, U+23F1 (Misc Technical, Unicode 6.0)
+
 MUTE_MARKER = "M"
-STATUS_ACTIVE = "●"
-STATUS_IDLE = "◒"
-STATUS_OFFLINE = "○"
-STATUS_INACTIVE = "•"
-BOT_MARKER = "♟"
-AUTOHIDE_TAB_LEFT_ARROW = "❰"
-AUTOHIDE_TAB_RIGHT_ARROW = "❱"
+STATUS_ACTIVE = "●"  # BLACK CIRCLE, U+25CF (Geometric shapes)
+STATUS_IDLE = "◒"  # CIRCLE WITH LOWER HALF BLACK, U+25D2 (Geometric shapes)
+STATUS_OFFLINE = "○"  # WHITE CIRCLE, U+25CB (Geometric shapes)
+STATUS_INACTIVE = "•"  # BULLET, U+2022 (General punctuation)
+BOT_MARKER = "♟"  # BLACK CHESS PAWN, U+265F (Misc symbols)
+
+# Unicode 3.2:
+AUTOHIDE_TAB_LEFT_ARROW = "❰"  # HEAVY LEFT-POINTING ANGLE BRACKET ORNAMENT, U+2770
+AUTOHIDE_TAB_RIGHT_ARROW = "❱"  # HEAVY RIGHT-POINTING ANGLE BRACKET ORNAMENT, U+2771
+
+# All in Block elements:
+POPUP_TOP_LINE = "▄"  # LOWER HALF BLOCK, U+2584
+POPUP_CONTENT_BORDER = dict(
+    tlcorner="▛",  # QUADRANT UPPER LEFT AND UPPER RIGHT AND LOWER LEFT, U+259B (v3.2)
+    tline="▀",  # UPPER HALF BLOCK, U+2580
+    trcorner="▜",  # QUADRANT UPPER LEFT AND UPPER RIGHT AND LOWER RIGHT, U+259C (v3.2)
+    rline="▐",  # RIGHT HALF BLOCK, U+2590
+    lline="▌",  # LEFT HALF BLOCK, U+258C
+    blcorner="▙",  # QUADRANT UPPER LEFT AND LOWER LEFT AND LOWER RIGHT, U+2599 (v3.2)
+    bline="▄",  # LOWER HALF BLOCK, U+2584
+    brcorner="▟",  # QUADRANT UPPER RIGHT AND LOWER LEFT AND LOWER RIGHT, U+259F (v3.2)
+)
+
+COMPOSE_HEADER_TOP = "━"  # BOX DRAWINGS HEAVY HORIZONTAL, U+2501 (Box drawing)
+COMPOSE_HEADER_BOTTOM = "─"  # BOX DRAWINGS LIGHT HORIZONTAL, U+2500 (Box drawing)
+
+_MESSAGE_RECIPIENTS_TOP = "─"  # BOX DRAWINGS LIGHT HORIZONTAL, U+2500 (Box drawing)
+_MESSAGE_RECIPIENTS_BOTTOM = _MESSAGE_RECIPIENTS_TOP
+MESSAGE_RECIPIENTS_BORDER = dict(
+    tline=_MESSAGE_RECIPIENTS_TOP,
+    lline="",
+    trcorner=_MESSAGE_RECIPIENTS_TOP,
+    tlcorner=_MESSAGE_RECIPIENTS_TOP,
+    blcorner=_MESSAGE_RECIPIENTS_BOTTOM,
+    rline="",
+    bline=_MESSAGE_RECIPIENTS_BOTTOM,
+    brcorner=_MESSAGE_RECIPIENTS_BOTTOM,
+)
diff --git a/zulipterminal/core.py b/zulipterminal/core.py
index f1f32e4..572efd1 100644
--- a/zulipterminal/core.py
+++ b/zulipterminal/core.py
@@ -19,6 +19,7 @@ import zulip
 from typing_extensions import Literal
 
 from zulipterminal.api_types import Composition, Message
+from zulipterminal.config.symbols import POPUP_CONTENT_BORDER, POPUP_TOP_LINE
 from zulipterminal.config.themes import ThemeSpec
 from zulipterminal.config.ui_sizes import (
     MAX_LINEAR_SCALING_WIDTH,
@@ -220,32 +221,14 @@ class Controller:
         return max_popup_cols, max_popup_rows
 
     def show_pop_up(self, to_show: Any, style: str) -> None:
-        border_lines = dict(
-            tlcorner="▛",
-            tline="▀",
-            trcorner="▜",
-            rline="▐",
-            lline="▌",
-            blcorner="▙",
-            bline="▄",
-            brcorner="▟",
-        )
         text = urwid.Text(to_show.title, align="center")
         title_map = urwid.AttrMap(urwid.Filler(text), style)
         title_box_adapter = urwid.BoxAdapter(title_map, height=1)
-        title_box = urwid.LineBox(
-            title_box_adapter,
-            tlcorner="▄",
-            tline="▄",
-            trcorner="▄",
-            rline="",
-            lline="",
-            blcorner="",
-            bline="",
-            brcorner="",
-        )
-        title = urwid.AttrMap(title_box, "popup_border")
-        content = urwid.LineBox(to_show, **border_lines)
+        title_top = urwid.AttrMap(urwid.Divider(POPUP_TOP_LINE), "popup_border")
+        title = urwid.Pile([title_top, title_box_adapter])
+
+        content = urwid.LineBox(to_show, **POPUP_CONTENT_BORDER)
+
         self.loop.widget = urwid.Overlay(
             urwid.AttrMap(urwid.Frame(header=title, body=content), "popup_border"),
             self.view,
diff --git a/zulipterminal/scripts/render_symbols.py b/zulipterminal/scripts/render_symbols.py
index e0e80b3..f4a3850 100755
--- a/zulipterminal/scripts/render_symbols.py
+++ b/zulipterminal/scripts/render_symbols.py
@@ -13,11 +13,24 @@ palette = [
     ("footer", "black", "white"),
 ]
 
-symbol_dict = {
+initial_symbol_dict = {
     name: symbol
     for name, symbol in vars(symbols).items()
     if not name.startswith("__") and not name.endswith("__")
 }
+nested_dict = {
+    f"{name}__{subname}": subsymbol
+    for name, symbol in initial_symbol_dict.items()
+    if isinstance(symbol, dict)
+    for subname, subsymbol in symbol.items()
+}
+symbol_dict = {
+    name: symbol
+    for name, symbol in initial_symbol_dict.items()
+    if not isinstance(symbol, dict)
+}
+symbol_dict.update(nested_dict)
+
 max_symbol_name_length = max([len(name) for name in symbol_dict])
 
 symbol_names_list = [urwid.Text(name, align="center") for name in symbol_dict]
diff --git a/zulipterminal/ui.py b/zulipterminal/ui.py
index fa14063..a8ffdf3 100644
--- a/zulipterminal/ui.py
+++ b/zulipterminal/ui.py
@@ -14,6 +14,7 @@ from zulipterminal.config.symbols import (
     APPLICATION_TITLE_BAR_LINE,
     AUTOHIDE_TAB_LEFT_ARROW,
     AUTOHIDE_TAB_RIGHT_ARROW,
+    COLUMN_DIVIDER_LINE,
     COLUMN_TITLE_BAR_LINE,
 )
 from zulipterminal.config.ui_sizes import LEFT_WIDTH, RIGHT_WIDTH, TAB_WIDTH
@@ -66,8 +67,8 @@ class View(urwid.WidgetWrap):
             title_attr="column_title",
             tline=COLUMN_TITLE_BAR_LINE,
             bline="",
-            trcorner="│",
-            tlcorner="│",
+            trcorner=COLUMN_DIVIDER_LINE,
+            tlcorner=COLUMN_DIVIDER_LINE,
         )
 
     def right_column_view(self) -> Any:
@@ -81,7 +82,7 @@ class View(urwid.WidgetWrap):
             tline=COLUMN_TITLE_BAR_LINE,
             trcorner=COLUMN_TITLE_BAR_LINE,
             lline="",
-            blcorner="─",
+            blcorner="",
             rline="",
             bline="",
             brcorner="",
diff --git a/zulipterminal/ui_tools/boxes.py b/zulipterminal/ui_tools/boxes.py
index d429828..36128f4 100644
--- a/zulipterminal/ui_tools/boxes.py
+++ b/zulipterminal/ui_tools/boxes.py
@@ -32,7 +32,13 @@ from zulipterminal.config.regexes import (
     REGEX_STREAM_AND_TOPIC_FENCED_HALF,
     REGEX_STREAM_AND_TOPIC_UNFENCED,
 )
-from zulipterminal.config.symbols import INVALID_MARKER, STREAM_TOPIC_SEPARATOR
+from zulipterminal.config.symbols import (
+    COMPOSE_HEADER_BOTTOM,
+    COMPOSE_HEADER_TOP,
+    INVALID_MARKER,
+    MESSAGE_RECIPIENTS_BORDER,
+    STREAM_TOPIC_SEPARATOR,
+)
 from zulipterminal.config.ui_mappings import STREAM_ACCESS_TYPE
 from zulipterminal.helper import (
     asynch,
@@ -218,16 +224,12 @@ class WriteBox(urwid.Pile):
         self.msg_write_box.set_completer_delims(DELIMS_MESSAGE_COMPOSE)
 
         self.header_write_box = urwid.Columns([self.to_write_box])
-        header_line_box = urwid.LineBox(
-            self.header_write_box,
-            tlcorner="━",
-            tline="━",
-            trcorner="━",
-            lline="",
-            blcorner="─",
-            bline="─",
-            brcorner="─",
-            rline="",
+        header_line_box = urwid.Pile(
+            [
+                urwid.Divider(COMPOSE_HEADER_TOP),
+                self.header_write_box,
+                urwid.Divider(COMPOSE_HEADER_BOTTOM),
+            ]
         )
         self.contents = [
             (header_line_box, self.options()),
@@ -354,16 +356,12 @@ class WriteBox(urwid.Pile):
             ],
             dividechars=1,
         )
-        header_line_box = urwid.LineBox(
-            self.header_write_box,
-            tlcorner="━",
-            tline="━",
-            trcorner="━",
-            lline="",
-            blcorner="─",
-            bline="─",
-            brcorner="─",
-            rline="",
+        header_line_box = urwid.Pile(
+            [
+                urwid.Divider(COMPOSE_HEADER_TOP),
+                self.header_write_box,
+                urwid.Divider(COMPOSE_HEADER_BOTTOM),
+            ]
         )
         write_box = [
             (header_line_box, self.options()),
@@ -938,14 +936,7 @@ class MessageSearchBox(urwid.Pile):
         self.recipient_bar = urwid.LineBox(
             self.msg_narrow,
             title="Current message recipients",
-            tline="─",
-            lline="",
-            trcorner="─",
-            tlcorner="─",
-            blcorner="─",
-            rline="",
-            bline="─",
-            brcorner="─",
+            **MESSAGE_RECIPIENTS_BORDER,
         )
         return [self.search_bar, self.recipient_bar]
 
diff --git a/zulipterminal/ui_tools/views.py b/zulipterminal/ui_tools/views.py
index 2c2f383..e27500a 100644
--- a/zulipterminal/ui_tools/views.py
+++ b/zulipterminal/ui_tools/views.py
@@ -23,6 +23,7 @@ from zulipterminal.config.symbols import (
     CHECK_MARK,
     COLUMN_TITLE_BAR_LINE,
     PINNED_STREAMS_DIVIDER,
+    SECTION_DIVIDER_LINE,
 )
 from zulipterminal.config.ui_mappings import (
     BOT_TYPE_BY_ID,
@@ -319,16 +320,8 @@ class StreamsView(urwid.Frame):
         )
         super().__init__(
             list_box,
-            header=urwid.LineBox(
-                self.stream_search_box,
-                tlcorner="─",
-                tline="",
-                lline="",
-                trcorner="─",
-                blcorner="─",
-                rline="",
-                bline="─",
-                brcorner="─",
+            header=urwid.Pile(
+                [self.stream_search_box, urwid.Divider(SECTION_DIVIDER_LINE)]
             ),
         )
         self.search_lock = threading.Lock()
@@ -420,21 +413,16 @@ class TopicsView(urwid.Frame):
             self, "SEARCH_TOPICS", self.update_topics
         )
         self.header_list = urwid.Pile(
-            [self.stream_button, urwid.Divider("─"), self.topic_search_box]
+            [
+                self.stream_button,
+                urwid.Divider(SECTION_DIVIDER_LINE),
+                self.topic_search_box,
+                urwid.Divider(SECTION_DIVIDER_LINE),
+            ]
         )
         super().__init__(
             self.list_box,
-            header=urwid.LineBox(
-                self.header_list,
-                tlcorner="─",
-                tline="",
-                lline="",
-                trcorner="─",
-                blcorner="─",
-                rline="",
-                bline="─",
-                brcorner="─",
-            ),
+            header=self.header_list,
         )
         self.search_lock = threading.Lock()
         self.empty_search = False
@@ -638,17 +626,8 @@ class RightColumnView(urwid.Frame):
         self.view = view
         self.user_search = PanelSearchBox(self, "SEARCH_PEOPLE", self.update_user_list)
         self.view.user_search = self.user_search
-        search_box = urwid.LineBox(
-            self.user_search,
-            tlcorner="─",
-            tline="",
-            lline="",
-            trcorner="─",
-            blcorner="─",
-            rline="",
-            bline="─",
-            brcorner="─",
-        )
+        search_box = urwid.Pile([self.user_search, urwid.Divider(SECTION_DIVIDER_LINE)])
+
         self.allow_update_user_list = True
         self.search_lock = threading.Lock()
         self.empty_search = False
@@ -842,7 +821,7 @@ class LeftColumnView(urwid.Pile):
             rline="",
             lline="",
             bline="",
-            brcorner="─",
+            brcorner="",
         )
         return w
 
@@ -874,7 +853,7 @@ class LeftColumnView(urwid.Pile):
             rline="",
             lline="",
             bline="",
-            brcorner="─",
+            brcorner="",
         )
         return w
 
@@ -1939,16 +1918,8 @@ class EmojiPickerView(PopUpView):
         self.emoji_search = PanelSearchBox(
             self, "SEARCH_EMOJIS", self.update_emoji_list
         )
-        search_box = urwid.LineBox(
-            self.emoji_search,
-            tlcorner="─",
-            tline="",
-            lline="",
-            trcorner="─",
-            blcorner="─",
-            rline="",
-            bline="─",
-            brcorner="─",
+        search_box = urwid.Pile(
+            [self.emoji_search, urwid.Divider(SECTION_DIVIDER_LINE)]
         )
         self.empty_search = False
         self.search_lock = threading.Lock()


Test patch:
diff --git a/tests/ui/test_ui_tools.py b/tests/ui/test_ui_tools.py
index 03d56f7..c04c9c3 100644
--- a/tests/ui/test_ui_tools.py
+++ b/tests/ui/test_ui_tools.py
@@ -606,7 +606,12 @@ class TestTopicsView:
             topic_view, "SEARCH_TOPICS", topic_view.update_topics
         )
         self.header_list.assert_called_once_with(
-            [topic_view.stream_button, self.divider("─"), topic_view.topic_search_box]
+            [
+                topic_view.stream_button,
+                self.divider("─"),
+                topic_view.topic_search_box,
+                self.divider("─"),
+            ]
         )
 
     @pytest.mark.parametrize(
@@ -953,7 +958,7 @@ class TestRightColumnView:
         self.view = mocker.Mock()
         self.user_search = mocker.patch(VIEWS + ".PanelSearchBox")
         self.connect_signal = mocker.patch(VIEWS + ".urwid.connect_signal")
-        self.line_box = mocker.patch(VIEWS + ".urwid.LineBox")
+        self.pile = mocker.patch(VIEWS + ".urwid.Pile")
         self.thread = mocker.patch(VIEWS + ".threading")
         self.super = mocker.patch(VIEWS + ".urwid.Frame.__init__")
         self.view.model.unread_counts = {  # Minimal, though an UnreadCounts
@@ -976,7 +981,7 @@ class TestRightColumnView:
         assert right_col_view.search_lock == self.thread.Lock()
         self.super.assert_called_once_with(
             right_col_view.users_view(),
-            header=self.line_box(right_col_view.user_search),
+            header=self.pile(right_col_view.user_search),
         )
 
     def test_update_user_list_editor_mode(self, mocker, right_col_view):