fix: file menu not updating active item and other menu issues

- Preselecting previous folder wasn't working sometimes.
- `join_path()` didn't account for drive letters already having a separator.

Also refactors how `open_file_navigation_menu()` stores menu items value. As a side effect this is now a lot faster as we don't have to serialize each item's path into meta table, but instead we now store just a path string and handle it as needed.

closes #355

scripts/uosc.lua

@@ -900,7 +900,7 @@ mp.add_key_binding(nil, 'open-file', function()
 	-- Update active file in directory navigation menu
 	local function handle_file_loaded()
 		if Menu:is_open('open-file') then
-			Elements.menu:activate_value(normalize_path(mp.get_property_native('path')))
+			Elements.menu:activate_one_value(normalize_path(mp.get_property_native('path')))
 		end
 	end
 

scripts/uosc_shared/elements/Menu.lua

@@ -351,7 +351,7 @@ end
 ---@param menu? MenuStack
 function Menu:select_value(value, menu)
 	menu = menu or self.current
-	local index = itable_find(menu.items, function(_, item) return item.value == value end)
+	local index = itable_find(menu.items, function(item) return item.value == value end)
 	self:select_index(index, 5)
 end
 
@@ -372,7 +372,7 @@ end
 
 ---@param index? integer
 ---@param menu? MenuStack
-function Menu:activate_unique_index(index, menu)
+function Menu:activate_one_index(index, menu)
 	self:deactivate_items(menu)
 	self:activate_index(index, menu)
 end
@@ -381,16 +381,16 @@ end
 ---@param menu? MenuStack
 function Menu:activate_value(value, menu)
 	menu = menu or self.current
-	local index = itable_find(menu.items, function(_, item) return item.value == value end)
+	local index = itable_find(menu.items, function(item) return item.value == value end)
 	self:activate_index(index, menu)
 end
 
 ---@param value? any
 ---@param menu? MenuStack
-function Menu:activate_unique_value(value, menu)
+function Menu:activate_one_value(value, menu)
 	menu = menu or self.current
-	local index = itable_find(menu.items, function(_, item) return item.value == value end)
+	local index = itable_find(menu.items, function(item) return item.value == value end)
-	self:activate_unique_index(index, menu)
+	self:activate_one_index(index, menu)
 end
 
 ---@param id string
@@ -420,7 +420,7 @@ end
 ---@param menu? MenuStack
 function Menu:delete_value(value, menu)
 	menu = menu or self.current
-	local index = itable_find(menu.items, function(_, item) return item.value == value end)
+	local index = itable_find(menu.items, function(item) return item.value == value end)
 	self:delete_index(index)
 end
 

scripts/uosc_shared/lib/menus.lua

@@ -159,6 +159,7 @@ function open_file_navigation_menu(directory_path, handle_select, opts)
 
 	local files, directories = read_directory(directory.path, opts.allowed_types)
 	local is_root = not directory.dirname
+	local path_separator = path_separator(directory.path)
 
 	if not files or not directories then return end
 
@@ -171,77 +172,72 @@ function open_file_navigation_menu(directory_path, handle_select, opts)
 
 	if is_root then
 		if state.os == 'windows' then
-			items[#items + 1] = {
+			items[#items + 1] = {title = '..', hint = 'Drives', value = '{drives}', separator = true}
-				title = '..', hint = 'Drives', value = {is_drives = true, is_to_parent = true}, separator = true,
-			}
 		end
 	else
-		local serialized = serialize_path(directory.dirname)
+		items[#items + 1] = {title = '..', hint = 'parent dir', value = directory.dirname, separator = true}
-		serialized.is_directory = true
-		serialized.is_to_parent = true
-		items[#items + 1] = {title = '..', hint = 'parent dir', value = serialized, separator = true}
 	end
 
-	local items_start_index = #items + 1
+	local selected_index = #items + 1
 
-	local path_separator = path_separator(directory.path)
 	for _, dir in ipairs(directories) do
-		local serialized = serialize_path(join_path(directory.path, dir))
+		items[#items + 1] = {title = dir, value = join_path(directory.path, dir), hint = path_separator}
-		if serialized then
-			serialized.is_directory = true
-			items[#items + 1] = {title = serialized.basename, value = serialized, hint = path_separator}
-		end
 	end
 
 	for _, file in ipairs(files) do
-		local serialized = serialize_path(join_path(directory.path, file))
+		items[#items + 1] = {title = file, value = join_path(directory.path, file)}
-		if serialized then
-			serialized.is_file = true
-			items[#items + 1] = {title = serialized.basename, value = serialized}
-		end
 	end
 
 	for index, item in ipairs(items) do
-		if not item.value.is_to_parent then
+		if not item.value.is_to_parent and opts.active_path == item.value then
-			if index == items_start_index then item.selected = true end
-
-			if opts.active_path == item.value.path then
 			item.active = true
-				if not opts.selected_path then item.selected = true end
+			if not opts.selected_path then selected_index = index end
 		end
 
-			if opts.selected_path == item.value.path then item.selected = true end
+		if opts.selected_path == item.value then selected_index = index end
-		end
 	end
 
 	local menu_data = {
 		type = opts.type, title = opts.title or directory.basename .. path_separator, items = items,
-		on_open = opts.on_open, on_close = opts.on_close,
+		selected_index = selected_index,
 	}
+	local menu_options = {on_open = opts.on_open, on_close = opts.on_close}
 
 	return Menu:open(menu_data, function(path)
+		local is_drives = path == '{drives}'
+		local is_to_parent = is_drives or #path < #directory_path
 		local inheritable_options = {
 			type = opts.type, title = opts.title, allowed_types = opts.allowed_types, active_path = opts.active_path,
 		}
 
-		if path.is_drives then
+		if is_drives then
 			open_drives_menu(function(drive_path)
 				open_file_navigation_menu(drive_path, handle_select, inheritable_options)
-			end, {type = inheritable_options.type, title = inheritable_options.title, selected_path = directory.path})
+			end, {
+				type = inheritable_options.type, title = inheritable_options.title, selected_path = directory.path,
+				on_open = opts.on_open, on_close = opts.on_close,
+			})
 			return
 		end
 
-		if path.is_directory then
+		local info, error = utils.file_info(path)
+
+		if not info then
+			msg.error('Can\'t retrieve path info for "' .. path .. '". Error: ' .. (error or ''))
+			return
+		end
+
+		if info.is_dir then
 			--  Preselect directory we are coming from
-			if path.is_to_parent then
+			if is_to_parent then
 				inheritable_options.selected_path = directory.path
 			end
 
-			open_file_navigation_menu(path.path, handle_select, inheritable_options)
+			open_file_navigation_menu(path, handle_select, inheritable_options)
 		else
-			handle_select(path.path)
+			handle_select(path)
 		end
-	end)
+	end, menu_options)
 end
 
 -- Opens a file navigation menu with Windows drives as items.
@@ -255,7 +251,7 @@ function open_drives_menu(handle_select, opts)
 		playback_only = false,
 		args = {'wmic', 'logicaldisk', 'get', 'name', '/value'},
 	})
-	local items = {}
+	local items, selected_index = {}, 1
 
 	if process.status == 0 then
 		for _, value in ipairs(split(process.stdout, '\n')) do
@@ -263,15 +259,17 @@ function open_drives_menu(handle_select, opts)
 			if drive then
 				local drive_path = normalize_path(drive)
 				items[#items + 1] = {
-					title = drive, hint = 'Drive', value = drive_path,
+					title = drive, hint = 'drive', value = drive_path, active = opts.active_path == drive_path,
-					selected = opts.selected_path == drive_path,
-					active = opts.active_path == drive_path,
 				}
+				if opts.selected_path == drive_path then selected_index = #items end
 			end
 		end
 	else
 		msg.error(process.stderr)
 	end
 
-	return Menu:open({type = opts.type, title = opts.title or 'Drives', items = items}, handle_select)
+	return Menu:open(
+		{type = opts.type, title = opts.title or 'Drives', items = items, selected_index = selected_index},
+		handle_select
+	)
 end

scripts/uosc_shared/lib/utils.lua

@@ -161,7 +161,10 @@ end)()
 ---@param p2 string
 ---@return string
 function join_path(p1, p2)
-	return p1 .. path_separator(p1) .. p2
+	local p1, separator = trim_trailing_separator(p1)
+	-- Prevents joining drive letters with a redundant separator (`C:\\foo`),
+	-- as `trim_trailing_separator()` doesn't trim separators from drive letters.
+	return p1:sub(#p1) == separator and p1 .. p2 or p1 .. separator.. p2
 end
 
 -- Check if path is absolute.
@@ -183,17 +186,17 @@ end
 
 -- Remove trailing slashes/backslashes.
 ---@param path string
----@return string
+---@return string path, string trimmed_separator_type
 function trim_trailing_separator(path)
-	path = trim_end(path, path_separator(path))
+	local separator = path_separator(path)
+	path = trim_end(path, separator)
 	if state.os == 'windows' then
 		-- Drive letters on windows need trailing backslash
-		if path:sub(#path) == ':' then return path .. '\\' end
+		if path:sub(#path) == ':' then path = path .. '\\' end
-		return path
 	else
-		if path == '' then return '/' end
+		if path == '' then path = '/' end
-		return path
 	end
+	return path, separator
 end
 
 -- Ensures path is absolute, remove trailing slashes/backslashes.
@@ -202,8 +205,8 @@ end
 ---@return string
 function normalize_path_lite(path)
 	if not path or is_protocol(path) then return path end
-	path = ensure_absolute(path)
+	path = trim_trailing_separator(ensure_absolute(path))
-	return trim_trailing_separator(path)
+	return path
 end
 
 -- Ensures path is absolute, remove trailing slashes/backslashes, normalization of path separators and deduplication.