Skip to content

Commit

Permalink
Decouple focus state management from mostRecentWindow
Browse files Browse the repository at this point in the history
Previously, Workspace.mostRecentWindow was the source of focus. This
commit introduces a separate 'focus: LiveFocus' property that is
responsible to track the focused window/emptyWorkspace.

Soon will become possible to do things like 'move-node-to-workspace --window-id 42 focused'
(#186)
Since I don't want this command changing the focus on the workspace,
we need to decouple focus state management from mostRecentWindow

This commit breaks the following tests:
- Test Case '-[AppBundleTests.FocusCommandTest testFocusWrapping]' failed (0.000 seconds).
- Test Case '-[AppBundleTests.FlattenWorkspaceTreeCommandTest testSimple]' failed (0.004 seconds).

I will fix the tests in the next commits
  • Loading branch information
nikitabobko committed Jul 20, 2024
1 parent f3562b4 commit c066b2a
Show file tree
Hide file tree
Showing 23 changed files with 230 additions and 160 deletions.
6 changes: 1 addition & 5 deletions Sources/AppBundle/command/CloseCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,5 @@ struct CloseCommand: Command {

private func successfullyClosedWindow(_ state: CommandMutableState, _ window: Window) {
window.asMacWindow().garbageCollect()
if let focusedWindow {
state.subject = .window(focusedWindow)
} else {
state.subject = .emptyWorkspace(focusedWorkspaceName)
}
state.subject = .focused
}
12 changes: 7 additions & 5 deletions Sources/AppBundle/command/Command.swift
Original file line number Diff line number Diff line change
Expand Up @@ -72,12 +72,14 @@ extension [Command] {
enum CommandSubject: Equatable {
case emptyWorkspace(String)
case window(Window)
static var focused: CommandSubject { focus.asLeaf.asCommandSubject }
}

static var focused: CommandSubject {
if let window = focusedWindow {
return .window(window)
} else {
return .emptyWorkspace(focusedWorkspaceName)
extension EffectiveLeaf {
var asCommandSubject: CommandSubject {
switch focus.asLeaf {
case .window(let w): .window(w)
case .emptyWorkspace(let w): .emptyWorkspace(w.name)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,6 @@ struct FlattenWorkspaceTreeCommand: Command {
for window in windows {
window.bind(to: workspace.rootTilingContainer, adaptiveWeight: 1, index: INDEX_BIND_LAST)
}
return state.subject.windowOrNil?.focus() != false // Preserve focused window
return true
}
}
4 changes: 2 additions & 2 deletions Sources/AppBundle/command/FocusCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ struct FocusCommand: Command {
case .emptyWorkspace(let name):
result = WorkspaceCommand.run(state, name) && result
case .window(let windowToFocus):
result = windowToFocus.focus() && result
result = windowToFocus.focusWindow() && result
}
return result
}
Expand Down Expand Up @@ -96,7 +96,7 @@ private func wrapAroundTheWorkspace(_ state: CommandMutableState, _ direction: C
return state.failCmd(msg: "No window to focus")
}
state.subject = .window(windowToFocus)
return windowToFocus.focus()
return windowToFocus.focusWindow()
}

private func makeFloatingWindowsSeenAsTiling(workspace: Workspace) -> [FloatingWindowData] {
Expand Down
3 changes: 1 addition & 2 deletions Sources/AppBundle/command/WorkspaceBackAndForthCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ struct WorkspaceBackAndForthCommand: Command {

func _run(_ state: CommandMutableState, stdin: String) -> Bool {
check(Thread.current.isMainThread)
guard let previousFocusedWorkspaceName else { return false }
return WorkspaceCommand.run(state, previousFocusedWorkspaceName)
return prevFocusedWorkspace?.focusWorkspace() != nil
}
}
15 changes: 4 additions & 11 deletions Sources/AppBundle/command/WorkspaceCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,12 @@ struct WorkspaceCommand: Command {
}
}
let workspace = Workspace.get(byName: workspaceName)
// todo drop anyLeafWindowRecursive. It must not be necessary
if let window = workspace.mostRecentWindow ?? workspace.anyLeafWindowRecursive { // switch to not empty workspace
state.subject = .window(window)
} else { // switch to empty workspace
check(workspace.isEffectivelyEmpty)
state.subject = .emptyWorkspace(workspaceName)
}
check(workspace.workspaceMonitor.setActiveWorkspace(workspace))
focusedWorkspaceName = workspace.name
return true
let result = workspace.focusWorkspace()
state.subject = .focused
return result
}

public static func run(_ state: CommandMutableState, _ name: String) -> Bool {
public static func run(_ state: CommandMutableState, _ name: String) -> Bool { // todo reduce usages
if let wName = WorkspaceName.parse(name).getOrNil(appendErrorTo: &state.stderr) {
let args = WorkspaceCmdArgs(rawArgs: [], .direct(WTarget.Direct(wName, autoBackAndForth: false)))
return WorkspaceCommand(args: args).run(state)
Expand Down
140 changes: 140 additions & 0 deletions Sources/AppBundle/focus.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,140 @@
import AppKit
import Common

enum EffectiveLeaf {
case window(Window)
case emptyWorkspace(Workspace)
}
extension LiveFocus {
var asLeaf: EffectiveLeaf {
if let windowOrNil { .window(windowOrNil) } else { .emptyWorkspace(workspace) }
}
}

extension Window {
var visualWorkspace: Workspace? { workspace ?? nodeMonitor?.activeWorkspace }
}

/// This object should be only passed around but never memorized
/// Alternative name: ResolvedFocus
struct LiveFocus: AeroAny, Equatable {
let windowOrNil: Window?
var workspace: Workspace

var frozen: FrozenFocus {
return FrozenFocus(
windowId: windowOrNil?.windowId,
workspaceName: workspace.name,
monitorId: workspace.workspaceMonitor.monitorId ?? 0
)
}
}

/// "old", "captured", "frozen in time" Focus
/// It's safe to keep a hard reference to this object.
/// Unlike in LiveFocus, information inside FrozenFocus isn't guaranteed to be self-consistent.
/// window - workspace - monitor relation could change since the object is created
struct FrozenFocus: AeroAny, Equatable {
let windowId: UInt32?
let workspaceName: String
// monitorId is not part of the focus. We keep it here only for 'on-monitor-changed' to work
let monitorId: Int // 0-based

var live: LiveFocus { // Important: don't access focus.monitorId here. monitorId is not part of the focus. Always prefer workspace
let windowId = windowId
let window: Window? = if let windowId {
isUnitTest
? Workspace.all.flatMap { $0.allLeafWindowsRecursive }.first(where: { $0.windowId == windowId })
: MacWindow.allWindowsMap[windowId]
} else {
nil
}
if let window, let ws = window.visualWorkspace {
return LiveFocus(windowOrNil: window, workspace: ws)
}
let workspace = Workspace.get(byName: workspaceName)
return LiveFocus(windowOrNil: workspace.mostRecentWindow, workspace: workspace)
}
}

var _focus: FrozenFocus = {
// It's fine to call *Inaccurate during startup
let monitor = focusedMonitorInaccurate ?? mainMonitor
return FrozenFocus(windowId: nil, workspaceName: monitor.activeWorkspace.name, monitorId: monitor.monitorId ?? 0)
}()
var focus: LiveFocus { _focus.live }

private func setFocus(to newFocus: LiveFocus) -> Bool {
if _focus == newFocus.frozen { return true }
let oldFocus = focus
// Normalize mruWindow when focus away from a workspace
if oldFocus.workspace != newFocus.workspace {
oldFocus.windowOrNil?.markAsMostRecentChild()
}

_focus = newFocus.frozen
let status = newFocus.workspace.workspaceMonitor.setActiveWorkspace(newFocus.workspace)

newFocus.windowOrNil?.markAsMostRecentChild()
return status
}
extension Window {
func focusWindow() -> Bool {
if let ws = self.visualWorkspace {
return setFocus(to: LiveFocus(windowOrNil: self, workspace: ws))
} else {
// todo We should also exit-native-hidden/unminimize[/exit-native-fullscreen?] window if we want to fix ID-B6E178F2
// and retry to focus the window. Otherwise, it's not possible to focus minimized/hidden windows
return false
}
}
}
extension Workspace {
func focusWorkspace() -> Bool {
if let w = mostRecentWindow {
return w.focusWindow()
} else {
check(anyLeafWindowRecursive == nil)
return setFocus(to: LiveFocus(windowOrNil: nil, workspace: self))
}
}
}

private var _lastKnownFocus: FrozenFocus = _focus

// Used by workspace-back-and-forth
var _prevFocusedWorkspaceName: String? = nil
var prevFocusedWorkspace: Workspace? { _prevFocusedWorkspaceName.map { Workspace.get(byName: $0) } }

// Used by focus-back-and-forth
var _prevFocus: FrozenFocus? = nil
var prevFocus: LiveFocus? { _prevFocus?.live }

func checkOnFocusChangedCallbacks() {
let focus = focus.frozen
if focus.workspaceName != _lastKnownFocus.workspaceName {
onWorkspaceChanged(_lastKnownFocus.workspaceName, focus.workspaceName)
_prevFocusedWorkspaceName = _lastKnownFocus.workspaceName
}
if focus.monitorId != _lastKnownFocus.monitorId {
// todo onMonitorChanged
}
if focus != _lastKnownFocus {
// todo onFocusChanged
_prevFocus = _lastKnownFocus
}
_lastKnownFocus = focus
}

private func onWorkspaceChanged(_ oldWorkspace: String, _ newWorkspace: String) {
if let exec = config.execOnWorkspaceChange.first {
let process = Process()
process.executableURL = URL(filePath: exec)
process.arguments = Array(config.execOnWorkspaceChange.dropFirst())
var environment = config.execConfig.envVariables
environment["AEROSPACE_FOCUSED_WORKSPACE"] = newWorkspace
environment["AEROSPACE_PREV_WORKSPACE"] = oldWorkspace
process.environment = environment
Result { try process.run() }.getOrThrow() // todo It's not perfect to fail here
}
}
4 changes: 2 additions & 2 deletions Sources/AppBundle/focusCache.swift
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
private var lastKnownNativeFocusedWindowId: UInt32? = nil

/// The data should flow (from nativeFocused to focusedWindow) and
/// The data should flow (from nativeFocused to focused) and
/// (from nativeFocused to lastKnownNativeFocusedWindowId)
/// Alternative names: takeFocusFromMacOs, syncFocusFromMacOs
func updateFocusCache(_ nativeFocused: Window?) {
if nativeFocused?.windowId != lastKnownNativeFocusedWindowId {
_ = nativeFocused?.focus()
_ = nativeFocused?.focusWindow()
lastKnownNativeFocusedWindowId = nativeFocused?.windowId
}
}
54 changes: 0 additions & 54 deletions Sources/AppBundle/focused.swift

This file was deleted.

17 changes: 17 additions & 0 deletions Sources/AppBundle/getNativeFocusedWindow.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import AppKit
import Common

var appForTests: AbstractApp? = nil

private var focusedApp: AbstractApp? {
if isUnitTest {
return appForTests
} else {
check(appForTests == nil)
return NSWorkspace.shared.frontmostApplication?.macApp
}
}

func getNativeFocusedWindow(startup: Bool) -> Window? {
focusedApp?.getFocusedWindow(startup: startup)
}
17 changes: 4 additions & 13 deletions Sources/AppBundle/layout/refresh.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,21 +13,21 @@ func refreshSession<T>(startup: Bool = false, forceFocus: Bool = false, body: ()
let nativeFocused = getNativeFocusedWindow(startup: startup)
if let nativeFocused { debugWindowsIfRecording(nativeFocused) }
updateFocusCache(nativeFocused)
let focusBefore = focusedWindow
let focusBefore = focus.windowOrNil

refreshModel()
let result = body()
refreshModel()

let focusAfter = focusedWindow
let focusAfter = focus.windowOrNil

if startup {
smartLayoutAtStartup()
}

if TrayMenuModel.shared.isEnabled {
if forceFocus || focusBefore != focusAfter {
focusedWindow?.nativeFocus() // syncFocusToMacOs
focusAfter?.nativeFocus() // syncFocusToMacOs
}

updateTrayText()
Expand All @@ -43,7 +43,7 @@ func refreshAndLayout(startup: Bool = false) {

func refreshModel() {
gc()
refreshFocusedWorkspaceBasedOnFocusedWindow()
checkOnFocusChangedCallbacks()
normalizeContainers()
}

Expand All @@ -58,15 +58,6 @@ func refreshObs(_ obs: AXObserver, ax: AXUIElement, notif: CFString, data: Unsaf
refreshAndLayout()
}

private func refreshFocusedWorkspaceBasedOnFocusedWindow() { // todo drop. It should no longer be necessary
if let focusedWindow = focusedWindow, let monitor = focusedWindow.nodeMonitor {
// todo it's rather refresh focused monitor
let focusedWorkspace: Workspace = monitor.activeWorkspace
check(focusedWorkspace.workspaceMonitor.setActiveWorkspace(focusedWorkspace))
focusedWorkspaceName = focusedWorkspace.name
}
}

private func layoutWorkspaces() {
// to reduce flicker, first unhide visible workspaces, then hide invisible ones
for workspace in Workspace.all where workspace.isVisible {
Expand Down
1 change: 0 additions & 1 deletion Sources/AppBundle/mouse/moveWithMouse.swift
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ private func moveTilingWindow(_ window: Window) {
} else if let swapTarget {
swapWindows(window, swapTarget)
}
_ = window.focus() // Keep the window focused
}

func swapWindows(_ window1: Window, _ window2: Window) {
Expand Down
9 changes: 5 additions & 4 deletions Sources/AppBundle/tree/MacWindow.swift
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,19 @@ final class MacWindow: Window, CustomStringConvertible {
return
}
let parent = unbindFromParent().parent
let workspace = parent.workspace?.name
let workspace = parent.workspace
for obs in axObservers {
AXObserverRemoveNotification(obs.obs, obs.ax, obs.notif)
}
axObservers = []
// todo the if is an approximation to filter out cases when window just closed itself (or was killed remotely)
// we might want to track the time of the latest workspace switch to make the approximation more accurate
if let workspace, workspace == previousFocusedWorkspaceName || workspace == focusedWorkspaceName {
let focus = focus
if let workspace, workspace == focus.workspace || workspace == prevFocusedWorkspace {
switch parent.cases {
case .tilingContainer, .workspace, .macosInvisibleWindowsContainer, .macosFullscreenWindowsContainer:
refreshSession(forceFocus: focusedWindow?.app != app) {
_ = WorkspaceCommand.run(.focused, workspace)
refreshSession(forceFocus: focus.windowOrNil?.app != app) {
_ = workspace.focusWorkspace()
}
case .macosPopupWindowsContainer:
break // Don't switch back on popup destruction
Expand Down
Loading

0 comments on commit c066b2a

Please sign in to comment.