Skip to content

refactor: remove dead code and consolidate SQL escaping#1732

Merged
datlechin merged 1 commit into
mainfrom
chore/cleanup-dead-code-and-sql-escaping
Jun 20, 2026
Merged

refactor: remove dead code and consolidate SQL escaping#1732
datlechin merged 1 commit into
mainfrom
chore/cleanup-dead-code-and-sql-escaping

Conversation

@datlechin

Copy link
Copy Markdown
Member

What

Two small, verified cleanups from a read-only codebase audit. No behavior change for users.

Dead code removal

Each symbol was confirmed to have zero references repo-wide (whole-word grep across app, plugins, and test targets) before removal:

  • TokenPermissionFilter (whole file) and its ConnectionIdentifiable protocol, both never used.
  • ConnectionEnvironment enum (badge type) never instantiated.
  • IdentifiableURL / IdentifiableConnections sheet-binding wrappers, never bound.
  • SQLFunctionLiteral marker struct, superseded by the isSQLFunctionExpression string check.
  • MCPAuditLogStorage.TimeRange nested enum, never produced or consumed.

SQL escaping consolidation

DatabaseDriver's default quoteIdentifier and escapeStringLiteral duplicated the canonical SQLEscaping logic verbatim. They now delegate to SQLEscaping, and a new SQLEscaping.quoteIdentifier replaces an inline copy in QueryResultExportDataSource. The export fallback now also strips null bytes, matching the canonical ANSI rule. Added quoteIdentifier unit tests.

Verification

  • Every deleted symbol grep-confirmed unreferenced (including test targets).
  • swiftlint lint --strict clean on all changed files.
  • Build is for the reviewer to run.

Deliberately out of scope

  • 19 String(localized:) interpolation fixes in plugins (real, but behavior-neutral and touch separately-released bundles, better as a plugin PR).
  • The no-comments rule (~1,617 lines across 219 files): too large and judgment-heavy for a blanket pass.
  • 3 unused LSP/Copilot wire types: verifiably unused now but likely scaffolding for in-progress work, left in place.

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution! Before we can merge this PR, you need to sign our Contributor License Agreement.

To sign, please comment below with:

I have read the CLA Document and I hereby sign the CLA.


I have read the CLA Document and I hereby sign the CLA.


xuhengyu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3fd9eac658

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +280 to +281
self.lastRefilterPrefix = prefix
self.lastRefilterItems = ranked

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Publish the debounced ranking result

In SuggestionViewModel.cursorsUpdated, the window only replaces items with the value returned synchronously from completionOnCursorMove; nothing observes this cache. When the user types a prefix and then pauses or accepts with Tab, the debounced filterAndRank result stored here is never shown for that final prefix, so the visible list keeps the immediate filterByPrefix ordering and stale/no match ranges instead of the ranked result this task computed.

Useful? React with 👍 / 👎.

Comment on lines +272 to +274
let ranked = await Task.detached(priority: .userInitiated) {
provider.filterAndRank(fullItems, prefix: prefix, context: sqlContext)
}.value

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep refilter work cancellable

When typing just slowly enough for the 30 ms debounce to elapse, canceling cursorRefilterTask on the next cursor move does not cancel the unstructured Task.detached spawned here. Stale filterAndRank calls can continue over the full suggestion list while newer prefixes start their own work, so CPU-heavy refilters pile up in the large-list fast-typing case this change is meant to fix; keep the ranking in the cancellable task or retain and cancel the detached handle.

Useful? React with 👍 / 👎.

@datlechin datlechin merged commit fed866d into main Jun 20, 2026
4 of 8 checks passed
@datlechin datlechin deleted the chore/cleanup-dead-code-and-sql-escaping branch June 20, 2026 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant