From e16911b36371c22e2b6062806c1164d14a3230de Mon Sep 17 00:00:00 2001 From: Brent Simmons Date: Sat, 4 Nov 2017 22:51:14 -0700 Subject: [PATCH] =?UTF-8?q?Make=20deleting=20work.=20Can=E2=80=99t=20undo?= =?UTF-8?q?=20yet.=20But=20now=20everything=20is=20messed-up=20because=20o?= =?UTF-8?q?f=20an=20AnyHashable=20casting=20bug.=20Don=E2=80=99t=20run=20t?= =?UTF-8?q?his=20build.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Commands/DeleteFromSidebarCommand.swift | 154 ++++++++++++------ .../Sidebar/SidebarViewController.swift | 12 +- .../Timeline/TimelineViewController.swift | 34 +--- Frameworks/Account/Account.swift | 4 +- Frameworks/Account/Container.swift | 35 +++- Frameworks/Account/Folder.swift | 4 +- .../RSCore/RSCore/UndoableCommand.swift | 38 +++++ 7 files changed, 193 insertions(+), 88 deletions(-) diff --git a/Commands/DeleteFromSidebarCommand.swift b/Commands/DeleteFromSidebarCommand.swift index 6b9733c57..230000e0a 100644 --- a/Commands/DeleteFromSidebarCommand.swift +++ b/Commands/DeleteFromSidebarCommand.swift @@ -14,14 +14,7 @@ import Data final class DeleteFromSidebarCommand: UndoableCommand { - private struct ActionName { - static let deleteFeed = NSLocalizedString("Delete Feed", comment: "command") - static let deleteFeeds = NSLocalizedString("Delete Feeds", comment: "command") - static let deleteFolder = NSLocalizedString("Delete Folder", comment: "command") - static let deleteFolders = NSLocalizedString("Delete Folders", comment: "command") - static let deleteFeedsAndFolders = NSLocalizedString("Delete Feeds and Folders", comment: "command") - } - + let undoManager: UndoManager let undoActionName: String var redoActionName: String { get { @@ -29,49 +22,40 @@ final class DeleteFromSidebarCommand: UndoableCommand { } } - let undoManager: UndoManager - + private let itemSpecifiers: [SidebarItemSpecifier] + init?(nodesToDelete: [Node], undoManager: UndoManager) { - var numberOfFeeds = 0 - var numberOfFolders = 0 - - for node in nodesToDelete { - if let _ = node.representedObject as? Feed { - numberOfFeeds += 1 - } - else if let _ = node.representedObject as? Folder { - numberOfFolders += 1 - } - else { - return nil // Delete only Feeds and Folders. - } - } - - if numberOfFeeds < 1 && numberOfFolders < 1 { - return nil - } - - if numberOfFolders < 1 { - self.undoActionName = numberOfFeeds == 1 ? ActionName.deleteFeed : ActionName.deleteFeeds - } - else if numberOfFeeds < 1 { - self.undoActionName = numberOfFolders == 1 ? ActionName.deleteFolder : ActionName.deleteFolders - } - else { - self.undoActionName = ActionName.deleteFeedsAndFolders - } - + guard DeleteFromSidebarCommand.canDelete(nodesToDelete) else { + return nil + } + guard let actionName = DeleteActionName.name(for: nodesToDelete) else { + return nil + } + + self.undoActionName = actionName self.undoManager = undoManager + + let itemSpecifiers = nodesToDelete.flatMap{ SidebarItemSpecifier(node: $0) } + guard !itemSpecifiers.isEmpty else { + return nil + } + self.itemSpecifiers = itemSpecifiers } func perform() { + BatchUpdate.shared.perform { + itemSpecifiers.forEach { $0.delete() } + } registerUndo() } func undo() { + BatchUpdate.shared.perform { + + } registerRedo() } @@ -103,19 +87,34 @@ final class DeleteFromSidebarCommand: UndoableCommand { private struct SidebarItemSpecifier { - weak var account: Account? - let folder: Folder? - let feed: Feed? - let path: ContainerPath - + private weak var account: Account? + private let parentFolder: Folder? + private let folder: Folder? + private let feed: Feed? + private let path: ContainerPath + + private var container: Container? { + get { + if let parentFolder = parentFolder { + return parentFolder + } + if let account = account { + return account + } + return nil + } + } + init?(node: Node) { var account: Account? - if let feed = node.representedObject as? Feed { + self.parentFolder = node.parentFolder() + + if let feed = node.representedObject as? Feed { self.feed = feed self.folder = nil - account = feed.account + account = feed.account } else if let folder = node.representedObject as? Folder { self.feed = nil @@ -132,10 +131,38 @@ private struct SidebarItemSpecifier { self.account = account! self.path = ContainerPath(account: account!, folders: node.containingFolders()) } + + func delete() { + + guard let container = container else { + return + } + + if let feed = feed { + container.deleteFeed(feed) + } + else if let folder = folder { + container.deleteFolder(folder) + } + } } private extension Node { + func parentFolder() -> Folder? { + + guard let parentNode = self.parent else { + return nil + } + if parentNode.isRoot { + return nil + } + if let folder = parentNode.representedObject as? Folder { + return folder + } + return nil + } + func containingFolders() -> [Folder] { var nomad = self.parent @@ -156,3 +183,38 @@ private extension Node { } +private struct DeleteActionName { + + private static let deleteFeed = NSLocalizedString("Delete Feed", comment: "command") + private static let deleteFeeds = NSLocalizedString("Delete Feeds", comment: "command") + private static let deleteFolder = NSLocalizedString("Delete Folder", comment: "command") + private static let deleteFolders = NSLocalizedString("Delete Folders", comment: "command") + private static let deleteFeedsAndFolders = NSLocalizedString("Delete Feeds and Folders", comment: "command") + + static func name(for nodes: [Node]) -> String? { + + var numberOfFeeds = 0 + var numberOfFolders = 0 + + for node in nodes { + if let _ = node.representedObject as? Feed { + numberOfFeeds += 1 + } + else if let _ = node.representedObject as? Folder { + numberOfFolders += 1 + } + else { + return nil // Delete only Feeds and Folders. + } + } + + if numberOfFolders < 1 { + return numberOfFeeds == 1 ? deleteFeed : deleteFeeds + } + if numberOfFeeds < 1 { + return numberOfFolders == 1 ? deleteFolder : deleteFolders + } + + return deleteFeedsAndFolders + } +} diff --git a/Evergreen/MainWindow/Sidebar/SidebarViewController.swift b/Evergreen/MainWindow/Sidebar/SidebarViewController.swift index 334f2e3e8..07172c39d 100644 --- a/Evergreen/MainWindow/Sidebar/SidebarViewController.swift +++ b/Evergreen/MainWindow/Sidebar/SidebarViewController.swift @@ -12,13 +12,14 @@ import Data import Account import RSCore -@objc class SidebarViewController: NSViewController, NSOutlineViewDelegate, NSOutlineViewDataSource { +@objc class SidebarViewController: NSViewController, NSOutlineViewDelegate, NSOutlineViewDataSource, UndoableCommandRunner { @IBOutlet var outlineView: SidebarOutlineView! let treeControllerDelegate = SidebarTreeControllerDelegate() lazy var treeController: TreeController = { TreeController(delegate: treeControllerDelegate) }() + var undoableCommands = [UndoableCommand]() //MARK: NSViewController @@ -71,15 +72,18 @@ import RSCore } let nodesToDelete = treeController.normalizedSelectedNodes(selectedNodes) + + guard let undoManager = undoManager, let deleteCommand = DeleteFromSidebarCommand(nodesToDelete: nodesToDelete, undoManager: undoManager) else { + return + } + let selectedRows = outlineView.selectedRowIndexes outlineView.beginUpdates() outlineView.removeItems(at: selectedRows, inParent: nil, withAnimation: [.slideDown]) outlineView.endUpdates() - BatchUpdate.shared.perform { - deleteItemsForNodes(nodesToDelete) - } + runCommand(deleteCommand) } // MARK: Navigation diff --git a/Evergreen/MainWindow/Timeline/TimelineViewController.swift b/Evergreen/MainWindow/Timeline/TimelineViewController.swift index 5242f99e3..7383a5908 100644 --- a/Evergreen/MainWindow/Timeline/TimelineViewController.swift +++ b/Evergreen/MainWindow/Timeline/TimelineViewController.swift @@ -12,7 +12,7 @@ import RSTextDrawing import Data import Account -class TimelineViewController: NSViewController, KeyboardDelegate { +class TimelineViewController: NSViewController, KeyboardDelegate, UndoableCommandRunner { @IBOutlet var tableView: TimelineTableView! @@ -22,7 +22,7 @@ class TimelineViewController: NSViewController, KeyboardDelegate { } } - private var undoableCommands = [UndoableCommand]() + var undoableCommands = [UndoableCommand]() private var cellAppearance: TimelineCellAppearance! private var showFeedNames = false private var didRegisterForNotifications = false @@ -319,36 +319,6 @@ class TimelineViewController: NSViewController, KeyboardDelegate { } -// MARK: - Undoable Commands - -private extension TimelineViewController { - - func runCommand(_ undoableCommand: UndoableCommand) { - - pushUndoableCommand(undoableCommand) - undoableCommand.perform() - } - - func pushUndoableCommand(_ undoableCommand: UndoableCommand) { - - undoableCommands += [undoableCommand] - } - - func clearUndoableCommands() { - - // When the timeline is reloaded and the list of articles changes, - // undoable commands should be dropped — otherwise things like - // Redo Mark Read are ambiguous. (Do they apply to the previous articles - // or to the current articles?) - - guard let undoManager = undoManager else { - return - } - undoableCommands.forEach { undoManager.removeAllActions(withTarget: $0) } - undoableCommands = [UndoableCommand]() - } -} - // MARK: - NSTableViewDataSource extension TimelineViewController: NSTableViewDataSource { diff --git a/Frameworks/Account/Account.swift b/Frameworks/Account/Account.swift index 331d5a377..e3e44d9cd 100644 --- a/Frameworks/Account/Account.swift +++ b/Frameworks/Account/Account.swift @@ -36,7 +36,7 @@ public enum AccountType: Int { public final class Account: DisplayNameProvider, UnreadCountProvider, Container, Hashable { - public struct UserInfoKey { + public struct UserInfoKey { public static let newArticles = "newArticles" // AccountDidDownloadArticles public static let updatedArticles = "updatedArticles" // AccountDidDownloadArticles public static let statuses = "statuses" // StatusesDidChange @@ -279,7 +279,7 @@ public final class Account: DisplayNameProvider, UnreadCountProvider, Container, return false // TODO } - public func importOPML(_ opmlDocument: RSOPMLDocument) { + public func importOPML(_ opmlDocument: RSOPMLDocument) { guard let children = opmlDocument.children else { return diff --git a/Frameworks/Account/Container.swift b/Frameworks/Account/Container.swift index a516a87c4..31bf9d810 100644 --- a/Frameworks/Account/Container.swift +++ b/Frameworks/Account/Container.swift @@ -16,9 +16,9 @@ extension NSNotification.Name { public static let ChildrenDidChange = Notification.Name("ChildrenDidChange") } -public protocol Container { +public protocol Container: class { - var children: [AnyObject] { get } + var children: [AnyObject] { get set } func hasAtLeastOneFeed() -> Bool func objectIsChild(_ object: AnyObject) -> Bool @@ -26,6 +26,9 @@ public protocol Container { func hasChildFolder(with: String) -> Bool func childFolder(with: String) -> Folder? + func deleteFeed(_ feed: Feed) + func deleteFolder(_ folder: Folder) + //Recursive func flattenedFeeds() -> Set func hasFeed(with feedID: String) -> Bool @@ -172,6 +175,34 @@ public extension Container { return nil } + func indexOf(_ object: T) -> Int? { + + return children.index(where: { (child) -> Bool in + if let oneObject = child as? T { + return oneObject == object + } + return false + }) + } + + func delete(_ object: T) { + + if let index = indexOf(object) { + children.remove(at: index) + postChildrenDidChangeNotification() + } + } + + func deleteFeed(_ feed: Feed) { + + return delete(feed) + } + + func deleteFolder(_ folder: Folder) { + + return delete(folder) + } + func postChildrenDidChangeNotification() { NotificationCenter.default.post(name: .ChildrenDidChange, object: self) diff --git a/Frameworks/Account/Folder.swift b/Frameworks/Account/Folder.swift index 92a61d0bd..8bb905555 100644 --- a/Frameworks/Account/Folder.swift +++ b/Frameworks/Account/Folder.swift @@ -10,7 +10,7 @@ import Foundation import Data import RSCore -public final class Folder: DisplayNameProvider, Container, UnreadCountProvider, Hashable { +public final class Folder: DisplayNameProvider, Container, UnreadCountProvider, Hashable { public weak var account: Account? public var children = [AnyObject]() @@ -133,7 +133,7 @@ public final class Folder: DisplayNameProvider, Container, UnreadCountProvider, } return true } - + // MARK: Notifications @objc func unreadCountDidChange(_ note: Notification) { diff --git a/Frameworks/RSCore/RSCore/UndoableCommand.swift b/Frameworks/RSCore/RSCore/UndoableCommand.swift index c1caa2183..1a79051b5 100644 --- a/Frameworks/RSCore/RSCore/UndoableCommand.swift +++ b/Frameworks/RSCore/RSCore/UndoableCommand.swift @@ -36,3 +36,41 @@ extension UndoableCommand { } } } + +// Useful for view controllers. + +public protocol UndoableCommandRunner: class { + + var undoableCommands: [UndoableCommand] { get set } + var undoManager: UndoManager? { get } + + func runCommand(_ undoableCommand: UndoableCommand) + func clearUndoableCommands() +} + +public extension UndoableCommandRunner { + + func runCommand(_ undoableCommand: UndoableCommand) { + + pushUndoableCommand(undoableCommand) + undoableCommand.perform() + } + + func pushUndoableCommand(_ undoableCommand: UndoableCommand) { + + undoableCommands += [undoableCommand] + } + + func clearUndoableCommands() { + + // Useful, for example, when timeline is reloaded and the list of articles changes. + // Otherwise things like Redo Mark Read are ambiguous. + // (Do they apply to the previous articles or to the current articles?) + + guard let undoManager = undoManager else { + return + } + undoableCommands.forEach { undoManager.removeAllActions(withTarget: $0) } + undoableCommands = [UndoableCommand]() + } +}