Merge pull request #3226 from actionless/recompute-menubar-widths-again-lol

fix(menubar: init: get_current_page): list_spacing depends on presence of icon; also take cursor block width into consideration

lib/menubar/init.lua

@@ -70,12 +70,15 @@ end
 -- @beautiful beautiful.menubar_bg_focus
 -- @param color
 
+--- Menubar font.
+-- @beautiful beautiful.menubar_font
+-- @param[opt=beautiful.font] font
+
 
 -- menubar
 local menubar = { menu_entries = {} }
 menubar.menu_gen = require("menubar.menu_gen")
 menubar.utils = require("menubar.utils")
-local compute_text_width = menubar.utils.compute_text_width
 
 -- Options section
 
@@ -119,10 +122,9 @@ menubar.right_label = "▶▶"
 -- @tfield[opt="◀◀"] string left_label
 menubar.left_label = "◀◀"
 
--- awful.widget.common.list_update adds three times a margin of dpi(4)
+-- awful.widget.common.list_update adds spacing of dpi(4) between items.
--- for each item:
+-- @tfield number list_spacing
--- @tfield number list_interspace
+local list_spacing = theme.xresources.apply_dpi(4)
-local list_interspace = theme.xresources.apply_dpi(4) * 3
 
 --- Allows user to specify custom parameters for prompt.run function
 -- (like colors). This will merge with the default parameters, overriding affected values.
@@ -149,7 +151,7 @@ end
 
 --- Get how the menu item should be displayed.
 -- @param o The menu item.
--- @return item name, item background color, background image, item icon.
+-- @return item name, item background color, background image, item icon, item args.
 local function label(o)
     local fg_color = theme.menubar_fg_normal or theme.menu_fg_normal or theme.fg_normal
     local bg_color = theme.menubar_bg_normal or theme.menu_bg_normal or theme.bg_normal
@@ -160,7 +162,8 @@ local function label(o)
     return colortext(gstring.xml_escape(o.name), fg_color),
            bg_color,
            nil,
-           o.icon
+           o.icon,
+           o.icon and {icon_size=instance.geometry.height}
 end
 
 local function load_count_table()
@@ -225,6 +228,11 @@ end
 -- @tparam number|screen scr Screen
 -- @return table List of items for current page.
 local function get_current_page(all_items, query, scr)
+
+    local compute_text_width = function(text, s)
+        return wibox.widget.textbox.get_markup_geometry(text, s, instance.font)['width']
+    end
+
     scr = get_screen(scr)
     if not instance.prompt.width then
         instance.prompt.width = compute_text_width(instance.prompt.prompt, scr)
@@ -235,16 +243,19 @@ local function get_current_page(all_items, query, scr)
     if not menubar.right_label_width then
         menubar.right_label_width = compute_text_width(menubar.right_label, scr)
     end
+    local border_width = theme.menubar_border_width or theme.menu_border_width or 0
     local available_space = instance.geometry.width - menubar.right_margin -
         menubar.right_label_width - menubar.left_label_width -
-        compute_text_width(query, scr) - instance.prompt.width
+        compute_text_width(query..' ', scr) - instance.prompt.width - border_width * 2
+        -- space character is added as input cursor placeholder
 
     local width_sum = 0
     local current_page = {}
     for i, item in ipairs(all_items) do
-        item.width = item.width or
+        item.width = item.width or (
-            compute_text_width(item.name, scr) +
+            compute_text_width(label(item), scr) +
-            (item.icon and instance.geometry.height or 0) + list_interspace
+            (item.icon and (instance.geometry.height + list_spacing) or 0) + list_spacing * 2
+        )
         if width_sum + item.width > available_space then
             if current_item < i then
                 table.insert(current_page, { name = menubar.right_label, icon = nil })
@@ -449,6 +460,7 @@ function menubar.show(scr)
     local bg_color = theme.menubar_bg_normal or theme.menu_bg_normal or theme.bg_normal
     local border_width = theme.menubar_border_width or theme.menu_border_width or 0
     local border_color = theme.menubar_border_color or theme.menu_border_color
+    local font = theme.menubar_font or theme.font or "Monospace 10"
 
     if not instance then
         -- Add to each category the name of its key in all_categories
@@ -467,11 +479,13 @@ function menubar.show(scr)
                 fg = fg_color,
                 border_width = border_width,
                 border_color = border_color,
+                font = font,
             },
             widget = common_args.w,
             prompt = awful.widget.prompt(),
             query = nil,
             count_table = nil,
+            font = font,
         }
         local layout = wibox.layout.fixed.horizontal()
         layout:add(instance.prompt)
@@ -490,7 +504,7 @@ function menubar.show(scr)
     local geometry = menubar.geometry
     instance.geometry = {x = geometry.x or scrgeom.x,
                              y = geometry.y or scrgeom.y,
-                             height = geometry.height or gmath.round(theme.get_font_height() * 1.5),
+                             height = geometry.height or gmath.round(theme.get_font_height(font) * 1.5),
                              width = (geometry.width or scrgeom.width) - border_width * 2}
     instance.wibox:geometry(instance.geometry)
 

lib/menubar/utils.lua

@@ -19,7 +19,6 @@ local glib = lgi.GLib
 local w_textbox = require("wibox.widget.textbox")
 local gdebug = require("gears.debug")
 local protected_call = require("gears.protected_call")
-local gstring = require("gears.string")
 local unpack = unpack or table.unpack -- luacheck: globals unpack (compatibility with Lua 5.1)
 
 local utils = {}
@@ -410,6 +409,8 @@ function utils.parse_dir(dir_path, callback)
     end)
 end
 
+-- luacov: disable
+
 function utils.compute_textbox_width(textbox, s)
     gdebug.deprecate("Use 'width, _ = textbox:get_preferred_size(s)' directly.", {deprecated_in=4})
     s = screen[s or mouse.screen]
@@ -417,16 +418,13 @@ function utils.compute_textbox_width(textbox, s)
     return w
 end
 
---- Compute text width.
+function utils.compute_text_width(text, s, font)
--- @tparam str text Text.
+    gdebug.deprecate("Use 'width = textbox.get_markup_geometry(text, s, font)['width']'.", {deprecated_in=4})
--- @tparam number|screen s Screen
+    return w_textbox.get_markup_geometry(text, s, font)['width']
--- @treturn int Text width.
--- @staticfct menubar.utils.compute_text_width
-function utils.compute_text_width(text, s)
-    local w, _ = w_textbox(gstring.xml_escape(text)):get_preferred_size(s)
-    return w
 end
 
+-- luacov: enable
+
 return utils
 
 -- vim: filetype=lua:expandtab:shiftwidth=4:tabstop=8:softtabstop=4:textwidth=80

lib/wibox/widget/textbox.lua

@@ -381,6 +381,7 @@ end
 -- @tparam[opt=nil] integer|screen s The screen on which the textbox would be displayed.
 -- @tparam[opt=beautiful.font] string font The font description as string.
 -- @treturn table Geometry (width, height) hashtable.
+-- @staticfct wibox.widget.textbox.get_markup_geometry
 function textbox.get_markup_geometry(text, s, font)
     font = font or beautiful.font
     local pctx = PangoCairo.font_map_get_default():create_context()

tests/test-menubar.lua

@@ -10,14 +10,10 @@ function menubar.refresh(...)
     orig_refresh(...)
 end
 
--- XXX We are building Lua 5.3 with -DLUA_USE_APICHECK=1 and this catches some
+-- XXX This test sporadically errors on LuaJIT ("attempt to call a number
--- bugs in lgi. Thus, do not run this test with Lua 5.3 until lgi is fixed.
--- We run into the same bug when doing code-coverage analysis, supposedly
--- because the additional GC activity means that something which is GC-able when
--- it should not gets collected too early.
--- This test also sporadically errors on LuaJIT ("attempt to call a number
 -- value"). This might be a different issue, but still, disable the test.
-if _VERSION == "Lua 5.3" or debug.gethook() or jit then --luacheck: globals jit
+-- And also breaks other tests due to weirdness of older lgi version.
+if os.getenv('LGIVER') == '0.8.0' or jit then --luacheck: globals jit
     print("Skipping this test since it would just fail.")
     runner.run_steps { function() return true end }
     return