From 5b4d965b074b90fa0b948b8e431186d66c20fc5f Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Thu, 3 Apr 2025 18:29:36 +1100 Subject: [PATCH 1/6] feat: add remote folder picker to file sync GUI --- Coder-Desktop/Coder-Desktop/Info.plist | 9 + .../Views/FileSync/FilePicker.swift | 255 ++++++++++++++++++ .../Views/FileSync/FileSyncSessionModal.swift | 15 +- .../Coder-DesktopTests/FilePickerTests.swift | 115 ++++++++ Coder-Desktop/Coder-DesktopTests/Util.swift | 25 ++ Coder-Desktop/CoderSDK/AgentLS.swift | 44 +++ Coder-Desktop/CoderSDK/Client.swift | 2 +- 7 files changed, 463 insertions(+), 2 deletions(-) create mode 100644 Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift create mode 100644 Coder-Desktop/Coder-DesktopTests/FilePickerTests.swift create mode 100644 Coder-Desktop/CoderSDK/AgentLS.swift diff --git a/Coder-Desktop/Coder-Desktop/Info.plist b/Coder-Desktop/Coder-Desktop/Info.plist index 8609906b..5e59b253 100644 --- a/Coder-Desktop/Coder-Desktop/Info.plist +++ b/Coder-Desktop/Coder-Desktop/Info.plist @@ -2,6 +2,15 @@ + NSAppTransportSecurity + + + NSAllowsArbitraryLoads + + NetworkExtension NEMachServiceName diff --git a/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift b/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift new file mode 100644 index 00000000..29a1115e --- /dev/null +++ b/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift @@ -0,0 +1,255 @@ +import CoderSDK +import Foundation +import SwiftUI + +struct FilePicker: View { + @Environment(\.dismiss) var dismiss + @StateObject private var model: FilePickerModel + @State private var selection: FilePickerItemModel.ID? + + @Binding var outputAbsPath: String + + let inspection = Inspection() + + init( + host: String, + outputAbsPath: Binding + ) { + _model = StateObject(wrappedValue: FilePickerModel(host: host)) + _outputAbsPath = outputAbsPath + } + + var body: some View { + VStack(spacing: 0) { + if model.isLoading { + Spacer() + ProgressView() + .controlSize(.large) + Spacer() + } else if let loadError = model.error { + Text("\(loadError.description)") + .font(.headline) + .foregroundColor(.red) + .multilineTextAlignment(.center) + .frame(maxWidth: .infinity, maxHeight: .infinity) + .padding() + } else { + List(selection: $selection) { + ForEach(model.rootFiles) { rootItem in + FilePickerItem(item: rootItem) + } + }.contextMenu( + forSelectionType: FilePickerItemModel.ID.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() } } + } + ).listStyle(.sidebar) + } + Divider() + HStack { + Spacer() + Button("Cancel", action: { dismiss() }).keyboardShortcut(.cancelAction) + Button("Select", action: submit).keyboardShortcut(.defaultAction).disabled(selection == nil) + }.padding(20) + } + .onAppear { + model.loadRoot() + } + .onReceive(inspection.notice) { inspection.visit(self, $0) } // ViewInspector + } + + private func submit() { + guard let selection else { return } + let files = model.findFilesByIds(ids: [selection]) + if let file = files.first { + outputAbsPath = file.absolute_path + } + dismiss() + } +} + +@MainActor +class FilePickerModel: ObservableObject { + @Published var rootFiles: [FilePickerItemModel] = [] + @Published var isLoading: Bool = false + @Published var error: ClientError? + + let client: Client + + init(host: String) { + client = Client(url: URL(string: "http://\(host):4")!) + } + + func loadRoot() { + error = nil + isLoading = true + Task { + defer { isLoading = false } + do throws(ClientError) { + rootFiles = try await client + .listAgentDirectory(.init(path: [], relativity: .root)) + .toModels(client: Binding(get: { self.client }, set: { _ in }), path: []) + } catch { + self.error = error + } + } + } + + func findFilesByIds(ids: Set) -> [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 + + var body: some View { + Group { + if item.dir { + directory + } else { + Label(item.name, systemImage: "doc") + .help(item.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) + } + } + } label: { + Label { + Text(item.name) + ZStack { + ProgressView().controlSize(.small).opacity(item.isLoading && item.error == nil ? 1 : 0) + Image(systemName: "exclamationmark.triangle.fill") + .opacity(item.error != nil ? 1 : 0) + } + } icon: { + Image(systemName: "folder") + }.help(item.error != nil ? item.error!.description : item.absolute_path) + } + } +} + +@MainActor +class FilePickerItemModel: Identifiable, ObservableObject { + nonisolated let id: [String] + let name: String + // Components of the path as an array + let path: [String] + let absolute_path: String + let dir: Bool + + // This being a binding is pretty important performance-wise, as it's a struct + // that would otherwise be recreated every time the the item row is rendered. + // Removing the binding results in very noticeable lag when scrolling a file tree. + @Binding var client: Client + + @Published var contents: [FilePickerItemModel]? + @Published var isLoading = false + @Published var error: ClientError? + @Published private var innerIsExpanded = false + var isExpanded: Bool { + get { innerIsExpanded } + set { + if !newValue { + withAnimation { self.innerIsExpanded = false } + } else { + Task { + self.loadContents() + } + } + } + } + + init( + name: String, + client: Binding, + absolute_path: String, + path: [String], + dir: Bool = false, + contents: [FilePickerItemModel]? = nil + ) { + self.name = name + _client = client + self.path = path + self.dir = dir + self.absolute_path = absolute_path + self.contents = contents + + // Swift Arrays are COW + id = path + } + + func loadContents() { + self.error = nil + withAnimation { isLoading = true } + Task { + defer { + withAnimation { + isLoading = false + innerIsExpanded = true + } + } + do throws(ClientError) { + contents = try await client + .listAgentDirectory(.init(path: path, relativity: .root)) + .toModels(client: $client, path: path) + } catch { + self.error = error + } + } + } +} + +extension LSResponse { + @MainActor + func toModels(client: Binding, path: [String]) -> [FilePickerItemModel] { + contents.compactMap { file in + // Filter dotfiles from the picker + guard !file.name.hasPrefix(".") else { return nil } + + return FilePickerItemModel( + name: file.name, + client: client, + absolute_path: file.absolute_path_string, + path: path + [file.name], + dir: file.is_dir, + contents: nil + ) + } + } +} diff --git a/Coder-Desktop/Coder-Desktop/Views/FileSync/FileSyncSessionModal.swift b/Coder-Desktop/Coder-Desktop/Views/FileSync/FileSyncSessionModal.swift index 0e42ea0c..fdefab95 100644 --- a/Coder-Desktop/Coder-Desktop/Views/FileSync/FileSyncSessionModal.swift +++ b/Coder-Desktop/Coder-Desktop/Views/FileSync/FileSyncSessionModal.swift @@ -13,6 +13,7 @@ struct FileSyncSessionModal: View { @State private var loading: Bool = false @State private var createError: DaemonError? + @State private var pickingRemote: Bool = false var body: some View { let agents = vpn.menuState.onlineAgents @@ -46,7 +47,16 @@ struct FileSyncSessionModal: View { } } Section { - TextField("Remote Path", text: $remotePath) + HStack(spacing: 5) { + TextField("Remote Path", text: $remotePath) + Spacer() + Button { + pickingRemote = true + } label: { + Image(systemName: "folder") + }.disabled(workspace == nil) + .help(workspace == nil ? "Select a workspace first" : "Open File Picker") + } } }.formStyle(.grouped).scrollDisabled(true).padding(.horizontal) Divider() @@ -72,6 +82,9 @@ struct FileSyncSessionModal: View { set: { if !$0 { createError = nil } } )) {} message: { Text(createError?.description ?? "An unknown error occurred.") + }.sheet(isPresented: $pickingRemote) { + FilePicker(host: workspace!.primaryHost!, outputAbsPath: $remotePath) + .frame(width: 300, height: 400) } } diff --git a/Coder-Desktop/Coder-DesktopTests/FilePickerTests.swift b/Coder-Desktop/Coder-DesktopTests/FilePickerTests.swift new file mode 100644 index 00000000..61bf2196 --- /dev/null +++ b/Coder-Desktop/Coder-DesktopTests/FilePickerTests.swift @@ -0,0 +1,115 @@ +@testable import Coder_Desktop +@testable import CoderSDK +import Mocker +import SwiftUI +import Testing +import ViewInspector + +@MainActor +@Suite(.timeLimit(.minutes(1))) +struct FilePickerTests { + let mockResponse: LSResponse + + init() { + mockResponse = LSResponse( + absolute_path: ["/"], + absolute_path_string: "/", + contents: [ + LSFile(name: "home", absolute_path_string: "/home", is_dir: true), + LSFile(name: "tmp", absolute_path_string: "/tmp", is_dir: true), + LSFile(name: "etc", absolute_path_string: "/etc", is_dir: true), + LSFile(name: "README.md", absolute_path_string: "/README.md", is_dir: false), + ] + ) + } + + @Test + func testLoadError() async throws { + let host = "test-error.coder" + let sut = FilePicker(host: host, outputAbsPath: .constant("")) + let view = sut + + let url = URL(string: "http://\(host):4")! + + let errorMessage = "Connection failed" + Mock( + url: url.appendingPathComponent("/api/v0/list-directory"), + contentType: .json, + statusCode: 500, + data: [.post: errorMessage.data(using: .utf8)!] + ).register() + + try await ViewHosting.host(view) { + try await sut.inspection.inspect { view in + try #expect(await eventually { @MainActor in + let text = try view.find(ViewType.Text.self) + return try text.string().contains("Connection failed") + }) + } + } + } + + @Test + func testSuccessfulFileLoad() async throws { + let host = "test-success.coder" + let sut = FilePicker(host: host, outputAbsPath: .constant("")) + let view = sut + + let url = URL(string: "http://\(host):4")! + + try Mock( + url: url.appendingPathComponent("/api/v0/list-directory"), + statusCode: 200, + data: [.post: Client.encoder.encode(mockResponse)] + ).register() + + try await ViewHosting.host(view) { + try await sut.inspection.inspect { view in + try #expect(await eventually { @MainActor in + _ = try view.find(ViewType.List.self) + return true + }) + _ = try view.find(text: "README.md") + _ = try view.find(text: "home") + let selectButton = try view.find(button: "Select") + #expect(selectButton.isDisabled()) + } + } + } + + @Test + func testDirectoryExpansion() async throws { + let host = "test-expansion.coder" + let sut = FilePicker(host: host, outputAbsPath: .constant("")) + let view = sut + + let url = URL(string: "http://\(host):4")! + + try Mock( + url: url.appendingPathComponent("/api/v0/list-directory"), + statusCode: 200, + data: [.post: Client.encoder.encode(mockResponse)] + ).register() + + try await ViewHosting.host(view) { + try await sut.inspection.inspect { view in + try #expect(await eventually { @MainActor in + _ = try view.find(ViewType.List.self) + return true + }) + + let disclosureGroup = try view.find(ViewType.DisclosureGroup.self) + #expect(view.findAll(ViewType.DisclosureGroup.self).count == 3) + try disclosureGroup.expand() + + // Disclosure group should expand out to 3 more directories + try #expect(await eventually { @MainActor in + return try view.findAll(ViewType.DisclosureGroup.self).count == 6 + }) + } + } + } + + // TODO: The writing of more extensive tests is blocked by ViewInspector, + // as it can't select an item in a list... +} diff --git a/Coder-Desktop/Coder-DesktopTests/Util.swift b/Coder-Desktop/Coder-DesktopTests/Util.swift index 4301cbc4..249aa10b 100644 --- a/Coder-Desktop/Coder-DesktopTests/Util.swift +++ b/Coder-Desktop/Coder-DesktopTests/Util.swift @@ -57,3 +57,28 @@ class MockFileSyncDaemon: FileSyncDaemon { } extension Inspection: @unchecked Sendable, @retroactive InspectionEmissary {} + +public func eventually( + timeout: Duration = .milliseconds(500), + interval: Duration = .milliseconds(10), + condition: @escaping () async throws -> Bool +) async throws -> Bool { + let endTime = ContinuousClock.now.advanced(by: timeout) + + var lastError: Error? + + while ContinuousClock.now < endTime { + do { + if try await condition() { return true } + lastError = nil + } catch { + lastError = error + try await Task.sleep(for: interval) + } + } + + if let lastError { + throw lastError + } + return false +} diff --git a/Coder-Desktop/CoderSDK/AgentLS.swift b/Coder-Desktop/CoderSDK/AgentLS.swift new file mode 100644 index 00000000..bd270d97 --- /dev/null +++ b/Coder-Desktop/CoderSDK/AgentLS.swift @@ -0,0 +1,44 @@ +public extension Client { + // The Client's URL MUST be set to that of an accessible agent + func listAgentDirectory(_ req: LSRequest) async throws(ClientError) -> LSResponse { + let res = try await request("/api/v0/list-directory", method: .post, body: req) + guard res.resp.statusCode == 200 else { + throw responseAsError(res) + } + return try decode(LSResponse.self, from: res.data) + } +} + +public struct LSRequest: Sendable, Codable { + // e.g. [], ["repos", "coder"] + public let path: [String] + // Whether the supplied path is relative to the user's home directory, + // or the root directory. + public let relativity: LSRelativity + + public init(path: [String], relativity: LSRelativity) { + self.path = path + self.relativity = relativity + } + + public enum LSRelativity: String, Sendable, Codable { + case root + case home + } +} + +public struct LSResponse: Sendable, Codable { + public let absolute_path: [String] + // e.g. Windows: "C:\\Users\\coder" + // Linux: "/home/coder" + public let absolute_path_string: String + public let contents: [LSFile] +} + +public struct LSFile: Sendable, Codable { + public let name: String + // e.g. "C:\\Users\\coder\\hello.txt" + // "/home/coder/hello.txt" + public let absolute_path_string: String + public let is_dir: Bool +} diff --git a/Coder-Desktop/CoderSDK/Client.swift b/Coder-Desktop/CoderSDK/Client.swift index 239db14a..98e1c8a9 100644 --- a/Coder-Desktop/CoderSDK/Client.swift +++ b/Coder-Desktop/CoderSDK/Client.swift @@ -1,6 +1,6 @@ import Foundation -public struct Client { +public struct Client: Sendable { public let url: URL public var token: String? public var headers: [HTTPHeader] From c148b670146216af62a1fa458a554537fd364e80 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Fri, 4 Apr 2025 14:28:59 +1100 Subject: [PATCH 2/6] review --- .../Coder-Desktop/Views/FileSync/FilePicker.swift | 10 +++++----- Coder-Desktop/CoderSDK/AgentClient.swift | 7 +++++++ Coder-Desktop/CoderSDK/AgentLS.swift | 9 ++++----- 3 files changed, 16 insertions(+), 10 deletions(-) create mode 100644 Coder-Desktop/CoderSDK/AgentClient.swift diff --git a/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift b/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift index 29a1115e..958ca20e 100644 --- a/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift +++ b/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift @@ -78,10 +78,10 @@ class FilePickerModel: ObservableObject { @Published var isLoading: Bool = false @Published var error: ClientError? - let client: Client + let client: AgentClient init(host: String) { - client = Client(url: URL(string: "http://\(host):4")!) + client = AgentClient(agentHost: host) } func loadRoot() { @@ -176,7 +176,7 @@ class FilePickerItemModel: Identifiable, ObservableObject { // This being a binding is pretty important performance-wise, as it's a struct // that would otherwise be recreated every time the the item row is rendered. // Removing the binding results in very noticeable lag when scrolling a file tree. - @Binding var client: Client + @Binding var client: AgentClient @Published var contents: [FilePickerItemModel]? @Published var isLoading = false @@ -197,7 +197,7 @@ class FilePickerItemModel: Identifiable, ObservableObject { init( name: String, - client: Binding, + client: Binding, absolute_path: String, path: [String], dir: Bool = false, @@ -237,7 +237,7 @@ class FilePickerItemModel: Identifiable, ObservableObject { extension LSResponse { @MainActor - func toModels(client: Binding, path: [String]) -> [FilePickerItemModel] { + func toModels(client: Binding, path: [String]) -> [FilePickerItemModel] { contents.compactMap { file in // Filter dotfiles from the picker guard !file.name.hasPrefix(".") else { return nil } diff --git a/Coder-Desktop/CoderSDK/AgentClient.swift b/Coder-Desktop/CoderSDK/AgentClient.swift new file mode 100644 index 00000000..6c2df6aa --- /dev/null +++ b/Coder-Desktop/CoderSDK/AgentClient.swift @@ -0,0 +1,7 @@ +public struct AgentClient: Sendable { + let client: Client + + public init(agentHost: String) { + client = Client(url: URL(string: "http://\(agentHost):4")!) + } +} diff --git a/Coder-Desktop/CoderSDK/AgentLS.swift b/Coder-Desktop/CoderSDK/AgentLS.swift index bd270d97..7110f405 100644 --- a/Coder-Desktop/CoderSDK/AgentLS.swift +++ b/Coder-Desktop/CoderSDK/AgentLS.swift @@ -1,11 +1,10 @@ -public extension Client { - // The Client's URL MUST be set to that of an accessible agent +public extension AgentClient { func listAgentDirectory(_ req: LSRequest) async throws(ClientError) -> LSResponse { - let res = try await request("/api/v0/list-directory", method: .post, body: req) + let res = try await client.request("/api/v0/list-directory", method: .post, body: req) guard res.resp.statusCode == 200 else { - throw responseAsError(res) + throw client.responseAsError(res) } - return try decode(LSResponse.self, from: res.data) + return try client.decode(LSResponse.self, from: res.data) } } From 1dfafa8d73597100e3c813b3cc42597d4f7909ae Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Fri, 4 Apr 2025 14:29:46 +1100 Subject: [PATCH 3/6] review --- .../Coder-Desktop/Views/FileSync/FilePicker.swift | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift b/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift index 958ca20e..e169d405 100644 --- a/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift +++ b/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift @@ -21,7 +21,7 @@ struct FilePicker: View { var body: some View { VStack(spacing: 0) { - if model.isLoading { + if model.rootIsLoading { Spacer() ProgressView() .controlSize(.large) @@ -75,7 +75,7 @@ struct FilePicker: View { @MainActor class FilePickerModel: ObservableObject { @Published var rootFiles: [FilePickerItemModel] = [] - @Published var isLoading: Bool = false + @Published var rootIsLoading: Bool = false @Published var error: ClientError? let client: AgentClient @@ -86,9 +86,9 @@ class FilePickerModel: ObservableObject { func loadRoot() { error = nil - isLoading = true + rootIsLoading = true Task { - defer { isLoading = false } + defer { rootIsLoading = false } do throws(ClientError) { rootFiles = try await client .listAgentDirectory(.init(path: [], relativity: .root)) From a553fb6d1edfb92b255a5bc0bdc281cbf69abedf Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Tue, 8 Apr 2025 13:22:03 +1000 Subject: [PATCH 4/6] rebase --- .../Coder-Desktop/Views/FileSync/FileSyncSessionModal.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Coder-Desktop/Coder-Desktop/Views/FileSync/FileSyncSessionModal.swift b/Coder-Desktop/Coder-Desktop/Views/FileSync/FileSyncSessionModal.swift index fdefab95..7b902f21 100644 --- a/Coder-Desktop/Coder-Desktop/Views/FileSync/FileSyncSessionModal.swift +++ b/Coder-Desktop/Coder-Desktop/Views/FileSync/FileSyncSessionModal.swift @@ -54,8 +54,8 @@ struct FileSyncSessionModal: View { pickingRemote = true } label: { Image(systemName: "folder") - }.disabled(workspace == nil) - .help(workspace == nil ? "Select a workspace first" : "Open File Picker") + }.disabled(remoteHostname == nil) + .help(remoteHostname == nil ? "Select a workspace first" : "Open File Picker") } } }.formStyle(.grouped).scrollDisabled(true).padding(.horizontal) @@ -83,7 +83,7 @@ struct FileSyncSessionModal: View { )) {} message: { Text(createError?.description ?? "An unknown error occurred.") }.sheet(isPresented: $pickingRemote) { - FilePicker(host: workspace!.primaryHost!, outputAbsPath: $remotePath) + FilePicker(host: remoteHostname!, outputAbsPath: $remotePath) .frame(width: 300, height: 400) } } From d0133ceb94d35e936a67c5755c562152db280954 Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Tue, 8 Apr 2025 17:22:09 +1000 Subject: [PATCH 5/6] replace binding with class --- .../Coder-Desktop/Views/FileSync/FilePicker.swift | 15 ++++++--------- Coder-Desktop/CoderSDK/AgentClient.swift | 2 +- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift b/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift index e169d405..b7df1771 100644 --- a/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift +++ b/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift @@ -92,7 +92,7 @@ class FilePickerModel: ObservableObject { do throws(ClientError) { rootFiles = try await client .listAgentDirectory(.init(path: [], relativity: .root)) - .toModels(client: Binding(get: { self.client }, set: { _ in }), path: []) + .toModels(client: client, path: []) } catch { self.error = error } @@ -173,10 +173,7 @@ class FilePickerItemModel: Identifiable, ObservableObject { let absolute_path: String let dir: Bool - // This being a binding is pretty important performance-wise, as it's a struct - // that would otherwise be recreated every time the the item row is rendered. - // Removing the binding results in very noticeable lag when scrolling a file tree. - @Binding var client: AgentClient + let client: AgentClient @Published var contents: [FilePickerItemModel]? @Published var isLoading = false @@ -197,14 +194,14 @@ class FilePickerItemModel: Identifiable, ObservableObject { init( name: String, - client: Binding, + client: AgentClient, absolute_path: String, path: [String], dir: Bool = false, contents: [FilePickerItemModel]? = nil ) { self.name = name - _client = client + self.client = client self.path = path self.dir = dir self.absolute_path = absolute_path @@ -227,7 +224,7 @@ class FilePickerItemModel: Identifiable, ObservableObject { do throws(ClientError) { contents = try await client .listAgentDirectory(.init(path: path, relativity: .root)) - .toModels(client: $client, path: path) + .toModels(client: client, path: path) } catch { self.error = error } @@ -237,7 +234,7 @@ class FilePickerItemModel: Identifiable, ObservableObject { extension LSResponse { @MainActor - func toModels(client: Binding, path: [String]) -> [FilePickerItemModel] { + func toModels(client: AgentClient, path: [String]) -> [FilePickerItemModel] { contents.compactMap { file in // Filter dotfiles from the picker guard !file.name.hasPrefix(".") else { return nil } diff --git a/Coder-Desktop/CoderSDK/AgentClient.swift b/Coder-Desktop/CoderSDK/AgentClient.swift index 6c2df6aa..ecdd3d43 100644 --- a/Coder-Desktop/CoderSDK/AgentClient.swift +++ b/Coder-Desktop/CoderSDK/AgentClient.swift @@ -1,4 +1,4 @@ -public struct AgentClient: Sendable { +public final class AgentClient: Sendable { let client: Client public init(agentHost: String) { From fc587b8ff29ccc1242c091a73422ffb7cdc80aca Mon Sep 17 00:00:00 2001 From: Ethan Dickson Date: Wed, 9 Apr 2025 12:57:16 +1000 Subject: [PATCH 6/6] review --- .../Views/FileSync/FilePicker.swift | 122 ++++++++---------- 1 file changed, 51 insertions(+), 71 deletions(-) diff --git a/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift b/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift index b7df1771..4ee31a62 100644 --- a/Coder-Desktop/Coder-Desktop/Views/FileSync/FilePicker.swift +++ b/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,52 +88,26 @@ 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] { - 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) } @@ -143,29 +115,29 @@ struct FilePickerItem: View { } 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 ) } }