f28fd22 · parseable-macos

git.iko.soy/parseable-macos

on master help sign in

Fix 5 bugs: column move rightward, stale server info, abandoned connection, table quote, filter parenthesization

f28fd22iko3 months ago

ParseableViewer/App/AppState.swift+4

compare with master

unifiedsplit

@@ -141,6 +141,10 @@ final class AppState {

141 if activeConnection?.id != connection.id {

142 selectedStream = nil

143 streamLoadError = nil

144+ // Drop the previous server's cached `about` so views seeding from it

145+ // (ServerInfoView) don't paint the old server's details while the new

146+ // one loads. It's repopulated below once the new server responds.

147+ serverAbout = nil

144 // Reset the sidebar search filter and tab too: a stale filter would

148 // Reset the sidebar search filter and tab too: a stale filter would

145 // silently hide the new server's streams, and a stream-specific tab

149 // silently hide the new server's streams, and a stream-specific tab

146 // would have no valid selection behind it.

150 // would have no valid selection behind it.

ParseableViewer/ViewModels/LiveTailViewModel.swift+6−3

@@ -278,9 +278,12 @@ final class LiveTailViewModel {

278 let toIndex = columnOrder.firstIndex(of: targetColumn),

279 fromIndex != toIndex else { return }

280 let item = columnOrder.remove(at: fromIndex)

281+ // Match the drop-indicator semantics: a rightward move (fromIndex < toIndex)

282+ // lands the column *after* the target, a leftward move lands it *before*.

281− // Removing an element before the target shifts the target down by one, so

283+ // After removing an earlier element the target shifts down by one, so

282− // inserting at the original `toIndex` would land the column past its target.

284+ // inserting at `toIndex` lands after the target for rightward moves; for

285+ // leftward moves the target is unshifted and `toIndex` lands before it.

283− let adjusted = fromIndex < toIndex ? toIndex - 1 : toIndex

286+ let adjusted = toIndex

284 columnOrder.insert(item, at: adjusted)

287 columnOrder.insert(item, at: adjusted)

285 saveColumnConfig()

288 saveColumnConfig()

286 }

289 }

ParseableViewer/ViewModels/QueryViewModel.swift+17−7

@@ -193,9 +193,12 @@ final class QueryViewModel {

193 let toIndex = columnOrder.firstIndex(of: targetColumn),

194 fromIndex != toIndex else { return }

195 let item = columnOrder.remove(at: fromIndex)

196+ // Match the drop-indicator semantics: a rightward move (fromIndex < toIndex)

197+ // lands the column *after* the target, a leftward move lands it *before*.

196− // Removing an element before the target shifts the target down by one, so

198+ // After removing an earlier element the target shifts down by one, so

197− // inserting at the original `toIndex` would land the column past its target.

199+ // inserting at `toIndex` lands after the target for rightward moves; for

200+ // leftward moves the target is unshifted and `toIndex` lands before it.

198− let adjusted = fromIndex < toIndex ? toIndex - 1 : toIndex

201+ let adjusted = toIndex

199 columnOrder.insert(item, at: adjusted)

202 columnOrder.insert(item, at: adjusted)

200 saveColumnConfig()

203 saveColumnConfig()

201 updateSQLColumns()

204 updateSQLColumns()

@@ -627,6 +630,9 @@ final class QueryViewModel {

627 var depth = 0

630 var depth = 0

628 var whereEnd: String.Index?

631 var whereEnd: String.Index?

629 var tailStart: String.Index?

632 var tailStart: String.Index?

633+ // Track whether the existing WHERE body has a top-level OR — only then does

634+ // AND-ing a new condition need parentheses to preserve precedence.

635+ var bodyHasTopLevelOr = false

630 for token in SQLTokenizer.tokenize(sql) {

636 for token in SQLTokenizer.tokenize(sql) {

631 switch token.kind {

637 switch token.kind {

632 case .leftParen:

638 case .leftParen:

@@ -638,6 +644,8 @@ final class QueryViewModel {

638 whereEnd = token.range.upperBound

644 whereEnd = token.range.upperBound

639 } else if tailKeywords.contains(kw), tailStart == nil {

645 } else if tailKeywords.contains(kw), tailStart == nil {

640 tailStart = token.range.lowerBound

646 tailStart = token.range.lowerBound

647+ } else if kw == "OR", whereEnd != nil, tailStart == nil {

648+ bodyHasTopLevelOr = true

641 }

649 }

642 default:

650 default:

643 break

651 break

@@ -646,16 +654,18 @@ final class QueryViewModel {

646

654

647 if let whereEnd {

655 if let whereEnd {

648 // Parenthesize the existing WHERE body before AND-ing the new

656 // Parenthesize the existing WHERE body before AND-ing the new

649− // condition, otherwise an existing top-level OR silently changes

657+ // condition only when it has a top-level OR, otherwise the OR

650− // meaning: `WHERE a = 1 OR b = 2` would become

658+ // silently changes meaning: `WHERE a = 1 OR b = 2` would become

651 // `WHERE a = 1 OR b = 2 AND c = 3`, which SQL binds as

659 // `WHERE a = 1 OR b = 2 AND c = 3`, which SQL binds as

652 // `a = 1 OR (b = 2 AND c = 3)` rather than the intended

660 // `a = 1 OR (b = 2 AND c = 3)` rather than the intended

653− // `(a = 1 OR b = 2) AND c = 3`.

661+ // `(a = 1 OR b = 2) AND c = 3`. A body without a top-level OR is

662+ // left un-parenthesized so simple filters stay readable.

654 let bodyEnd = tailStart ?? sql.endIndex

663 let bodyEnd = tailStart ?? sql.endIndex

655 let head = sql[..<whereEnd]

664 let head = sql[..<whereEnd]

656 let body = sql[whereEnd..<bodyEnd].trimmingCharacters(in: .whitespacesAndNewlines)

665 let body = sql[whereEnd..<bodyEnd].trimmingCharacters(in: .whitespacesAndNewlines)

657 let tail = sql[bodyEnd...].trimmingCharacters(in: .whitespacesAndNewlines)

666 let tail = sql[bodyEnd...].trimmingCharacters(in: .whitespacesAndNewlines)

667+ let wrappedBody = bodyHasTopLevelOr ? "(\(body))" : body

658− sql = "\(head) (\(body)) AND \(condition)"

668+ sql = "\(head) \(wrappedBody) AND \(condition)"

659 if !tail.isEmpty {

669 if !tail.isEmpty {

660 sql += " \(tail)"

670 sql += " \(tail)"

661 }

671 }

ParseableViewer/Views/ConnectionSheet.swift+7

@@ -123,6 +123,13 @@ struct ConnectionSheet: View {

123 .accessibilityLabel("Test connection")

124

125 Button("Cancel") {

126+ // A new connection is persisted on the first save so a retry

127+ // updates it in place. If the user abandons it via Cancel

128+ // (e.g. after a failed connect), remove that never-connected

129+ // entry so it doesn't linger in the sidebar.

130+ if let created = createdConnection {

131+ appState.removeConnection(created)

132+ }

126 password = ""

133 password = ""

127 dismiss()

134 dismiss()

128 }

135 }

ParseableViewer/Views/SQLEditorView.swift+6

@@ -221,6 +221,12 @@ struct SQLEditorView: NSViewRepresentable {

221 if item.kind == .table, wordStart > 0,

222 nsText.character(at: wordStart - 1) == 0x22 {

223 wordStart -= 1

224+ // Mirror on the trailing side: if a matching closing quote follows

225+ // the word (caret inside an already-quoted `FROM "my|"`), absorb it

226+ // too so the inserted `"name"` doesn't leave a dangling `"name""`.

227+ if wordEnd < nsText.length, nsText.character(at: wordEnd) == 0x22 {

228+ wordEnd += 1

229+ }

224 }

230 }

225

231

226 let replaceRange = NSRange(location: wordStart, length: wordEnd - wordStart)

232 let replaceRange = NSRange(location: wordStart, length: wordEnd - wordStart)

ParseableViewer/Views/ServerInfoView.swift+7

@@ -126,6 +126,13 @@ struct ServerInfoView: View {

126 // Key on the active connection so switching (or reconnecting to a

127 // different) server reloads instead of showing the previous server's info.

128 .task(id: appState.activeConnection?.id) {

129+ // Drop the previous server's details immediately on switch so they

130+ // aren't shown while the newly-selected server's info loads. (Local

131+ // @State persists across the .task id change, so it must be cleared

132+ // explicitly.) The manual Refresh button keeps the current values.

133+ about = nil

134+ isHealthy = false

135+ errorMessage = nil

129 await loadInfo()

136 await loadInfo()

130 }

137 }

131 }

138 }

ParseableViewerTests/QueryViewModelTests.swift+28

@@ -259,6 +259,15 @@ final class QueryViewModelTests: XCTestCase {

259 XCTAssertTrue(vm.sqlQuery.contains("WHERE \"level\" = 'error'"))

260 }

261

262+ func testAddColumnFilterParenthesizesExistingOr() {

263+ // An existing top-level OR must be parenthesized before AND-ing the new

264+ // condition so precedence is preserved.

265+ let vm = QueryViewModel()

266+ vm.sqlQuery = "SELECT * FROM \"logs\" WHERE \"a\" = 1 OR \"b\" = 2"

267+ vm.addColumnFilter(column: "host", value: .string("web-1"), exclude: false)

268+ XCTAssertTrue(vm.sqlQuery.contains("(\"a\" = 1 OR \"b\" = 2) AND \"host\" = 'web-1'"))

269+ }

270+

262 func testAddColumnFilterIntValue() {

271 func testAddColumnFilterIntValue() {

263 let vm = QueryViewModel()

272 let vm = QueryViewModel()

264 vm.sqlQuery = "SELECT * FROM \"logs\""

273 vm.sqlQuery = "SELECT * FROM \"logs\""

@@ -348,6 +357,25 @@ final class QueryViewModelTests: XCTestCase {

348 XCTAssertEqual(vm.columnOrder, ["c", "a", "b"])

357 XCTAssertEqual(vm.columnOrder, ["c", "a", "b"])

349 }

358 }

350

359

360+ func testMoveColumnRightwardLandsAfterTarget() {

361+ // Rightward move (e.g. the "Move Right" context action, which passes the

362+ // adjacent column as the target) must land the column *after* the target,

363+ // not produce a no-op.

364+ let vm = QueryViewModel()

365+ vm.columns = ["a", "b", "c"]

366+ vm.columnOrder = ["a", "b", "c"]

367+ vm.moveColumn("b", to: "c")

368+ XCTAssertEqual(vm.columnOrder, ["a", "c", "b"])

369+ }

370+

371+ func testMoveColumnLeftwardLandsBeforeTarget() {

372+ let vm = QueryViewModel()

373+ vm.columns = ["a", "b", "c"]

374+ vm.columnOrder = ["a", "b", "c"]

375+ vm.moveColumn("b", to: "a")

376+ XCTAssertEqual(vm.columnOrder, ["b", "a", "c"])

377+ }

378+

351 func testMoveColumnByIndexSet() {

379 func testMoveColumnByIndexSet() {

352 let vm = QueryViewModel()

380 let vm = QueryViewModel()

353 vm.columns = ["a", "b", "c"]

381 vm.columns = ["a", "b", "c"]