Skip to content

Commit e003141

Browse files
authored
fix(editor): restore the cursor and selection when a query tab reopens (#2049)
1 parent 60d4f6b commit e003141

15 files changed

Lines changed: 203 additions & 11 deletions

File tree

.github/workflows/macos-tests.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,12 @@ jobs:
6666
- name: Run editor package tests
6767
run: swift test --package-path LocalPackages/CodeEditTextView
6868

69+
# Scoped to the controller suite on purpose. The rest of this package's tests
70+
# (HighlighterTests, TagEditingTests) already fail on main and one of them aborts
71+
# the runner, so gating on the whole package would keep CI permanently red.
72+
- name: Run source editor controller tests
73+
run: swift test --package-path LocalPackages/CodeEditSourceEditor --filter TextViewControllerTests
74+
6975
app-tests:
7076
name: macOS App Tests
7177
runs-on: macos-26

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2222

2323
### Fixed
2424

25+
- The SQL editor now returns the cursor to where you left it when a tab is reopened or the app restarts, and restores what you had selected, not just the caret. The saved position was never applied.
2526
- Autocomplete now keeps suggesting tables and columns after a connection drops and reconnects on its own. The reconnect cleared the schema it had loaded and never asked for it again, so a window that looked connected offered nothing but keywords for the rest of the session.
2627
- The JSON view of a result now updates when you run another query. It kept showing the previous result until you switched to the data grid and back, and a query that returned the same number of rows never updated at all.
2728
- The JSON view shows the whole result again when no row is selected. A selection left over from an earlier query made it show an empty list, and a column filter made it show the wrong rows.

LocalPackages/CodeEditSourceEditor/Sources/CodeEditSourceEditor/SourceEditor/SourceEditor.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -162,7 +162,7 @@ public struct SourceEditor: NSViewControllerRepresentable {
162162
}
163163

164164
private func updateControllerWithState(_ state: SourceEditorState, controller: TextViewController) {
165-
if let cursorPositions = state.cursorPositions, cursorPositions != state.cursorPositions {
165+
if let cursorPositions = state.cursorPositions, cursorPositions != controller.cursorPositions {
166166
controller.setCursorPositions(cursorPositions)
167167
}
168168

LocalPackages/CodeEditSourceEditor/Tests/CodeEditSourceEditorTests/Controller/TextViewControllerTests.swift

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -391,6 +391,26 @@ final class TextViewControllerTests: XCTestCase {
391391
XCTAssertEqual(controller.cursorPositions[1].start.column, 2)
392392
}
393393

394+
func test_setCursorPositionsClampsARangeBeyondTheText() {
395+
controller.setText("Hello")
396+
397+
controller.setCursorPositions([CursorPosition(range: NSRange(location: 2, length: 500))])
398+
399+
XCTAssertEqual(controller.cursorPositions.count, 1)
400+
XCTAssertEqual(controller.cursorPositions[0].range.location, 2)
401+
XCTAssertEqual(controller.cursorPositions[0].range.length, 3)
402+
}
403+
404+
func test_setCursorPositionsClampsALocationBeyondTheText() {
405+
controller.setText("Hello")
406+
407+
controller.setCursorPositions([CursorPosition(range: NSRange(location: 900, length: 0))])
408+
409+
XCTAssertEqual(controller.cursorPositions.count, 1)
410+
XCTAssertEqual(controller.cursorPositions[0].range.location, 5)
411+
XCTAssertEqual(controller.cursorPositions[0].range.length, 0)
412+
}
413+
394414
func test_cursorPositionRowColInit() {
395415
_ = controller.textView.becomeFirstResponder()
396416
controller.setText("Hello World")

LocalPackages/CodeEditTextView/Sources/CodeEditTextView/TextSelectionManager/TextSelectionManager.swift

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -89,9 +89,12 @@ public class TextSelectionManager: NSObject {
8989
let oldRanges = textSelections.map(\.range)
9090

9191
textSelections.forEach { $0.view?.removeFromSuperview() }
92-
// Remove duplicates, clamp out-of-bounds ranges, update suggested X position.
92+
// Remove duplicates, drop malformed ranges, clamp stale ones, update suggested X position.
93+
// A negative location is malformed and is discarded. A location past the end is a stale but
94+
// well-formed range, which happens whenever the text shrinks under an existing selection;
95+
// clamping keeps a usable selection there instead of leaving the view with none at all.
9396
let storageLength = textStorage?.length ?? 0
94-
textSelections = Set(ranges.map { $0.clamped(toLength: storageLength) })
97+
textSelections = Set(ranges.filter { $0.location >= 0 }.map { $0.clamped(toLength: storageLength) })
9598
.sorted(by: { $0.location < $1.location })
9699
.map {
97100
let selection = TextSelection(range: $0)
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//
2+
// NSRange+ClampToText.swift
3+
// TablePro
4+
//
5+
6+
import Foundation
7+
8+
internal extension NSRange {
9+
/// Moves the range inside `0..<length`, keeping as much of it as still fits.
10+
///
11+
/// A selection restored from disk was measured against the text as it was when the tab was
12+
/// saved. The query can be shorter now, so the saved range has to be clamped against the text
13+
/// the editor actually holds before it is applied.
14+
func clampedToTextLength(_ length: Int) -> NSRange {
15+
let start = Swift.min(Swift.max(location, 0), length)
16+
let end = Swift.min(Swift.max(location + self.length, 0), length)
17+
return NSRange(location: start, length: Swift.max(0, end - start))
18+
}
19+
}

TablePro/Models/Query/QueryTab.swift

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,20 @@ struct QueryTab: Identifiable, Equatable {
3535
var pendingRestoredSort: [PersistedSortColumn]?
3636
var restoredPage: Int?
3737
var restoredCursorOffset: Int?
38+
var restoredCursorLength: Int?
3839

3940
private static func clampedCursorOffset(_ offset: Int?, in query: String) -> Int? {
4041
guard let offset, offset >= 0 else { return nil }
4142
return min(offset, (query as NSString).length)
4243
}
4344

45+
private static func clampedCursorLength(_ length: Int?, from offset: Int?, in query: String) -> Int? {
46+
guard let length, length > 0, let start = clampedCursorOffset(offset, in: query) else { return nil }
47+
let available = (query as NSString).length - start
48+
guard available > 0 else { return nil }
49+
return min(length, available)
50+
}
51+
4452
init(
4553
id: UUID = UUID(),
4654
title: String = "Query",
@@ -70,6 +78,7 @@ struct QueryTab: Identifiable, Equatable {
7078
self.pendingRestoredSort = nil
7179
self.restoredPage = nil
7280
self.restoredCursorOffset = nil
81+
self.restoredCursorLength = nil
7382
}
7483

7584
init(from persisted: PersistedTab, defaultPageSize: Int) {
@@ -105,6 +114,11 @@ struct QueryTab: Identifiable, Equatable {
105114
self.pendingRestoredSort = persisted.sortColumns
106115
self.restoredPage = persisted.restoredPage.map { max(1, $0) }
107116
self.restoredCursorOffset = Self.clampedCursorOffset(persisted.cursorOffset, in: persisted.query)
117+
self.restoredCursorLength = Self.clampedCursorLength(
118+
persisted.cursorLength,
119+
from: persisted.cursorOffset,
120+
in: persisted.query
121+
)
108122
}
109123

110124
@MainActor static func buildBaseTableQuery(
@@ -182,6 +196,11 @@ struct QueryTab: Identifiable, Equatable {
182196
sortColumns: persistedSort,
183197
restoredPage: restoredPage,
184198
cursorOffset: Self.clampedCursorOffset(restoredCursorOffset, in: persistedQuery),
199+
cursorLength: Self.clampedCursorLength(
200+
restoredCursorLength,
201+
from: restoredCursorOffset,
202+
in: persistedQuery
203+
),
185204
columnWidths: widths,
186205
windowGroupIndex: windowGroupIndex
187206
)

TablePro/Models/Query/QueryTabState.swift

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ struct PersistedTab: Codable {
3737
var sortColumns: [PersistedSortColumn]?
3838
var restoredPage: Int?
3939
var cursorOffset: Int?
40+
var cursorLength: Int?
4041
var columnWidths: [String: CGFloat]?
4142
var windowGroupIndex: Int?
4243

@@ -58,6 +59,7 @@ struct PersistedTab: Codable {
5859
sortColumns: [PersistedSortColumn]? = nil,
5960
restoredPage: Int? = nil,
6061
cursorOffset: Int? = nil,
62+
cursorLength: Int? = nil,
6163
columnWidths: [String: CGFloat]? = nil,
6264
windowGroupIndex: Int? = nil
6365
) {
@@ -75,14 +77,15 @@ struct PersistedTab: Codable {
7577
self.sortColumns = sortColumns
7678
self.restoredPage = restoredPage
7779
self.cursorOffset = cursorOffset
80+
self.cursorLength = cursorLength
7881
self.columnWidths = columnWidths
7982
self.windowGroupIndex = windowGroupIndex
8083
}
8184

8285
private enum CodingKeys: String, CodingKey {
8386
case id, title, query, tabType, tableName, isView, databaseName, schemaName
8487
case sourceFileURL, erDiagramSchemaKey, queryParameters
85-
case sortColumns, restoredPage, cursorOffset, columnWidths, windowGroupIndex
88+
case sortColumns, restoredPage, cursorOffset, cursorLength, columnWidths, windowGroupIndex
8689
case overflowFileName
8790
}
8891

@@ -102,6 +105,7 @@ struct PersistedTab: Codable {
102105
sortColumns = try container.decodeIfPresent([PersistedSortColumn].self, forKey: .sortColumns)
103106
restoredPage = try container.decodeIfPresent(Int.self, forKey: .restoredPage)
104107
cursorOffset = try container.decodeIfPresent(Int.self, forKey: .cursorOffset)
108+
cursorLength = try container.decodeIfPresent(Int.self, forKey: .cursorLength)
105109
columnWidths = try container.decodeIfPresent([String: CGFloat].self, forKey: .columnWidths)
106110
windowGroupIndex = try container.decodeIfPresent(Int.self, forKey: .windowGroupIndex)
107111
overflowFileName = try container.decodeIfPresent(String.self, forKey: .overflowFileName)

TablePro/Views/Editor/QueryEditorView.swift

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ struct QueryEditorView: View {
2424
var connectionAIPolicy: AIConnectionPolicy?
2525
var tabID: UUID?
2626
var claimFocusOnAppear: Bool = false
27+
var restoredCursorRange: NSRange?
2728
var onCloseTab: (() -> Void)?
2829
var onExecuteQuery: (() -> Void)?
2930
var onExplain: ((ClickHouseExplainVariant?) -> Void)?
@@ -67,6 +68,7 @@ struct QueryEditorView: View {
6768
connectionAIPolicy: connectionAIPolicy,
6869
tabID: tabID,
6970
claimFocusOnAppear: claimFocusOnAppear,
71+
restoredCursorRange: restoredCursorRange,
7072
vimMode: $vimMode,
7173
onCloseTab: onCloseTab,
7274
onExecuteQuery: onExecuteQuery,

TablePro/Views/Editor/SQLEditorCoordinator.swift

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,27 @@ final class SQLEditorCoordinator: TextViewCoordinator, TextViewDelegate {
5252
@ObservationIgnored private var hasInstalledEditorServices = false
5353
@ObservationIgnored private weak var windowSentinel: WindowAccessorView?
5454

55+
@ObservationIgnored private var cursorRestorePending: NSRange?
56+
5557
var pendingFocusClaim: Bool { focusClaimPending }
5658

59+
var pendingCursorRestore: NSRange? { cursorRestorePending }
60+
5761
func scheduleEditorFocusClaim() {
5862
focusClaimPending = true
5963
}
6064

65+
/// Latches a saved selection to apply once, the moment the editor is in a window.
66+
///
67+
/// The caller sets this from `body`, which runs many times before the text view exists, so the
68+
/// setter is idempotent and the value is consumed exactly once in `installEditorServices`.
69+
/// Pushing it through the SwiftUI cursor binding instead would fight live typing, because the
70+
/// binding is written on every selection change the user makes.
71+
func scheduleCursorRestore(_ range: NSRange) {
72+
guard !hasInstalledEditorServices else { return }
73+
cursorRestorePending = range
74+
}
75+
6176
/// Vim mode for UI observation
6277
private(set) var vimMode: VimMode = .normal
6378
@ObservationIgnored private var vimEngine: VimEngine?
@@ -147,7 +162,11 @@ final class SQLEditorCoordinator: TextViewCoordinator, TextViewDelegate {
147162
Self.logger.debug("Editor focus claim: pending=\(claimPending) isKey=\(window.isKeyWindow) made=\(made)")
148163
}
149164

150-
if controller.cursorPositions.isEmpty {
165+
if let restored = cursorRestorePending {
166+
cursorRestorePending = nil
167+
let clamped = restored.clampedToTextLength(textView.textStorage.length)
168+
controller.setCursorPositions([CursorPosition(range: clamped)], scrollToVisible: true)
169+
} else if controller.cursorPositions.isEmpty {
151170
controller.setCursorPositions([CursorPosition(range: NSRange(location: 0, length: 0))])
152171
}
153172
}

0 commit comments

Comments
 (0)