Skip to content

Decouple Copilot CodeBlock from editor integration#3096

Merged
nighca merged 4 commits into
goplus:devfrom
nighca:issue-3043
Apr 29, 2026
Merged

Decouple Copilot CodeBlock from editor integration#3096
nighca merged 4 commits into
goplus:devfrom
nighca:issue-3043

Conversation

@nighca
Copy link
Copy Markdown
Collaborator

@nighca nighca commented Apr 29, 2026

This PR decouples the default Copilot code block renderer from editor-specific insertion behavior.

Closes #3043

Summary

  • Keep the Copilot default code block implementation editor-agnostic, with display and copy behavior only.
  • Add a markdown element override registration path so integrations can provide custom renderers for built-in markdown structures.
  • Register an editor-specific code block renderer from the SPX editor Copilot integration to preserve the existing Insert action there.

Issue coverage

CodeChange is already outside the shared Copilot module and is registered from the editor Copilot integration. This PR finishes the remaining CodeBlock part by removing editor/code-editor dependencies from the shared default CodeBlock implementation.

Checks

  • npm run type-check
  • npm run lint -- src/components/copilot/copilot.ts src/components/copilot/MarkdownView.vue src/components/editor/copilot/index.ts

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces a mechanism to customize markdown elements within the Copilot component by adding a registerMarkdownElements API and a markdownElements registry. The base CodeBlock component has been refactored to be more generic, removing editor-specific logic and introducing an actions slot, while a new editor-specific CodeBlock implementation is registered during initialization. Feedback suggests improving the encapsulation of the markdownElements property in the Copilot class by making it private and providing a read-only getter.

Comment thread spx-gui/src/components/copilot/copilot.ts
Comment thread spx-gui/src/components/copilot/copilot.ts
@nighca nighca marked this pull request as ready for review April 29, 2026 08:14
Copilot AI review requested due to automatic review settings April 29, 2026 08:14
Copy link
Copy Markdown
Contributor

@fennoai fennoai Bot left a comment

Choose a reason for hiding this comment

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

Clean architectural split — the slot-based composition is a good fit here and the disposal pattern is consistent with the rest of the codebase. A few things worth addressing:

Notable issues

  1. markdownElements should be private (or at least have a getter). All other registries on Copilot (customElementMap, toolMap, stateIndicatorComponentMap, etc.) are private and mediated through register/dispose methods. Leaving this field public exposes direct mutation that bypasses the disposal lifecycle, and MarkdownView.vue reading it directly creates an internal-representation coupling.

  2. registerMarkdownElements silently overwrites existing keys (via Object.assign) while every other register method emits a console.warn on duplicate names (see registerStateIndicatorComponent). In a context where LLM-rendered code blocks can trigger editor actions, silently replacing the renderer component is a meaningful trust-boundary concern worth adding a warning for.

  3. useSlotText() is called independently in both the editor CodeBlock wrapper and the base CodeBlock. Vue walks the slot VNode subtree in each computed independently, so the slot content is traversed twice per render. Since code.value in the editor wrapper is used only for handleInsert, passing code as a prop (or emitting it from the base component) would eliminate the redundancy.

Minor

  1. registerMarkdownElements has no JSDoc comment, unlike every other register* method on the class.

  2. The disposal closure uses Object.entries(elements).forEach(...) with an inline as keyof MarkdownElementDefinitions cast. If elements has undefined values (e.g. { codeBlock: undefined }), Object.entries will include them and trigger a delete that removes a subsequently-registered component. A simple guard (if (component == null) return) would close this edge case.

Positive change

The i18n string fix in CodeBlock.vue ('Failed to copy link to clipboard''Failed to copy code to clipboard') is a correct and welcome correction.

Comment thread spx-gui/src/components/copilot/copilot.ts
Comment thread spx-gui/src/components/copilot/copilot.ts
Comment thread spx-gui/src/components/copilot/copilot.ts
Comment thread spx-gui/src/components/editor/copilot/CodeBlock.vue Outdated
Comment thread spx-gui/src/components/copilot/copilot.ts
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR decouples Copilot’s default markdown code block rendering from the SPX editor integration by making the shared CodeBlock renderer display/copy-only, while allowing integrations (like the SPX editor) to override built-in markdown element renderers (e.g., to re-add “Insert”).

Changes:

  • Added a Copilot.registerMarkdownElements(...) API and reactive markdownElements registry for overriding built-in markdown renderers.
  • Updated Copilot Markdown rendering to use an override code block component when provided.
  • Moved the editor-specific “Insert code” action into a new SPX editor integration CodeBlock.vue and registered it from the editor Copilot setup.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
spx-gui/src/components/editor/copilot/index.ts Registers the editor-specific markdown code block renderer override.
spx-gui/src/components/editor/copilot/CodeBlock.vue New editor-specific CodeBlock wrapper that adds “Insert” behavior on top of the shared renderer.
spx-gui/src/components/copilot/custom-elements/CodeBlock.vue Makes shared CodeBlock editor-agnostic; supports optional actions slot; fixes copy messaging.
spx-gui/src/components/copilot/copilot.ts Introduces markdown element override registry + registerMarkdownElements disposer.
spx-gui/src/components/copilot/MarkdownView.vue Uses copilot.markdownElements.codeBlock override when present.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread spx-gui/src/components/copilot/copilot.ts
Comment thread spx-gui/src/components/copilot/copilot.ts Outdated
@nighca nighca merged commit 3d14c12 into goplus:dev Apr 29, 2026
4 checks passed
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.

Make Copilot CodeBlock and CodeChange project-type-agnostic

3 participants