Skip to content

feat(language-service): provide completion for $event variable - #34566

Closed
ayazhafiz wants to merge 4 commits into
angular:masterfrom
ayazhafiz:i/$event-completion
Closed

ayazhafiz wants to merge 4 commits into
angular:masterfrom
ayazhafiz:i/$event-completion

Conversation

@ayazhafiz

Copy link
Copy Markdown
Contributor

This commit adds a completion for the $event variable in bound event
expressions.

This is the first in a series of PRs to support completions for
properties on $event variables (angular/vscode-ng-language-service#531).

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Feature

Does this PR introduce a breaking change?

  • Yes
  • No

@ayazhafiz ayazhafiz added feature Label used to distinguish feature request from other issues area: language-service Issues related to Angular's VS Code language service target: patch This PR is targeted for the next patch release labels Dec 26, 2019
@ayazhafiz
ayazhafiz requested review from a team and kyliau December 26, 2019 14:46
@ngbot ngbot Bot added this to the needsTriage milestone Dec 26, 2019
@ayazhafiz
ayazhafiz requested a review from kyliau January 10, 2020 01:29
@kyliau kyliau added the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jan 10, 2020
@kyliau

kyliau commented Jan 10, 2020

Copy link
Copy Markdown
Contributor

Could you please rerun circle CI? the test status seems to be stale

@ayazhafiz
ayazhafiz force-pushed the i/$event-completion branch from 813290a to 803758d Compare January 10, 2020 21:19
@kyliau

kyliau commented Jan 10, 2020

Copy link
Copy Markdown
Contributor
packages/language-service/src/completions.ts:337:83 - error TS2554: Expected 2 arguments, but got 3.

337       new ExpressionVisitor(info, position, () => getExpressionScope(dinfo, path, false));

@ayazhafiz

Copy link
Copy Markdown
Contributor Author

Forgot about that. Thanks.

@ayazhafiz
ayazhafiz force-pushed the i/$event-completion branch from acf49b6 to 97ec21c Compare January 10, 2020 23:39
@ayazhafiz

Copy link
Copy Markdown
Contributor Author

@kyliau ptal

@kyliau kyliau 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 13, 2020
@atscott

atscott commented Jan 13, 2020

Copy link
Copy Markdown
Contributor

Hi @ayazhafiz, can you rebase and resolve the merge conflict? Thanks!

@ayazhafiz
ayazhafiz force-pushed the i/$event-completion branch from 97ec21c to 3e45d12 Compare January 13, 2020 23:27
@atscott

atscott commented Jan 14, 2020

Copy link
Copy Markdown
Contributor

Sorry @ayazhafiz! I merged another PR that conflicted with this one. Can you rebase again?

@ayazhafiz
ayazhafiz force-pushed the i/$event-completion branch from 3e45d12 to a883ee5 Compare January 14, 2020 23:27
atscott pushed a commit that referenced this pull request Jan 14, 2020
This commit adds a completion for the `$event` variable in bound event
expressions.

This is the first in a series of PRs to support completions for
properties on `$event` variables (angular/vscode-ng-language-service#531).

PR Close #34566
@atscott atscott closed this in d7ea389 Jan 14, 2020
@angular-automatic-lock-bot

Copy link
Copy Markdown

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 14, 2020
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 feature Label used to distinguish feature request from other issues target: patch This PR is targeted for the next patch release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants