Skip to content

feat(language-service): support to fix missing required inputs diagno… #50911

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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

feat(language-service): support to fix missing required inputs diagno… #50911

wants to merge 1 commit into from
+645 −0

Conversation

ivanwonder
Copy link
Contributor

@ivanwonder ivanwonder commented Jul 1, 2023 •
edited
Loading

…stic

Support to add the missing required inputs into the element and append after the last attribute of the element.

But it skips the structural directive shorthand attribute. For example, <a *ngFor=""></a>, its shorthand is <ng-template ngFor>, the valueSpan of the ngFor is empty, and the info is lost, so it can't use to insert the missing attribute after it.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@angular-robot angular-robot bot added the detected: feature PR contains a feature commit label Jul 1, 2023
@ivanwonder ivanwonder force-pushed the codefix-for-require-input branch from aa9d62a to f0c16b0 Compare July 1, 2023 04:03
@ivanwonder ivanwonder marked this pull request as ready for review July 1, 2023 04:17
@pullapprove pullapprove bot requested review from alxhub and JoostK July 1, 2023 04:17
@pkozlowski-opensource pkozlowski-opensource added the area: language-service Issues related to Angular's VS Code language service label Jul 5, 2023
@ngbot ngbot bot added this to the Backlog milestone Jul 5, 2023
@thePunderWoman thePunderWoman marked this pull request as draft May 21, 2025 15:24
@thePunderWoman
Copy link
Contributor

@ivanwonder Sorry for the long wait on getting this looked at. If you're still willing to get this landed, can you rebase and resolve the conflicts. It looks like a nice feature to get landed.

@thePunderWoman thePunderWoman force-pushed the codefix-for-require-input branch from f0c16b0 to b5f7c12 Compare June 4, 2025 15:42
@thePunderWoman
Copy link
Contributor

@ivanwonder Sorry for the long wait on getting this looked at. If you're still willing to get this landed, can you rebase and resolve the conflicts. It looks like a nice feature to get landed.

Actually, nevermind on the rebase. I was able to resolve those for you.

@thePunderWoman thePunderWoman marked this pull request as ready for review June 4, 2025 15:43
@thePunderWoman thePunderWoman requested a review from atscott June 4, 2025 15:43
@thePunderWoman thePunderWoman added action: review The PR is still awaiting reviews from at least one requested reviewer target: patch This PR is targeted for the next patch release labels Jun 4, 2025
@thePunderWoman thePunderWoman force-pushed the codefix-for-require-input branch 2 times, most recently from de4f346 to 8360b43 Compare June 4, 2025 16:21
@thePunderWoman
Copy link
Contributor

@ivanwonder Yeah, looks like even with resolving the conflicts, this is broken. Can you take a look?

@thePunderWoman thePunderWoman 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 Jun 4, 2025
@ivanwonder
Copy link
Contributor Author

@ivanwonder Yeah, looks like even with resolving the conflicts, this is broken. Can you take a look?

OK, I'll take a look.

@ivanwonder
Copy link
Contributor Author

@ivanwonder Yeah, looks like even with resolving the conflicts, this is broken. Can you take a look?

@thePunderWoman Done. It seems that the NgModule does not recognize the standalone component in the test case.

@thePunderWoman thePunderWoman removed the action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews label Jun 5, 2025
@ivanwonder ivanwonder force-pushed the codefix-for-require-input branch 2 times, most recently from 06b6bd1 to efc2a14 Compare June 5, 2025 14:05
@atscott atscott added target: minor This PR is targeted for the next minor release and removed target: patch This PR is targeted for the next patch release labels Jun 11, 2025
atscott
atscott approved these changes Jun 11, 2025
View reviewed changes
@angular-robot angular-robot bot requested a review from atscott June 12, 2025 02:30
@ivanwonder
feat(language-service): support to fix missing required inputs diagno…
67763fb 
…stic

Support to add the missing required inputs into the element and append
after the last attribute of the element.

But it skips the structural directive shorthand attribute. For example,
`<a *ngFor=""></a>`, its shorthand is `<ng-template ngFor>`, the `valueSpan`
of the `ngFor` is empty, and the info is lost, so it can't use to insert
the missing attribute after it.
@ivanwonder ivanwonder force-pushed the codefix-for-require-input branch from 9200747 to 67763fb Compare June 12, 2025 02:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@atscott atscott Awaiting requested review from atscott

Assignees
No one assigned
Labels
area: language-service Issues related to Angular's VS Code language service detected: feature PR contains a feature commit target: minor This PR is targeted for the next minor release
Projects
None yet
Milestone
Backlog
Development

Successfully merging this pull request may close these issues.
4 participants
@ivanwonder @thePunderWoman @atscott @pkozlowski-opensource