review

Author: ethanndickson

Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift

@@ -5,7 +5,7 @@ import SwiftUI
 struct FilePicker: View {
     @Environment(\.dismiss) var dismiss
     @StateObject private var model: FilePickerModel
-    @State private var selection: FilePickerItemModel.ID?
+    @State private var selection: FilePickerEntryModel?
 
     @Binding var outputAbsPath: String
 
@@ -35,17 +35,16 @@ struct FilePicker: View {
                     .padding()
             } else {
                 List(selection: $selection) {
-                    ForEach(model.rootFiles) { rootItem in
-                        FilePickerItem(item: rootItem)
+                    ForEach(model.rootEntries) { entry in
+                        FilePickerEntry(entry: entry).tag(entry)
                     }
                 }.contextMenu(
-                    forSelectionType: FilePickerItemModel.ID.self,
+                    forSelectionType: FilePickerEntryModel.self,
                     menu: { _ in },
                     primaryAction: { selections in
                         // Per the type of `selection`, this will only ever be a set of
-                        // one item.
-                        let files = model.findFilesByIds(ids: selections)
-                        files.forEach { file in withAnimation { file.isExpanded.toggle() } }
+                        // one entry.
+                        selections.forEach { entry in withAnimation { entry.isExpanded.toggle() } }
                     }
                 ).listStyle(.sidebar)
             }
@@ -64,20 +63,19 @@ struct FilePicker: View {
 
     private func submit() {
         guard let selection else { return }
-        let files = model.findFilesByIds(ids: [selection])
-        if let file = files.first {
-            outputAbsPath = file.absolute_path
-        }
+        outputAbsPath = selection.absolute_path
         dismiss()
     }
 }
 
 @MainActor
 class FilePickerModel: ObservableObject {
-    @Published var rootFiles: [FilePickerItemModel] = []
+    @Published var rootEntries: [FilePickerEntryModel] = []
     @Published var rootIsLoading: Bool = false
     @Published var error: ClientError?
 
+    // It's important that `AgentClient` is a reference type (class)
+    // as we were having performance issues with a struct (unless it was a binding).
     let client: AgentClient
 
     init(host: String) {
@@ -90,82 +88,56 @@ class FilePickerModel: ObservableObject {
         Task {
             defer { rootIsLoading = false }
             do throws(ClientError) {
-                rootFiles = try await client
+                rootEntries = try await client
                     .listAgentDirectory(.init(path: [], relativity: .root))
-                    .toModels(client: client, path: [])
+                    .toModels(client: client)
             } catch {
                 self.error = error
             }
         }
     }
-
-    func findFilesByIds(ids: Set<FilePickerItemModel.ID>) -> [FilePickerItemModel] {
-        var result: [FilePickerItemModel] = []
-
-        for id in ids {
-            if let file = findFileByPath(path: id, in: rootFiles) {
-                result.append(file)
-            }
-        }
-
-        return result
-    }
-
-    private func findFileByPath(path: [String], in files: [FilePickerItemModel]?) -> FilePickerItemModel? {
-        guard let files, !path.isEmpty else { return nil }
-
-        if let file = files.first(where: { $0.name == path[0] }) {
-            if path.count == 1 {
-                return file
-            }
-            // Array slices are just views, so this isn't expensive
-            return findFileByPath(path: Array(path[1...]), in: file.contents)
-        }
-
-        return nil
-    }
 }
 
-struct FilePickerItem: View {
-    @ObservedObject var item: FilePickerItemModel
+struct FilePickerEntry: View {
+    @ObservedObject var entry: FilePickerEntryModel
 
     var body: some View {
         Group {
-            if item.dir {
+            if entry.dir {
                 directory
             } else {
-                Label(item.name, systemImage: "doc")
-                    .help(item.absolute_path)
+                Label(entry.name, systemImage: "doc")
+                    .help(entry.absolute_path)
                     .selectionDisabled()
                     .foregroundColor(.secondary)
             }
         }
     }
 
     private var directory: some View {
-        DisclosureGroup(isExpanded: $item.isExpanded) {
-            if let contents = item.contents {
-                ForEach(contents) { item in
-                    FilePickerItem(item: item)
+        DisclosureGroup(isExpanded: $entry.isExpanded) {
+            if let entries = entry.entries {
+                ForEach(entries) { entry in
+                    FilePickerEntry(entry: entry).tag(entry)
                 }
             }
         } label: {
             Label {
-                Text(item.name)
+                Text(entry.name)
                 ZStack {
-                    ProgressView().controlSize(.small).opacity(item.isLoading && item.error == nil ? 1 : 0)
+                    ProgressView().controlSize(.small).opacity(entry.isLoading && entry.error == nil ? 1 : 0)
                     Image(systemName: "exclamationmark.triangle.fill")
-                        .opacity(item.error != nil ? 1 : 0)
+                        .opacity(entry.error != nil ? 1 : 0)
                 }
             } icon: {
                 Image(systemName: "folder")
-            }.help(item.error != nil ? item.error!.description : item.absolute_path)
+            }.help(entry.error != nil ? entry.error!.description : entry.absolute_path)
         }
     }
 }
 
 @MainActor
-class FilePickerItemModel: Identifiable, ObservableObject {
+class FilePickerEntryModel: Identifiable, Hashable, ObservableObject {
     nonisolated let id: [String]
     let name: String
     // Components of the path as an array
@@ -175,7 +147,7 @@ class FilePickerItemModel: Identifiable, ObservableObject {
 
     let client: AgentClient
 
-    @Published var contents: [FilePickerItemModel]?
+    @Published var entries: [FilePickerEntryModel]?
     @Published var isLoading = false
     @Published var error: ClientError?
     @Published private var innerIsExpanded = false
@@ -186,7 +158,7 @@ class FilePickerItemModel: Identifiable, ObservableObject {
                 withAnimation { self.innerIsExpanded = false }
             } else {
                 Task {
-                    self.loadContents()
+                    self.loadEntries()
                 }
             }
         }
@@ -198,20 +170,20 @@ class FilePickerItemModel: Identifiable, ObservableObject {
         absolute_path: String,
         path: [String],
         dir: Bool = false,
-        contents: [FilePickerItemModel]? = nil
+        entries: [FilePickerEntryModel]? = nil
     ) {
         self.name = name
         self.client = client
         self.path = path
         self.dir = dir
         self.absolute_path = absolute_path
-        self.contents = contents
+        self.entries = entries
 
-        // Swift Arrays are COW
+        // Swift Arrays are copy on write
         id = path
     }
 
-    func loadContents() {
+    func loadEntries() {
         self.error = nil
         withAnimation { isLoading = true }
         Task {
@@ -222,30 +194,38 @@ class FilePickerItemModel: Identifiable, ObservableObject {
                 }
             }
             do throws(ClientError) {
-                contents = try await client
+                entries = try await client
                     .listAgentDirectory(.init(path: path, relativity: .root))
-                    .toModels(client: client, path: path)
+                    .toModels(client: client)
             } catch {
                 self.error = error
             }
         }
     }
+
+    nonisolated static func == (lhs: FilePickerEntryModel, rhs: FilePickerEntryModel) -> Bool {
+        lhs.id == rhs.id
+    }
+
+    nonisolated func hash(into hasher: inout Hasher) {
+        hasher.combine(id)
+    }
 }
 
 extension LSResponse {
     @MainActor
-    func toModels(client: AgentClient, path: [String]) -> [FilePickerItemModel] {
-        contents.compactMap { file in
+    func toModels(client: AgentClient) -> [FilePickerEntryModel] {
+        contents.compactMap { entry in
             // Filter dotfiles from the picker
-            guard !file.name.hasPrefix(".") else { return nil }
+            guard !entry.name.hasPrefix(".") else { return nil }
 
-            return FilePickerItemModel(
-                name: file.name,
+            return FilePickerEntryModel(
+                name: entry.name,
                 client: client,
-                absolute_path: file.absolute_path_string,
-                path: path + [file.name],
-                dir: file.is_dir,
-                contents: nil
+                absolute_path: entry.absolute_path_string,
+                path: self.absolute_path + [entry.name],
+                dir: entry.is_dir,
+                entries: nil
             )
         }
     }