Skip to content

Fix ColorPresentation.is rejecting presentations with a textEdit - #1865

Merged
Dirk Bäumer (dbaeumer) merged 1 commit into
microsoft:mainfrom
kwy404:fix-color-presentation-is
Sep 28, 2026
Merged

Dirk Bäumer (dbaeumer) merged 1 commit into
microsoft:mainfrom
kwy404:fix-color-presentation-is

Conversation

@kwy404

Copy link
Copy Markdown
Contributor

ColorPresentation.is validates the optional textEdit by calling TextEdit.is(candidate) on the presentation itself instead of on candidate.textEdit. A presentation has no range or newText, so every valid presentation that carries a textEdit is rejected. For example ColorPresentation.is(ColorPresentation.create('red', TextEdit.insert(Position.create(0, 0), 'red'))) returns false.

The fix passes candidate.textEdit to TextEdit.is.

I added a ColorPresentation.is case to types/src/test/typeguards.test.ts. It fails before the change (false !== true) and passes after it. The full types test suite (39 tests) passes and eslint is clean.

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI 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.

Copilot review overview

🟢 Approval recommended

The targeted fix is correct and covered by an appropriate regression test.

Review effort: Balanced
Findings: None

What changed in this PR

Fixes ColorPresentation.is so presentations containing a valid textEdit are accepted.

Changes:

  • Validates candidate.textEdit rather than the containing presentation.
  • Adds regression coverage for a presentation with an insert edit.
File Description
types/​src/​main.ts Corrects textEdit validation.
types/​src/​test/​typeguards.test.ts Adds regression coverage.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@dbaeumer

Copy link
Copy Markdown
Member

/azp run

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
Successfully started running 1 pipeline(s).
@dbaeumer
Dirk Bäumer (dbaeumer) merged commit 9d23f4b into microsoft:main Sep 28, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

5 participants