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(language-service): fix go to definition for template variables and references #40455

Closed
wants to merge 2 commits into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Jan 16, 2021

The current "go to definition" is broken for template variables and
references when a template is overridden. This is because we get the
file url from the source span, which uses the overridden name
'override.html'. Instead, we can retrieve the template file from the
compiler in the same manner that is done for references.

Another way to fix this would have been to use the real template file path when
overriding a template, but this was the more straightforward fix since
the strategy was already used in find references and rename locations.

fixes angular/vscode-ng-language-service#1054

@atscott atscott added action: review The PR is still awaiting reviews from at least one requested reviewer area: language-service Issues related to Angular's VS Code language service target: rc This PR is targeted for the next release-candidate labels Jan 16, 2021
@atscott atscott requested a review from kyliau January 16, 2021 00:38
@ngbot ngbot bot modified the milestone: Backlog Jan 16, 2021
@google-cla google-cla bot added the cla: yes label Jan 16, 2021
…d references

The current "go to definition" is broken for template variables and
references when a template is overridden. This is because we get the
file url from the source span, which uses the overridden name
'override.html'. Instead, we can retrieve the template file from the
compiler in the same manner that is done for references.

Another way to fix this would have been to use the real template file path when
overriding a template, but this was the more straightforward fix since
the strategy was already used in find references and rename locations.

fixes angular/vscode-ng-language-service#1054
@kyliau kyliau added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jan 19, 2021
@atscott atscott added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Jan 19, 2021
@AndrewKushnir AndrewKushnir added target: minor This PR is targeted for the next minor release and removed target: rc This PR is targeted for the next release-candidate labels Jan 19, 2021
@AndrewKushnir
Copy link
Contributor

@atscott FYI this change conflicts with RC branch (11.1.x), so I merged it into master only. Could you please create a separate PR with these changes for RC branch if needed? Thank you.

atscott added a commit to atscott/angular that referenced this pull request Jan 19, 2021
…d references (angular#40455)

The current "go to definition" is broken for template variables and
references when a template is overridden. This is because we get the
file url from the source span, which uses the overridden name
'override.html'. Instead, we can retrieve the template file from the
compiler in the same manner that is done for references.

Another way to fix this would have been to use the real template file path when
overriding a template, but this was the more straightforward fix since
the strategy was already used in find references and rename locations.

fixes angular/vscode-ng-language-service#1054

PR Close angular#40455
AndrewKushnir pushed a commit that referenced this pull request Jan 19, 2021
…d references (#40455) (#40491)

The current "go to definition" is broken for template variables and
references when a template is overridden. This is because we get the
file url from the source span, which uses the overridden name
'override.html'. Instead, we can retrieve the template file from the
compiler in the same manner that is done for references.

Another way to fix this would have been to use the real template file path when
overriding a template, but this was the more straightforward fix since
the strategy was already used in find references and rename locations.

fixes angular/vscode-ng-language-service#1054

PR Close #40455

PR Close #40491
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Feb 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: language-service Issues related to Angular's VS Code language service cla: yes target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Go to definition" sometimes does not work for variables and template references
3 participants