Skip to content

fix(editor): implement missing methods, fix cursor position clearing#38603

Merged
michael-s-molina merged 5 commits intoapache:masterfrom
michael-s-molina:fix-cursor-position
Mar 13, 2026
Merged

fix(editor): implement missing methods, fix cursor position clearing#38603
michael-s-molina merged 5 commits intoapache:masterfrom
michael-s-molina:fix-cursor-position

Conversation

@michael-s-molina
Copy link
Member

@michael-s-molina michael-s-molina commented Mar 12, 2026

User description

SUMMARY

Adds two missing methods to the extension APIs and corrects a bug in cursor positioning:

  1. EditorHandle.onDidChangeContent — adds a content-change subscription method to EditorHandle and implements it in AceEditorProvider. Uses editor.session.on('change', ...) and returns a Disposable for cleanup. The listener receives the full editor content on every change.

  2. sqlLab.setActivePanel — adds a missing setActivePanel(panelId) function to the SQL Lab extension API. Dispatches SET_ACTIVE_SOUTHPANE_TAB to programmatically switch between south pane panels ('Results', 'History', or a pinned table panel ID).

  3. moveCursorToPosition fix — clears any active selection before moving the cursor in AceEditorProvider, preventing leftover highlighted text after a programmatic cursor jump.

TESTING INSTRUCTIONS

onDidChangeContent

  1. In an extension or SQL Lab context, obtain an EditorHandle via onReady.
  2. Subscribe: const d = editor.onDidChangeContent(sql => console.log(sql));
  3. Type in the editor — the listener should fire with the full content on each keystroke.
  4. Call d.dispose() and confirm the listener stops firing.

setActivePanel

  1. Open SQL Lab and run a query.
  2. From an extension, call sqlLab.setActivePanel('Results') — the Results tab should become active.
  3. Call sqlLab.setActivePanel('History') — the History tab should become active.

moveCursorToPosition fix

  1. Select some text in the SQL editor.
  2. Programmatically call editor.moveCursorToPosition({ line: 0, column: 0 }).
  3. Confirm the selection is cleared and only the cursor moves.

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

CodeAnt-AI Description

Implement editor content-change subscription, add SQL Lab panel switch, and stop leftover selection when moving the cursor

What Changed

  • EditorHandle exposes onDidChangeContent so callers receive change events with a lazy full-content accessor and precise change details on every edit
  • SQL Lab extension API gains setActivePanel(panelId) to programmatically switch the south pane to 'Results', 'History', or a pinned table panel
  • Programmatic cursor moves clear any active selection first so only the cursor moves and no text remains highlighted

Impact

✅ Immediate content-change notifications for extensions
✅ Ability to programmatically focus Results/History/pinned tables
✅ No leftover highlighted text after programmatic cursor jumps

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 12, 2026

Code Review Agent Run #87713b

Actionable Suggestions - 0
Review Details
  • Files reviewed - 4 · Commit Range: e6e2aad..e6e2aad
    • superset-frontend/packages/superset-core/src/editors/index.ts
    • superset-frontend/packages/superset-core/src/sqlLab/index.ts
    • superset-frontend/src/core/editors/AceEditorProvider.tsx
    • superset-frontend/src/core/sqlLab/index.ts
  • Files skipped - 0
  • Tools
    • Eslint (Linter) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@michael-s-molina michael-s-molina moved this from To Do to In Review in Superset Extensions Mar 12, 2026
@dosubot dosubot bot added change:frontend Requires changing the frontend sqllab Namespace | Anything related to the SQL Lab labels Mar 12, 2026
@codeant-ai-for-open-source codeant-ai-for-open-source bot added the size:M This PR changes 30-99 lines, ignoring generated files label Mar 12, 2026
@codeant-ai-for-open-source
Copy link
Contributor

Sequence Diagram

This PR completes two extension-facing APIs and fixes cursor movement behavior in the Ace editor integration. It adds content change subscriptions and panel switching support, and ensures selections are cleared before programmatic cursor jumps.

sequenceDiagram
    participant Extension
    participant EditorHandle
    participant AceEditor
    participant SQLLabAPI
    participant Store

    Extension->>EditorHandle: Subscribe to content changes
    EditorHandle->>AceEditor: Attach session change listener
    AceEditor-->>Extension: Send full editor content on each edit

    Extension->>SQLLabAPI: Set active panel by panel id
    SQLLabAPI->>Store: Dispatch set active south pane tab

    Extension->>EditorHandle: Move cursor to position
    EditorHandle->>AceEditor: Clear selection and move cursor
Loading

Generated by CodeAnt AI

@netlify
Copy link

netlify bot commented Mar 12, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit e6e2aad
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/69b2ce973d762d000831998d
😎 Deploy Preview https://deploy-preview-38603--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

const bound = (thisArgs ? listener.bind(thisArgs) : listener) as (
value: string,
) => void;
const handler = () => bound(editor.getValue());
Copy link
Member

Choose a reason for hiding this comment

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

Is there risk that getting the full editor value could be excessively expensive in some editors? I wonder if we should rather pass a handle that offers getting the contents when/if needed. For instance, to control load, one may want to debounce rather than eagerly reacting to every content change.

Copy link
Member Author

Choose a reason for hiding this comment

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

Great observation @villebro.

The onDidChangeContent event has been updated to deliver a ContentChangeEvent instead of a raw string — providing a lazy getValue() to avoid unnecessary work on every keystroke, alongside a changes array describing the individual edits. Debouncing is left to consumers rather than built in, since different subscribers have different latency requirements and no single delay can satisfy all of them — following the convention of VS Code, Monaco, and CodeMirror.

@bito-code-review
Copy link
Contributor

Yes, calling editor.getValue() on every change event could be expensive for large editor content, as it retrieves the full string each time. Consider debouncing the listener to batch updates or passing a lazy handle (e.g., a function to get content on demand) to avoid unnecessary computations.

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 12, 2026

Code Review Agent Run #b91498

Actionable Suggestions - 0
Review Details
  • Files reviewed - 1 · Commit Range: e6e2aad..aa15390
    • superset-frontend/src/core/editors/AceEditorProvider.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@pull-request-size pull-request-size bot added size/L and removed size/M labels Mar 12, 2026
@codeant-ai-for-open-source codeant-ai-for-open-source bot added size:L This PR changes 100-499 lines, ignoring generated files and removed size:M This PR changes 30-99 lines, ignoring generated files labels Mar 12, 2026
@codeant-ai-for-open-source
Copy link
Contributor

Sequence Diagram

This PR completes two missing extension APIs and fixes cursor behavior. Extensions can now subscribe to editor content changes, switch SQL Lab south pane panels programmatically, and move the cursor without leaving stale text selections.

sequenceDiagram
    participant Extension
    participant EditorHandle
    participant AceEditor
    participant SQLLabAPI
    participant Store
    participant SouthPane

    Extension->>EditorHandle: Subscribe to content changes
    EditorHandle->>AceEditor: Register change listener
    AceEditor-->>Extension: Emit change event with content access

    Extension->>EditorHandle: Move cursor to position
    EditorHandle->>AceEditor: Clear selection and move cursor

    Extension->>SQLLabAPI: Set active panel by id
    SQLLabAPI->>Store: Dispatch active south pane tab action
    Store-->>SouthPane: Activate target panel
Loading

Generated by CodeAnt AI

Copy link
Member

@villebro villebro left a comment

Choose a reason for hiding this comment

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

Approving, but at the risk of overthinking this, couldn't the same apply to changes, too? Any reason why we don't want to do getChanges(): ReadonlyArray<ContentChange>? I just want to make sure we're as broadly compatible with the entire population of editors, and if the changes aren't eagerly produced by all editors, I'm not sure we should require them to do so. But again, this may be overkill..

@bito-code-review
Copy link
Contributor

bito-code-review bot commented Mar 12, 2026

Code Review Agent Run #33f9fe

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: aa15390..c443590
    • superset-frontend/packages/superset-core/src/editors/index.ts
    • superset-frontend/src/core/editors/AceEditorProvider.tsx
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@michael-s-molina
Copy link
Member Author

Approving, but at the risk of overthinking this, couldn't the same apply to changes, too? Any reason why we don't want to do getChanges(): ReadonlyArray? I just want to make sure we're as broadly compatible with the entire population of editors, and if the changes aren't eagerly produced by all editors, I'm not sure we should require them to do so. But again, this may be overkill..

The getValue() lazy pattern has a concrete motivation: materializing the full document string is work that many listeners don't need. But the changes are a different story — every editor event system inherently knows what changed in order to fire the event at all. There's no implementation where you'd have to do extra work to produce them; they're a byproduct of the event itself (Ace's delta, Monaco's e.changes, CodeMirror's viewUpdate.changes).

@michael-s-molina michael-s-molina merged commit 1867336 into apache:master Mar 13, 2026
68 checks passed
@github-project-automation github-project-automation bot moved this from In Review to Done in Superset Extensions Mar 13, 2026
michael-s-molina added a commit that referenced this pull request Mar 17, 2026
@michael-s-molina michael-s-molina moved this from Done to Cherried in Superset 6.1.0 Release Bugs Mar 17, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:frontend Requires changing the frontend packages size/L size:L This PR changes 100-499 lines, ignoring generated files sqllab Namespace | Anything related to the SQL Lab

Projects

Development

Successfully merging this pull request may close these issues.

2 participants