Fix file dialog crash and drive list on Linux

ImGui.Popups/FilesystemBrowser.cs

@@ -5,11 +5,11 @@
 namespace ktsu.ImGui.Popups;
 
 using System;
+using System.Collections.Generic;
 using System.Collections.ObjectModel;
 using System.IO;
 using System.Linq;
 using System.Numerics;
-using System.Text;
 using System.Text.Json.Serialization;
 using Hexa.NET.ImGui;
 using ktsu.Extensions;
@@ -59,6 +59,22 @@ public enum FilesystemBrowserTarget
 	/// </summary>
 	public class FilesystemBrowser
 	{
+		/// <summary>
+		/// Mount point prefixes that are hidden from the drive list on Unix-like systems.
+		/// </summary>
+		/// <remarks>
+		/// <see cref="Environment.GetLogicalDrives"/> reports every mount point on Unix, so the raw list is
+		/// dominated by kernel pseudo filesystems (<c>/proc</c>, <c>/sys</c>, <c>/dev</c>) and runtime state
+		/// (<c>/run</c>) that hold nothing a user would browse to. Removable media is commonly mounted under
+		/// <c>/run/media</c>, so that subtree is kept — see <see cref="RemovableMediaPrefix"/>.
+		/// </remarks>
+		private static readonly string[] HiddenUnixMountPrefixes = ["/proc", "/sys", "/dev", "/run"];
+
+		/// <summary>
+		/// The mount point prefix for removable media, kept even though it sits under a hidden prefix.
+		/// </summary>
+		private const string RemovableMediaPrefix = "/run/media";
+
 		/// <summary>
 		/// Gets or sets the mode of the browser (Open or Save).
 		/// </summary>
@@ -205,7 +221,7 @@ private void OpenPopup(string title, FilesystemBrowserMode mode, FilesystemBrows
 			Matcher = new();
 			Matcher.AddInclude(Glob);
 			Drives.Clear();
-			Environment.GetLogicalDrives().ForEach(Drives.Add);
+			GetNavigableDrives().ForEach(Drives.Add);
 			RefreshContents();
 			Modal.Open(title, ShowContent, customSize);
 		}
@@ -237,18 +253,19 @@ private void ShowContent()
 		/// </summary>
 		private void DrawDrivesCombo()
 		{
-			if (Drives.Count != 0 && ImGui.BeginCombo("##Drives", Drives[0]))
+			if (Drives.Count == 0)
 			{
-				StringBuilder currentDriveStringBuilder = new();
-				currentDriveStringBuilder.Append(CurrentDirectory.Split(Path.VolumeSeparatorChar).Current);
-				currentDriveStringBuilder.Append(Path.VolumeSeparatorChar);
-				currentDriveStringBuilder.Append(Path.DirectorySeparatorChar);
-				string currentDrive = currentDriveStringBuilder.ToString();
+				return;
+			}
 
+			string currentDrive = CurrentDriveOf(CurrentDirectory, Drives[0]);
+
+			if (ImGui.BeginCombo("##Drives", currentDrive))
+			{
 #pragma warning disable S3267 // Explicit loop is clearer; LINQ rewrite would add unnecessary allocation and complicate ImGui immediate-mode usage.
 				foreach (string drive in Drives)
 				{
-					if (ImGui.Selectable(drive, drive == currentDrive))
+					if (ImGui.Selectable(drive, string.Equals(drive, currentDrive, StringComparison.OrdinalIgnoreCase)))
 					{
 						CurrentDirectory = drive.As<AbsoluteDirectoryPath>();
 						RefreshContents();
@@ -260,6 +277,45 @@ private void DrawDrivesCombo()
 			}
 		}
 
+		/// <summary>
+		/// Gets the logical drives worth offering in the drive combo, deduplicated and ordered.
+		/// </summary>
+		/// <returns>The drives to display.</returns>
+		internal static IEnumerable<string> GetNavigableDrives() =>
+			Environment.GetLogicalDrives()
+				.Where(IsNavigableDrive)
+				.Distinct(StringComparer.OrdinalIgnoreCase)
+				.OrderBy(drive => drive, StringComparer.OrdinalIgnoreCase);
+
+		/// <summary>
+		/// Determines whether a logical drive is user-navigable storage rather than a pseudo filesystem.
+		/// </summary>
+		/// <param name="drive">The drive reported by <see cref="Environment.GetLogicalDrives"/>.</param>
+		/// <returns><see langword="true"/> if the drive should be listed; otherwise, <see langword="false"/>.</returns>
+		internal static bool IsNavigableDrive(string drive)
+		{
+			if (OperatingSystem.IsWindows())
+			{
+				return true;
+			}
+
+			return drive.StartsWith(RemovableMediaPrefix, StringComparison.Ordinal)
+				|| !HiddenUnixMountPrefixes.Any(prefix => drive.StartsWith(prefix, StringComparison.Ordinal));
+		}
+
+		/// <summary>
+		/// Gets the root of the given directory in the same form <see cref="Environment.GetLogicalDrives"/> reports,
+		/// so it can be matched against <see cref="Drives"/>.
+		/// </summary>
+		/// <param name="directory">The directory to take the root of.</param>
+		/// <param name="fallback">The value to return when the directory has no determinable root.</param>
+		/// <returns>The directory's root, or <paramref name="fallback"/> when the root cannot be determined.</returns>
+		internal static string CurrentDriveOf(AbsoluteDirectoryPath directory, string fallback)
+		{
+			string? root = Path.GetPathRoot(directory.WeakString);
+			return string.IsNullOrEmpty(root) ? fallback : root;
+		}
+
 		/// <summary>
 		/// Draws the table listing the parent directory entry and the current directory contents.
 		/// </summary>
@@ -273,7 +329,7 @@ private void DrawFileTable()
 				ImGuiSelectableFlags flags = ImGuiSelectableFlags.SpanAllColumns | ImGuiSelectableFlags.AllowDoubleClick | ImGuiSelectableFlags.NoAutoClosePopups;
 				DrawParentDirectoryRow(flags);
 
-				foreach (IAbsolutePath? path in CurrentContents.OrderBy(p => p is not AbsoluteDirectoryPath).ThenBy(p => p).ToCollection())
+				foreach (IAbsolutePath path in SortContents(CurrentContents))
 				{
 					DrawContentRow(path, flags);
 				}
@@ -284,6 +340,26 @@ private void DrawFileTable()
 			ImGui.EndChild();
 		}
 
+		/// <summary>
+		/// Orders filesystem entries for display, listing directories before files and sorting each group by name.
+		/// </summary>
+		/// <param name="contents">The filesystem entries to order.</param>
+		/// <returns>The ordered entries.</returns>
+		/// <remarks>
+		/// The secondary sort key is the entry's string value rather than the entry itself because
+		/// <see cref="IAbsolutePath"/> does not implement <see cref="IComparable{T}"/>. Sorting by the entry
+		/// would make LINQ fall back to <see cref="Comparer{T}.Default"/>, which calls the non-generic
+		/// <see cref="IComparable.CompareTo(object)"/> on the underlying semantic string, which throws
+		/// <see cref="ArgumentException"/> when handed a path object in ktsu.Semantics.Strings 2.7.10 and
+		/// earlier. Sorting on the string value also gives case-insensitive display order.
+		/// </remarks>
+		internal static Collection<IAbsolutePath> SortContents(IEnumerable<IAbsolutePath> contents) =>
+			contents
+				.OrderBy(p => p is not AbsoluteDirectoryPath)
+				.ThenBy(p => p.ToString(), StringComparer.OrdinalIgnoreCase)
+				.ThenBy(p => p.ToString(), StringComparer.Ordinal)
+				.ToCollection();
+
 		/// <summary>
 		/// Draws the ".." row that navigates to the parent directory.
 		/// </summary>

ImGui.Popups/ImGui.Popups.csproj

@@ -6,6 +6,10 @@
     <TargetFrameworks>net10.0;net9.0;net8.0</TargetFrameworks>
   </PropertyGroup>
 
+  <ItemGroup>
+    <InternalsVisibleTo Include="ktsu.ImGui.Popups.Tests" />
+  </ItemGroup>
+
   <ItemGroup>
     <PackageReference Include="Hexa.NET.ImGui" />
     <PackageReference Include="ktsu.Extensions" />

ImGui.sln

@@ -49,6 +49,8 @@ Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ImGui.Markdown.Tests", "tes
 EndProject
 Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ImGuiMarkdownDemo", "examples\ImGuiMarkdownDemo\ImGuiMarkdownDemo.csproj", "{8766DCC8-BA77-4983-8653-2ABFBC2D3CE1}"
 EndProject
+Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "ImGui.Popups.Tests", "tests\ImGui.Popups.Tests\ImGui.Popups.Tests.csproj", "{43E27D79-983E-471B-ACC4-D1F73ECB53D0}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -311,6 +313,18 @@ Global
 		{8766DCC8-BA77-4983-8653-2ABFBC2D3CE1}.Release|x64.Build.0 = Release|Any CPU
 		{8766DCC8-BA77-4983-8653-2ABFBC2D3CE1}.Release|x86.ActiveCfg = Release|Any CPU
 		{8766DCC8-BA77-4983-8653-2ABFBC2D3CE1}.Release|x86.Build.0 = Release|Any CPU
+		{43E27D79-983E-471B-ACC4-D1F73ECB53D0}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{43E27D79-983E-471B-ACC4-D1F73ECB53D0}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{43E27D79-983E-471B-ACC4-D1F73ECB53D0}.Debug|x64.ActiveCfg = Debug|Any CPU
+		{43E27D79-983E-471B-ACC4-D1F73ECB53D0}.Debug|x64.Build.0 = Debug|Any CPU
+		{43E27D79-983E-471B-ACC4-D1F73ECB53D0}.Debug|x86.ActiveCfg = Debug|Any CPU
+		{43E27D79-983E-471B-ACC4-D1F73ECB53D0}.Debug|x86.Build.0 = Debug|Any CPU
+		{43E27D79-983E-471B-ACC4-D1F73ECB53D0}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{43E27D79-983E-471B-ACC4-D1F73ECB53D0}.Release|Any CPU.Build.0 = Release|Any CPU
+		{43E27D79-983E-471B-ACC4-D1F73ECB53D0}.Release|x64.ActiveCfg = Release|Any CPU
+		{43E27D79-983E-471B-ACC4-D1F73ECB53D0}.Release|x64.Build.0 = Release|Any CPU
+		{43E27D79-983E-471B-ACC4-D1F73ECB53D0}.Release|x86.ActiveCfg = Release|Any CPU
+		{43E27D79-983E-471B-ACC4-D1F73ECB53D0}.Release|x86.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -327,6 +341,7 @@ Global
 		{D9556A7D-9672-47F7-9348-68E0B13777EE} = {5F0E6F1C-44C1-42B7-ADF4-770ABB471B40}
 		{4CC3D936-4F07-4C65-9EA1-D55662268D89} = {5F0E6F1C-44C1-42B7-ADF4-770ABB471B40}
 		{8766DCC8-BA77-4983-8653-2ABFBC2D3CE1} = {02EA681E-C7D8-13C7-8484-4AC65E1B71E8}
+		{43E27D79-983E-471B-ACC4-D1F73ECB53D0} = {5F0E6F1C-44C1-42B7-ADF4-770ABB471B40}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution
 		SolutionGuid = {14515AC1-A74C-4CBB-98F2-9DA660E967EF}

tests/ImGui.Popups.Tests/FilesystemBrowserDriveTests.cs

@@ -0,0 +1,118 @@
+// Copyright (c) ktsu.dev
+// All rights reserved.
+// Licensed under the MIT license.
+
+namespace ktsu.ImGui.Popups.Tests;
+
+using System;
+using System.Collections.Generic;
+using System.IO;
+using System.Linq;
+
+using ktsu.Semantics.Paths;
+using ktsu.Semantics.Strings;
+
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+
+[TestClass]
+public sealed class FilesystemBrowserDriveTests
+{
+	[TestMethod]
+	public void CurrentDriveOf_ReturnsRootOfDirectory()
+	{
+		string root = Path.GetPathRoot(Path.GetTempPath())!;
+		AbsoluteDirectoryPath directory = Path.Combine(Path.GetTempPath(), "some", "nested", "directory").As<AbsoluteDirectoryPath>();
+
+		Assert.AreEqual(root, ImGuiPopups.FilesystemBrowser.CurrentDriveOf(directory, "fallback"));
+	}
+
+	[TestMethod]
+	public void CurrentDriveOf_WithoutDeterminableRoot_ReturnsFallback()
+	{
+		AbsoluteDirectoryPath directory = string.Empty.As<AbsoluteDirectoryPath>();
+
+		Assert.AreEqual("fallback", ImGuiPopups.FilesystemBrowser.CurrentDriveOf(directory, "fallback"));
+	}
+
+	[TestMethod]
+	public void GetNavigableDrives_IsSortedAndDeduplicated()
+	{
+		List<string> drives = [.. ImGuiPopups.FilesystemBrowser.GetNavigableDrives()];
+
+		CollectionAssert.AreEqual(drives.Distinct(StringComparer.OrdinalIgnoreCase).ToList(), drives, "Drives should be deduplicated.");
+		CollectionAssert.AreEqual(drives.OrderBy(d => d, StringComparer.OrdinalIgnoreCase).ToList(), drives, "Drives should be sorted.");
+	}
+
+	[TestMethod]
+	public void GetNavigableDrives_IncludesRootOfCurrentDirectory()
+	{
+		string root = Path.GetPathRoot(Environment.CurrentDirectory)!;
+
+		List<string> drives = [.. ImGuiPopups.FilesystemBrowser.GetNavigableDrives()];
+
+		Assert.IsTrue(
+			drives.Contains(root, StringComparer.OrdinalIgnoreCase),
+			$"Expected the root of the current directory ('{root}') among [{string.Join(", ", drives)}].");
+	}
+
+	[TestMethod]
+	[TestCategory("OS-Specific")]
+	public void IsNavigableDrive_OnUnix_ExcludesPseudoFilesystems()
+	{
+		if (OperatingSystem.IsWindows())
+		{
+			Assert.Inconclusive("Unix-only mount point filtering.");
+			return;
+		}
+
+		Assert.IsFalse(ImGuiPopups.FilesystemBrowser.IsNavigableDrive("/proc"));
+		Assert.IsFalse(ImGuiPopups.FilesystemBrowser.IsNavigableDrive("/proc/sys/fs/binfmt_misc"));
+		Assert.IsFalse(ImGuiPopups.FilesystemBrowser.IsNavigableDrive("/sys/kernel/debug"));
+		Assert.IsFalse(ImGuiPopups.FilesystemBrowser.IsNavigableDrive("/dev/shm"));
+		Assert.IsFalse(ImGuiPopups.FilesystemBrowser.IsNavigableDrive("/run/user/1000/gvfs"));
+	}
+
+	[TestMethod]
+	[TestCategory("OS-Specific")]
+	public void IsNavigableDrive_OnUnix_KeepsRealStorage()
+	{
+		if (OperatingSystem.IsWindows())
+		{
+			Assert.Inconclusive("Unix-only mount point filtering.");
+			return;
+		}
+
+		Assert.IsTrue(ImGuiPopups.FilesystemBrowser.IsNavigableDrive("/"));
+		Assert.IsTrue(ImGuiPopups.FilesystemBrowser.IsNavigableDrive("/home"));
+		Assert.IsTrue(ImGuiPopups.FilesystemBrowser.IsNavigableDrive("/tmp"));
+		Assert.IsTrue(ImGuiPopups.FilesystemBrowser.IsNavigableDrive("/mnt/data"));
+		Assert.IsTrue(ImGuiPopups.FilesystemBrowser.IsNavigableDrive("/media/usb"));
+	}
+
+	[TestMethod]
+	[TestCategory("OS-Specific")]
+	public void IsNavigableDrive_OnUnix_KeepsRemovableMediaUnderRun()
+	{
+		if (OperatingSystem.IsWindows())
+		{
+			Assert.Inconclusive("Unix-only mount point filtering.");
+			return;
+		}
+
+		Assert.IsTrue(ImGuiPopups.FilesystemBrowser.IsNavigableDrive("/run/media/user/USB"));
+	}
+
+	[TestMethod]
+	[TestCategory("OS-Specific")]
+	public void IsNavigableDrive_OnWindows_KeepsEveryDrive()
+	{
+		if (!OperatingSystem.IsWindows())
+		{
+			Assert.Inconclusive("Windows-only drive handling.");
+			return;
+		}
+
+		Assert.IsTrue(ImGuiPopups.FilesystemBrowser.IsNavigableDrive(@"C:\"));
+		Assert.IsTrue(ImGuiPopups.FilesystemBrowser.IsNavigableDrive(@"Z:\"));
+	}
+}