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

feat: add a first code-action quickfix (add missing attributes) #1549

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

kevineor
Copy link

@kevineor kevineor commented Jan 4, 2024

Implementation for a first LS CodeAction quickfix feature

Related issues

hashicorp/vscode-terraform#801
#1530

Proposal

The implementation relies on Diagnostics data property : https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#diagnostic

This property is stated as an extra data that is kept between textDocument/publishDiagnostics notifications and textDocument/codeAction requests, this permits to avoid the need of parsing again the document.

This could be proposed for other issues that are pushed through Diagnostics

Actual state

Jan-04-2024 21-41-52

Currently filling the missing attribute using a null value, it may be possible to use the default value, when one is provided.

@kevineor kevineor requested a review from a team as a code owner January 4, 2024 20:45
@hashicorp-cla
Copy link

hashicorp-cla commented Jan 4, 2024

CLA assistant check
All committers have signed the CLA.

@kevineor kevineor changed the title feat: add a first code-action quickfix feat: add a first code-action quickfix (add missing attributes) Jan 4, 2024
@jeremychauvet
Copy link

Waw!

Copy link
Member

@radeksimko radeksimko left a comment

Choose a reason for hiding this comment

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

There is a plan for introducing some abstraction layers first before we begin to introduce more code actions. This is to ensure they all stay reasonably maintainable as we can anticipate many more will come.

I outlined some more details in #1575 but this is mostly for transparency, not in the anticipation of an external contributor picking up any of that work. We do expect external contributors like you to participate in maintaining those individual code actions however, which will hopefully make use of the abstraction layers.

I know this may not be the answer you were hoping for but I prefer to be honest and open, even if that implies high likelihood of the PR being closed in the near term and/or need significant rewrite later.

For now I'm putting it on hold and a team member will revisit this PR later. I cannot commit to any timeline at this point unfortunately.

Thank you for your effort and understanding!

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.

None yet

4 participants