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.

Sign up for GitHub

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

Jump to bottom

feat(language-service): add quick info for inline templates in ivy #39060

Closed
wants to merge 1 commit into from
Closed

feat(language-service): add quick info for inline templates in ivy #39060

wants to merge 1 commit into from

Conversation

atscott
Copy link
Contributor

@atscott atscott commented Sep 30, 2020

Adds implementation for getQuickInfoAtPosition to the Ivy Language
Service, which now returns ts.QuickInfo for inline templates.

@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: major This PR is targeted for the next major release labels Sep 30, 2020
@atscott atscott self-assigned this Sep 30, 2020
@ngbot ngbot bot modified the milestone: needsTriage Sep 30, 2020
@atscott atscott force-pushed the keenpairing branch 2 times, most recently from 2aa4b3a to 4c5ee5e Compare September 30, 2020 16:11
josephperrott
josephperrott approved these changes Sep 30, 2020
View reviewed changes
Copy link
Member

@josephperrott josephperrott left a comment

Choose a reason for hiding this comment

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

LGTM

Reviewed-for: dev-infra

@atscott atscott force-pushed the keenpairing branch 2 times, most recently from 98122c1 to 85480f7 Compare September 30, 2020 19:43
ayazhafiz
ayazhafiz reviewed Sep 30, 2020
View reviewed changes
Copy link
Member

@ayazhafiz ayazhafiz left a comment

Choose a reason for hiding this comment

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

This is very nice!! Awesome stuff.

A general comment: does this work with ref-, bind-, on- syntaxes? If so, could tests for that be added as well?

packages/language-service/ivy/test/quick_info_spec.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/test/quick_info_spec.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/test/quick_info_spec.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/test/quick_info_spec.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/test/quick_info_spec.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/test/quick_info_spec.ts Outdated Show resolved Hide resolved
kyliau
kyliau suggested changes Sep 30, 2020
View reviewed changes
packages/language-service/ivy/BUILD.bazel Show resolved Hide resolved
packages/language-service/ivy/language_service.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/quick_info.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/quick_info.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/quick_info.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/utils.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/utils.ts Outdated Show resolved Hide resolved
packages/language-service/test/project/tsconfig.json Outdated Show resolved Hide resolved
packages/language-service/ivy/test/quick_info_spec.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/test/quick_info_spec.ts Outdated Show resolved Hide resolved
ayazhafiz
ayazhafiz reviewed Oct 1, 2020
View reviewed changes
packages/language-service/ivy/utils.ts Show resolved Hide resolved
packages/language-service/ivy/utils.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/utils.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/utils.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/utils.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/utils.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/utils.ts Outdated Show resolved Hide resolved
packages/language-service/src/hover.ts Show resolved Hide resolved
@atscott atscott force-pushed the keenpairing branch 3 times, most recently from a0f4333 to e5a730c Compare October 1, 2020 18:37
ayazhafiz
ayazhafiz reviewed Oct 1, 2020
View reviewed changes
packages/language-service/ivy/quick_info.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/quick_info.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/test/language_service_spec.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/quick_info.ts Show resolved Hide resolved
packages/language-service/ivy/utils.ts Outdated
Comment on lines 199 to 203
/**
* Returns a new `ts.SymbolDisplayPart` array which has the alias imports from the tcb filtered
* out, i.e. `i0.NgForOf`.
*/
export function filterAliasImports(displayParts: ts.SymbolDisplayPart[]) {
Copy link
Member

Choose a reason for hiding this comment

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

I almost feel like this should be a util provided by the compiler. But I guess the language service relies on the TCB leaking, so I am ambivalent either way. Just thought I would comment.

@atscott atscott force-pushed the keenpairing branch 3 times, most recently from 4dc345b to 06713f3 Compare October 1, 2020 22:50
kyliau
kyliau approved these changes Oct 2, 2020
View reviewed changes
packages/language-service/ivy/quick_info.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/quick_info.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/test/BUILD.bazel Outdated Show resolved Hide resolved
packages/language-service/ivy/utils.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/utils.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/utils.ts Outdated Show resolved Hide resolved
packages/language-service/ivy/utils.ts Outdated Show resolved Hide resolved
@atscott
feat(language-service): add quick info for inline templates in ivy
e40ae27 
Adds implementation for `getQuickInfoAtPosition` to the Ivy Language
Service, which now returns `ts.QuickInfo` for inline templates.
@atscott atscott added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 2, 2020
@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 Nov 2, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@alxhub alxhub alxhub left review comments

@ayazhafiz ayazhafiz ayazhafiz left review comments

@kyliau kyliau kyliau approved these changes

@josephperrott josephperrott josephperrott approved these changes

Assignees

@atscott atscott

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: major This PR is targeted for the next major release
Projects
None yet
Milestone
needsTriage
Development

Successfully merging this pull request may close these issues.
6 participants
@atscott @kyliau @alxhub @josephperrott @ayazhafiz @googlebot