Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(typescript): fix inlay hints mapping for large chunks of source code mapped verbatim to generated code. #236

Merged
merged 1 commit into from
Sep 4, 2024

Conversation

piotrtomiak
Copy link
Contributor

I've started working on support of inlay hints for Angular templates and I've noticed some issues.

Copy link

pkg-pr-new bot commented Aug 30, 2024

commit: 588dcea

@volar/eslint

pnpm add https://pkg.pr.new/volarjs/volar.js/@volar/eslint@236

@volar/jsdelivr

pnpm add https://pkg.pr.new/volarjs/volar.js/@volar/jsdelivr@236

@volar/kit

pnpm add https://pkg.pr.new/volarjs/volar.js/@volar/kit@236

@volar/language-core

pnpm add https://pkg.pr.new/volarjs/volar.js/@volar/language-core@236

@volar/language-server

pnpm add https://pkg.pr.new/volarjs/volar.js/@volar/language-server@236

@volar/language-service

pnpm add https://pkg.pr.new/volarjs/volar.js/@volar/language-service@236

@volar/monaco

pnpm add https://pkg.pr.new/volarjs/volar.js/@volar/monaco@236

@volar/source-map

pnpm add https://pkg.pr.new/volarjs/volar.js/@volar/source-map@236

@volar/test-utils

pnpm add https://pkg.pr.new/volarjs/volar.js/@volar/test-utils@236

@volar/typescript

pnpm add https://pkg.pr.new/volarjs/volar.js/@volar/typescript@236

@volar/vscode

pnpm add https://pkg.pr.new/volarjs/volar.js/@volar/vscode@236

Open in Stackblitz

@@ -936,14 +936,29 @@ function provideInlayHints(language: Language<string>, provideInlayHints: ts.Lan
if (serviceScript) {
let start: number | undefined;
let end: number | undefined;
const map = language.maps.get(serviceScript.code, targetScript);
const map = language.maps.get(serviceScript.code, sourceScript);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

First of all maps were taken for the wrong script. It should be a source script to have correct mappings from the template to generated code

if (mappingStart >= span.start && mappingStart <= span.start + span.length) {
genStart = mapping.generatedOffsets[0];
genEnd = mapping.generatedOffsets[mapping.generatedOffsets.length - 1]
+ (mapping.generatedLengths ?? mapping.lengths)[mapping.generatedOffsets.length - 1];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The end offset should include the whole length of the generated code.

+ (mapping.generatedLengths ?? mapping.lengths)[mapping.generatedOffsets.length - 1];
} else if (mappingStart < span.start && span.start < mappingStart + mapping.lengths[0]
&& mapping.sourceOffsets.length == 1
&& (!mapping.generatedLengths || mapping.generatedLengths[0] === mapping.lengths[0])
Copy link
Contributor Author

Choose a reason for hiding this comment

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

My Angular VirtualCode has large pieces of code copied verbatim from the original file. This condition ensures that it is correctly handled and not ignored.

@johnsoncodehk
Copy link
Member

Since I had difficulty testing it, it took me some time to understand. Thank you for your PR!

Are you planning to make the same changes to getEncodedSemanticClassifications?

@johnsoncodehk johnsoncodehk merged commit 2fcb374 into volarjs:master Sep 4, 2024
5 checks passed
@piotrtomiak
Copy link
Contributor Author

@johnsoncodehk - I haven't planned since we are not using these currently, so I would have hard time testing it. Thanks for merging the PR!

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.

2 participants