From 3da11ff1da7e6b2cca3f411e787deb3159117883 Mon Sep 17 00:00:00 2001 From: boris Date: Sat, 1 Aug 2026 20:59:05 +0300 Subject: [PATCH] Fix #1729: Keep status bar cursor label in sync on open Reuse the EditorInstance stored in tabs, avoid clearing caret on nil SourceEditor updates, and bind the status bar label by object identity so Line/Col appears without a tab switch. --- .../Editor/Models/Editor/Editor.swift | 34 ++++-- .../Editor/Models/EditorInstance.swift | 20 +++- .../Features/Editor/Views/CodeFileView.swift | 6 +- .../StatusBarCursorPositionLabel.swift | 110 ++++++++++++------ .../Features/Editor/EditorTabReuseTests.swift | 63 ++++++++++ .../StatusBarCursorPositionLabelTests.swift | 72 ++++++++++++ .../ProjectNavigatorUITests.swift | 50 ++++++++ .../App Window/StatusBarView.md | 7 ++ 8 files changed, 318 insertions(+), 44 deletions(-) create mode 100644 CodeEditTests/Features/Editor/EditorTabReuseTests.swift create mode 100644 CodeEditTests/Features/StatusBar/StatusBarCursorPositionLabelTests.swift diff --git a/CodeEdit/Features/Editor/Models/Editor/Editor.swift b/CodeEdit/Features/Editor/Models/Editor/Editor.swift index 782b956b71..fc8963443e 100644 --- a/CodeEdit/Features/Editor/Models/Editor/Editor.swift +++ b/CodeEdit/Features/Editor/Models/Editor/Editor.swift @@ -185,8 +185,11 @@ final class Editor: ObservableObject, Identifiable { let item = EditorInstance(workspace: workspace, file: file) // Item is already opened in a tab. guard !tabs.contains(item) || !asTemporary else { - selectedTab = item - addToHistory(item) + // Reuse the instance already in `tabs`. ``EditorInstance`` equality is by file, so a fresh + // instance would leave the editor and status bar observing different objects (#1729). + let existing = tabs.first(where: { $0.file == file }) ?? item + selectedTab = existing + addToHistory(existing) return } @@ -203,7 +206,7 @@ final class Editor: ObservableObject, Identifiable { openTab(file: item.file) case (.none, true): openTab(file: item.file) - temporaryTab = item + temporaryTab = selectedTab case (.none, false): openTab(file: item.file) } @@ -230,7 +233,7 @@ final class Editor: ObservableObject, Identifiable { } else { // If we couldn't find the current temporary tab (invalid state) we should still do *something* openTab(file: newItem.file) - temporaryTab = newItem + temporaryTab = selectedTab } } @@ -240,6 +243,21 @@ final class Editor: ObservableObject, Identifiable { /// - index: Index where the tab needs to be added. If nil, it is added to the back. /// - fromHistory: Indicates whether the tab has been opened from going back in history. func openTab(file: CEWorkspaceFile, at index: Int? = nil, fromHistory: Bool = false) { + // Always select the instance that lives in `tabs` so cursor publishers stay shared with the editor view. + if let existing = tabs.first(where: { $0.file == file }) { + selectedTab = existing + if !fromHistory { + clearFuture() + addToHistory(existing) + } + do { + try openFile(item: existing) + } catch { + logger.error("Error opening file: \(error)") + } + return + } + let item = Tab(workspace: workspace, file: file) if let index { tabs.insert(item, at: index) @@ -251,13 +269,15 @@ final class Editor: ObservableObject, Identifiable { } } - selectedTab = item + // `tabs` may keep a previously inserted equal element; bind selection to that stored instance. + let stored = tabs.first(where: { $0.file == file }) ?? item + selectedTab = stored if !fromHistory { clearFuture() - addToHistory(item) + addToHistory(stored) } do { - try openFile(item: item) + try openFile(item: stored) } catch { logger.error("Error opening file: \(error)") } diff --git a/CodeEdit/Features/Editor/Models/EditorInstance.swift b/CodeEdit/Features/Editor/Models/EditorInstance.swift index fd11333bb7..43307a52e2 100644 --- a/CodeEdit/Features/Editor/Models/EditorInstance.swift +++ b/CodeEdit/Features/Editor/Models/EditorInstance.swift @@ -14,6 +14,8 @@ import CodeEditSourceEditor /// A single instance of an editor in a group with a published ``EditorInstance/cursorPositions`` variable to publish /// the user's current location in a file. class EditorInstance: ObservableObject, Hashable { + private static let defaultCursorPositions = [CursorPosition(line: 1, column: 1)] + /// The file presented in this editor instance. let file: CEWorkspaceFile @@ -43,9 +45,12 @@ class EditorInstance: ObservableObject, Hashable { replaceText = workspace?.searchState?.replaceText replaceTextSubject = PassthroughSubject() - self.cursorPositions = ( - cursorPositions ?? editorState?.editorCursorPositions ?? [CursorPosition(line: 1, column: 1)] - ) + // Prefer an explicit position, then a non-empty restored position, else a caret at 1:1. + // Empty restored arrays must not wipe the default — the status bar would show nothing. + let restoredCursorPositions = editorState?.editorCursorPositions + self.cursorPositions = cursorPositions + ?? (restoredCursorPositions?.isEmpty == false ? restoredCursorPositions : nil) + ?? Self.defaultCursorPositions self.scrollPosition = editorState?.scrollPosition // Setup listeners @@ -124,6 +129,9 @@ class EditorInstance: ObservableObject, Hashable { /// Translates ranges (eg: from a cursor position) to other information like the number of lines in a range. class RangeTranslator: TextViewCoordinator { + /// Emits when the text view controller becomes visible so observers can refresh resolved cursor labels. + let controllerDidAppearSubject = PassthroughSubject() + private weak var textViewController: TextViewController? init() { } @@ -136,6 +144,7 @@ class EditorInstance: ObservableObject, Hashable { if controller.isEditable && controller.isSelectable { controller.view.window?.makeFirstResponder(controller.textView) } + controllerDidAppearSubject.send() } func destroy() { @@ -158,6 +167,11 @@ class EditorInstance: ObservableObject, Hashable { return (endTextLine.index - startTextLine.index) + 1 } + /// Resolves a cursor position through the text view when available; otherwise returns the input unchanged. + func resolveCursorPosition(_ cursorPosition: CursorPosition) -> CursorPosition { + textViewController?.resolveCursorPosition(cursorPosition) ?? cursorPosition + } + func moveLinesUp() { guard let controller = textViewController else { return } controller.moveLinesUp() diff --git a/CodeEdit/Features/Editor/Views/CodeFileView.swift b/CodeEdit/Features/Editor/Views/CodeFileView.swift index f22f6cce3d..26024af672 100644 --- a/CodeEdit/Features/Editor/Views/CodeFileView.swift +++ b/CodeEdit/Features/Editor/Views/CodeFileView.swift @@ -158,7 +158,11 @@ struct CodeFileView: View { ) }, set: { newState in - editorInstance.cursorPositions = newState.cursorPositions ?? [] + // Keep the last known caret when SourceEditor omits cursor state (e.g. scroll-only updates). + // Writing `?? []` cleared the status bar until the next tab switch (#1729). + if let cursorPositions = newState.cursorPositions { + editorInstance.cursorPositions = cursorPositions + } editorInstance.scrollPosition = newState.scrollPosition editorInstance.findText = newState.findText editorInstance.findTextSubject.send(newState.findText) diff --git a/CodeEdit/Features/StatusBar/Views/StatusBarItems/StatusBarCursorPositionLabel.swift b/CodeEdit/Features/StatusBar/Views/StatusBarItems/StatusBarCursorPositionLabel.swift index 6939fca4ec..d43a19e70e 100644 --- a/CodeEdit/Features/StatusBar/Views/StatusBarItems/StatusBarCursorPositionLabel.swift +++ b/CodeEdit/Features/StatusBar/Views/StatusBarItems/StatusBarCursorPositionLabel.swift @@ -23,7 +23,9 @@ struct StatusBarCursorPositionLabel: View { var body: some View { Group { if let currentTab = tab { + // Identity by object, not file equality — ``EditorInstance`` compares equal by file. LineLabel(editorInstance: currentTab) + .id(ObjectIdentifier(currentTab)) } else { Text("").accessibilityLabel("No Selection") } @@ -38,6 +40,65 @@ struct StatusBarCursorPositionLabel: View { .onReceive(editorManager.tabBarTabIdSubject) { _ in updateSource() } + .onReceive(editorManager.$activeEditor) { _ in + updateSource() + } + .onChange(of: editorManager.activeEditor.selectedTab) { _, newTab in + tab = newTab + } + } + + /// Formats the status-bar cursor label from cursor positions. + /// + /// Extracted for unit testing. When line/column are unresolved (`<= 0`), falls back to a safe + /// `Line: 1 Col: 1` caret label (or character offset when Option is held). + static func formatLabel( + cursorPositions: [CursorPosition], + optionKeyPressed: Bool, + linesInRange: (NSRange) -> Int + ) -> String { + if cursorPositions.isEmpty { + return "" + } + + // More than one selection, display the number of selections. + if cursorPositions.count > 1 { + return "\(cursorPositions.count) selected ranges" + } + + let position = cursorPositions[0] + + // If the selection is more than just a cursor, return the length. + if position.range.length > 0 { + // When the option key is pressed display the character range. + if optionKeyPressed { + return "Char: \(position.range.location) Len: \(position.range.length)" + } + + let lineCount = linesInRange(position.range) + + if lineCount > 1 { + return "\(lineCount) lines" + } + + return "\(position.range.length) characters" + } + + // When the option key is pressed display the character offset. + if optionKeyPressed { + if position.range != .notFound { + return "Char: \(position.range.location) Len: 0" + } + return "Char: 0 Len: 0" + } + + // Unresolved line/column (range-only positions from SourceEditor) until the controller fills them in. + if position.start.line <= 0 || position.start.column <= 0 { + return "Line: 1 Col: 1" + } + + // When there's a single cursor, display the line and column. + return "Line: \(position.start.line) Col: \(position.start.column)" } struct LineLabel: View { @@ -50,10 +111,11 @@ struct StatusBarCursorPositionLabel: View { let editorInstance: EditorInstance - @State private var cursorPositions: [CursorPosition] = [] + @State private var cursorPositions: [CursorPosition] init(editorInstance: EditorInstance) { self.editorInstance = editorInstance + self._cursorPositions = State(initialValue: editorInstance.cursorPositions) } var body: some View { @@ -61,9 +123,17 @@ struct StatusBarCursorPositionLabel: View { .font(statusBarViewModel.statusBarFont) .foregroundColor(foregroundColor) .lineLimit(1) + .onAppear { + cursorPositions = editorInstance.cursorPositions + } .onReceive(editorInstance.$cursorPositions) { newValue in self.cursorPositions = newValue } + .onReceive(editorInstance.rangeTranslator.controllerDidAppearSubject) { _ in + self.cursorPositions = editorInstance.cursorPositions.map { + editorInstance.rangeTranslator.resolveCursorPosition($0) + } + } } private var foregroundColor: Color { @@ -84,38 +154,12 @@ struct StatusBarCursorPositionLabel: View { /// Create a label string for cursor positions. /// - Returns: A string describing the user's location in a document. func getLabel() -> String { - if cursorPositions.isEmpty { - return "" - } - - // More than one selection, display the number of selections. - if cursorPositions.count > 1 { - return "\(cursorPositions.count) selected ranges" - } - - // If the selection is more than just a cursor, return the length. - if cursorPositions[0].range.length > 0 { - // When the option key is pressed display the character range. - if modifierKeys.contains(.option) { - return "Char: \(cursorPositions[0].range.location) Len: \(cursorPositions[0].range.length)" - } - - let lineCount = getLines(cursorPositions[0].range) - - if lineCount > 1 { - return "\(lineCount) lines" - } - - return "\(cursorPositions[0].range.length) characters" - } - - // When the option key is pressed display the character offset. - if modifierKeys.contains(.option) { - return "Char: \(cursorPositions[0].range.location) Len: 0" - } - - // When there's a single cursor, display the line and column. - return "Line: \(cursorPositions[0].start.line) Col: \(cursorPositions[0].start.column)" + let resolved = cursorPositions.map { editorInstance.rangeTranslator.resolveCursorPosition($0) } + return StatusBarCursorPositionLabel.formatLabel( + cursorPositions: resolved, + optionKeyPressed: modifierKeys.contains(.option), + linesInRange: getLines + ) } } } diff --git a/CodeEditTests/Features/Editor/EditorTabReuseTests.swift b/CodeEditTests/Features/Editor/EditorTabReuseTests.swift new file mode 100644 index 0000000000..a7237964c3 --- /dev/null +++ b/CodeEditTests/Features/Editor/EditorTabReuseTests.swift @@ -0,0 +1,63 @@ +// +// EditorTabReuseTests.swift +// CodeEditTests +// +// Created by Boris Serzhanovich on 1/8/26. +// + +import Testing +import Foundation +import OrderedCollections +import CodeEditSourceEditor +@testable import CodeEdit + +@Suite("Editor tab instance reuse") +struct EditorTabReuseTests { + + @Test + @MainActor + func reopeningSameFileReusesEditorInstance() throws { + try withTempDir { dir in + let fileURL = dir.appending(path: "Sample.swift") + try "print(1)\n".write(to: fileURL, atomically: true, encoding: .utf8) + let file = CEWorkspaceFile(url: fileURL) + + // Disambiguate overloaded `Editor` inits (`OrderedSet` vs `OrderedSet`). + let editor = Editor(files: OrderedSet(), workspace: nil) + editor.openTab(file: file) + + let firstInstance = try #require(editor.selectedTab) + firstInstance.cursorPositions = [CursorPosition(line: 3, column: 2)] + + // Re-open the same file (as the navigator / history paths do). + editor.openTab(file: file) + + let secondInstance = try #require(editor.selectedTab) + #expect(ObjectIdentifier(firstInstance) == ObjectIdentifier(secondInstance)) + #expect(secondInstance.cursorPositions.first?.start.line == 3) + #expect(editor.tabs.count == 1) + } + } + + @Test + @MainActor + func temporaryReopenReusesExistingInstance() throws { + try withTempDir { dir in + let fileURL = dir.appending(path: "Temp.swift") + try "let x = 1\n".write(to: fileURL, atomically: true, encoding: .utf8) + let file = CEWorkspaceFile(url: fileURL) + + let editor = Editor(files: OrderedSet(), workspace: nil) + editor.openTab(file: file, asTemporary: true) + + let firstInstance = try #require(editor.selectedTab) + firstInstance.cursorPositions = [CursorPosition(line: 1, column: 5)] + + editor.openTab(file: file, asTemporary: true) + + let secondInstance = try #require(editor.selectedTab) + #expect(ObjectIdentifier(firstInstance) == ObjectIdentifier(secondInstance)) + #expect(secondInstance.cursorPositions.first?.start.column == 5) + } + } +} diff --git a/CodeEditTests/Features/StatusBar/StatusBarCursorPositionLabelTests.swift b/CodeEditTests/Features/StatusBar/StatusBarCursorPositionLabelTests.swift new file mode 100644 index 0000000000..aa52eb2902 --- /dev/null +++ b/CodeEditTests/Features/StatusBar/StatusBarCursorPositionLabelTests.swift @@ -0,0 +1,72 @@ +// +// StatusBarCursorPositionLabelTests.swift +// CodeEditTests +// +// Created by Boris Serzhanovich on 1/8/26. +// + +import XCTest +import AppKit +import CodeEditSourceEditor +@testable import CodeEdit + +final class StatusBarCursorPositionLabelTests: XCTestCase { + + func testLabelForSingleCursorUsesLineAndColumn() { + let label = StatusBarCursorPositionLabel.formatLabel( + cursorPositions: [CursorPosition(line: 12, column: 4)], + optionKeyPressed: false, + linesInRange: { _ in 0 } + ) + XCTAssertEqual(label, "Line: 12 Col: 4") + } + + func testLabelFallsBackWhenLineColumnUnresolved() { + let label = StatusBarCursorPositionLabel.formatLabel( + cursorPositions: [CursorPosition(range: NSRange(location: 10, length: 0))], + optionKeyPressed: false, + linesInRange: { _ in 0 } + ) + XCTAssertEqual(label, "Line: 1 Col: 1") + } + + func testLabelForMultipleSelections() { + let label = StatusBarCursorPositionLabel.formatLabel( + cursorPositions: [ + CursorPosition(line: 1, column: 1), + CursorPosition(line: 2, column: 1) + ], + optionKeyPressed: false, + linesInRange: { _ in 0 } + ) + XCTAssertEqual(label, "2 selected ranges") + } + + func testLabelForEmptyPositionsIsEmpty() { + let label = StatusBarCursorPositionLabel.formatLabel( + cursorPositions: [], + optionKeyPressed: false, + linesInRange: { _ in 0 } + ) + XCTAssertEqual(label, "") + } + + func testLabelForResolvedCharacterSelection() { + let label = StatusBarCursorPositionLabel.formatLabel( + cursorPositions: [CursorPosition(line: 1, column: 1)], + optionKeyPressed: false, + linesInRange: { _ in 0 } + ) + // Length 0 caret → line/col path + XCTAssertEqual(label, "Line: 1 Col: 1") + } + + func testLabelUsesOptionKeyCharacterOffset() { + let label = StatusBarCursorPositionLabel.formatLabel( + cursorPositions: [CursorPosition(range: NSRange(location: 42, length: 0))], + optionKeyPressed: true, + linesInRange: { _ in 0 } + ) + XCTAssertEqual(label, "Char: 42 Len: 0") + } +} diff --git a/CodeEditUITests/Features/NavigatorArea/ProjectNavigator/ProjectNavigatorUITests.swift b/CodeEditUITests/Features/NavigatorArea/ProjectNavigator/ProjectNavigatorUITests.swift index baac35964a..0c517e9df3 100644 --- a/CodeEditUITests/Features/NavigatorArea/ProjectNavigator/ProjectNavigatorUITests.swift +++ b/CodeEditUITests/Features/NavigatorArea/ProjectNavigator/ProjectNavigatorUITests.swift @@ -39,6 +39,25 @@ final class ProjectNavigatorUITests: XCTestCase { XCTAssertTrue(readmeEditor.exists) XCTAssertNotNil(readmeEditor.value as? String) + let cursorPositionLabel = window.staticTexts["CursorPositionLabel"] + XCTAssertTrue(cursorPositionLabel.waitForExistence(timeout: 2.0), "Cursor position label not found") + assertResolvedCursorPosition(cursorPositionLabel) + + let licenseRow = Query.Navigator.getProjectNavigatorRow(fileTitle: "LICENSE.md", navigator) + XCTAssertFalse(Query.Navigator.rowContainsDisclosureIndicator(licenseRow), "File has disclosure indicator") + licenseRow.click() + + let licenseTab = Query.TabBar.getTab(labeled: "LICENSE.md", tabBar) + XCTAssertTrue(licenseTab.exists) + + let licenseEditor = Query.Window.getFirstEditor(window) + let licenseContent = NSPredicate(format: "value CONTAINS %@", "MIT License") + expectation(for: licenseContent, evaluatedWith: licenseEditor) + waitForExpectations(timeout: 2.0) + + assertResolvedCursorPosition(cursorPositionLabel) + assertCursorPositionChanges(in: licenseEditor, cursorPositionLabel) + let rowCount = navigator.descendants(matching: .outlineRow).count // Open a folder @@ -59,4 +78,35 @@ final class ProjectNavigatorUITests: XCTestCase { XCTAssertTrue(newRowCount > finalRowCount, "Rows were not hidden after closing a folder") XCTAssertEqual(rowCount, finalRowCount, "Different Number of rows loaded") } + + private func assertResolvedCursorPosition(_ cursorPositionLabel: XCUIElement) { + let resolvedCursorPosition = NSPredicate( + format: "value CONTAINS %@ AND NOT value CONTAINS %@", + "Line:", + "-1" + ) + expectation(for: resolvedCursorPosition, evaluatedWith: cursorPositionLabel) + waitForExpectations(timeout: 2.0) + } + + private func assertCursorPositionChanges(in editor: XCUIElement, _ cursorPositionLabel: XCUIElement) { + assertCursorPositionChanges(cursorPositionLabel) { + editor.coordinate(withNormalizedOffset: CGVector(dx: 0.15, dy: 0.15)).click() + } + assertCursorPositionChanges(cursorPositionLabel) { + editor.coordinate(withNormalizedOffset: CGVector(dx: 0.75, dy: 0.75)).click() + } + } + + private func assertCursorPositionChanges(_ cursorPositionLabel: XCUIElement, after action: () -> Void) { + guard let originalValue = cursorPositionLabel.value as? String else { + XCTFail("Cursor position label value not found") + return + } + action() + let changed = NSPredicate(format: "value != %@", originalValue) + expectation(for: changed, evaluatedWith: cursorPositionLabel) + waitForExpectations(timeout: 2.0) + assertResolvedCursorPosition(cursorPositionLabel) + } } diff --git a/Documentation.docc/App Window/StatusBarView.md b/Documentation.docc/App Window/StatusBarView.md index c786a4afad..0c086ab207 100644 --- a/Documentation.docc/App Window/StatusBarView.md +++ b/Documentation.docc/App Window/StatusBarView.md @@ -1,5 +1,12 @@ # ``CodeEdit/StatusBarView`` +The status bar shows editor context for the active tab, including the cursor line and column. + +``StatusBarCursorPositionLabel`` observes the active ``EditorInstance`` by object identity +(not file equality) and keeps the last known caret when SourceEditor omits cursor state on +scroll-only updates. Unresolved range-only positions fall back to `Line: 1 Col: 1` until the +text view resolves line and column. + ## Topics ### Model