tabbar: address vertical alignment feedback

Author: bvolpato

kitty/options/definition.py

@@ -1679,12 +1679,12 @@
 are automatically restricted to work only on matching tabs.
 ''')
 
-opt('tab_bar_align', 'left',
-    choices=('left', 'center', 'right'),
+opt('tab_bar_align', 'start',
+    choices=('start', 'center', 'end', 'left', 'right'),
     long_text='''
-The horizontal alignment of the tab bar. For vertical tab bars this controls the
-alignment of each tab title within the sidebar. Can be one of: :code:`left`,
-:code:`center`, :code:`right`.
+The alignment of the tab bar, can be one of: :code:`start`, :code:`center`,
+:code:`end`, :code:`left`, :code:`right`. The values :code:`left` and
+:code:`right` are aliases for :code:`start` and :code:`end` respectively.
 '''
     )
 

kitty/options/parse.py

@@ -103,6 +103,7 @@ def as_rgb(x: int) -> int:
 
 
 VERTICAL_EDGES = frozenset({LEFT_EDGE, RIGHT_EDGE})
+MAX_VERTICAL_TAB_LINES = 2
 
 
 def is_vertical_edge(edge: int) -> bool:
@@ -118,6 +119,14 @@ def edge_name(edge: int) -> EdgeLiteral:
     }.get(edge, 'bottom')
 
 
+def normalized_tab_bar_align(align: str) -> str:
+    if align == 'left':
+        return 'start'
+    if align == 'right':
+        return 'end'
+    return align
+
+
 @lru_cache
 def report_template_failure(template: str, e: str) -> None:
     log_error(f'Invalid tab title template: "{template}" with error: {e}')
@@ -657,15 +666,16 @@ def apply_options(self) -> None:
             self.draw_func = load_custom_draw_tab()
         else:
             self.draw_func = draw_tab_with_fade
-        if opts.tab_bar_align == 'center':
+        self.tab_bar_align = normalized_tab_bar_align(opts.tab_bar_align)
+        if self.tab_bar_align == 'center':
             self.align_factor = 2
-        elif opts.tab_bar_align == 'right':
+        elif self.tab_bar_align == 'end':
             self.align_factor = 1
         else:
             self.align_factor = 0
-        if opts.tab_bar_align == 'center':
+        if self.tab_bar_align == 'center':
             self.align: Callable[[], None] = partial(self.align_with_factor, 2)
-        elif opts.tab_bar_align == 'right':
+        elif self.tab_bar_align == 'end':
             self.align = self.align_with_factor
         else:
             self.align = lambda: None
@@ -886,23 +896,33 @@ def update_vertical(self, data: Sequence[TabBarData]) -> None:
         if not data:
             return
         max_tab_length = max(1, s.columns - 1)
-        rows_to_draw = min(len(data), s.lines)
-        draw_ellipsis = len(data) > s.lines and s.lines > 1
+        tab_line_height = max(1, min(MAX_VERTICAL_TAB_LINES, s.lines // max(1, len(data))))
+        rows_to_draw = min(len(data), max(1, s.lines // tab_line_height))
+        draw_ellipsis = len(data) > rows_to_draw and s.lines > 1
         if draw_ellipsis:
+            tab_line_height = 1
+            rows_to_draw = min(len(data), s.lines)
             rows_to_draw -= 1
+        total_lines = rows_to_draw * tab_line_height + int(draw_ellipsis)
+        if self.tab_bar_align == 'center':
+            start_row = max(0, (s.lines - total_lines) // 2)
+        elif self.tab_bar_align == 'end':
+            start_row = max(0, s.lines - total_lines)
+        else:
+            start_row = 0
         cr: list[TabExtent] = []
         for i, t in enumerate(data[:rows_to_draw]):
             s.cursor.x = 0
-            s.cursor.y = i
+            row = start_row + i * tab_line_height
+            s.cursor.y = row
             s.cursor.bg = as_rgb(self.draw_data.tab_bg(t))
             s.cursor.fg = as_rgb(self.draw_data.tab_fg(t))
             s.cursor.bold, s.cursor.italic = self.active_font_style if t.is_active else self.inactive_font_style
-            end = self.draw_func(self.draw_data, s, t, 0, max_tab_length, i + 1, True, ExtraData())
-            self.align_row(i, end)
-            cr.append(TabExtent(tab_id=t.tab_id, x=CellRange(0, s.columns - 1), y=CellRange(i, i)))
+            self.draw_func(self.draw_data, s, t, 0, max_tab_length, i + 1, True, ExtraData())
+            cr.append(TabExtent(tab_id=t.tab_id, x=CellRange(0, s.columns - 1), y=CellRange(row, min(s.lines - 1, row + tab_line_height - 1))))
         if draw_ellipsis:
             s.cursor.x = 0
-            s.cursor.y = s.lines - 1
+            s.cursor.y = start_row + rows_to_draw * tab_line_height
             s.cursor.bg = as_rgb(color_as_int(self.draw_data.default_bg))
             s.cursor.fg = as_rgb(0xff0000)
             s.draw('…')
@@ -918,16 +938,6 @@ def align_with_factor(self, factor: int = 1) -> None:
             self.screen.insert_characters(shift)
             self.tab_extents = tuple(te.shifted(x=shift) for te in self.tab_extents)
 
-    def align_row(self, row: int, end: int) -> None:
-        if not self.align_factor:
-            return
-        if end < self.screen.columns - 1:
-            shift = (self.screen.columns - end) // self.align_factor
-            if shift > 0:
-                self.screen.cursor.y = row
-                self.screen.cursor.x = 0
-                self.screen.insert_characters(shift)
-
     def destroy(self) -> None:
         self.screen.reset_callbacks()
         del self.screen

kitty/options/to-c-generated.h

@@ -259,6 +259,14 @@ def keys_for_func(opts, name):
     self.ae(opts.tab_bar_edge, LEFT_EDGE)
     opts = p('tab_bar_edge right')
     self.ae(opts.tab_bar_edge, RIGHT_EDGE)
+    opts = p('tab_bar_align start')
+    self.ae(opts.tab_bar_align, 'start')
+    opts = p('tab_bar_align end')
+    self.ae(opts.tab_bar_align, 'end')
+    opts = p('tab_bar_align left')
+    self.ae(opts.tab_bar_align, 'left')
+    opts = p('tab_bar_align right')
+    self.ae(opts.tab_bar_align, 'right')
     opts = p('clear_all_shortcuts y', 'map f1 next_window')
     self.ae(len(opts.keyboard_modes[''].keymap), 1)
     opts = p('clear_all_mouse_actions y', 'mouse_map left click ungrabbed mouse_click_url_or_select')

kitty/options/types.py

@@ -52,7 +52,38 @@ def test_vertical_tab_bar_hit_testing(self) -> None:
         self.ae(geometries[-1], (0, 0, 120, 160))
         self.ae(tb.drag_axis_coordinate(5, 35), 35)
         self.ae(tb.tab_id_at(5, 10), 1)
-        self.ae(tb.tab_id_at(110, 35), 2)
-        self.ae(tb.tab_id_at(60, 55), 3)
-        self.ae(tb.tab_id_at(60, 95), 0)
+        self.ae(tb.tab_id_at(110, 35), 1)
+        self.ae(tb.tab_id_at(60, 55), 2)
+        self.ae(tb.tab_id_at(60, 95), 3)
+        self.ae(tb.tab_id_at(60, 135), 0)
         self.ae(tb.tab_id_at(180, 10), 0)
+
+    def test_vertical_tab_bar_alignment(self) -> None:
+        self.set_options({
+            'tab_bar_align': 'end',
+            'tab_bar_edge': LEFT_EDGE,
+            'tab_bar_style': 'separator',
+            'tab_title_template': '{title}',
+        })
+        central = region(120, 0, 400, 160)
+        tab_bar = region(0, 0, 120, 160)
+        boss = DummyBoss()
+
+        with (
+            patch('kitty.tab_bar.cell_size_for_window', return_value=(10, 20)),
+            patch('kitty.tab_bar.viewport_for_window', return_value=(central, tab_bar, 400, 160, 10, 20)),
+            patch('kitty.tab_bar.set_tab_bar_render_data'),
+            patch('kitty.tab_bar.get_boss', return_value=boss),
+        ):
+            tb = TabBar(1)
+            tb.layout()
+            tb.update((
+                TabBarData(title='one', tab_id=1, is_active=True),
+                TabBarData(title='two', tab_id=2),
+            ))
+
+        self.ae(tb.tab_extents[0].y, (4, 5))
+        self.ae(tb.tab_extents[1].y, (6, 7))
+        self.ae(tb.tab_id_at(5, 10), 0)
+        self.ae(tb.tab_id_at(5, 110), 1)
+        self.ae(tb.tab_id_at(5, 150), 2)