Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Add codeAction support #298
Add codeAction support #298
Changes from 11 commits
90303ff
656e2ce
779625e
81daaf4
84a53aa
9d5412a
0dfe4ae
22b6eca
5505867
55e6659
a7a1acc
d59f5b7
5952cfe
8b0a122
3841eae
a88fa9d
f0c6834
0815c40
3a221ad
ea444c4
2816575
3cd55a6
2cc7122
76f9b04
f208a46
a46c522
289a445
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These awaits will potentially accumulate code action requests - should there instead be a mechanism to ignore codeactions when the file isn't ready? We may "miss" some actions but just moving the cursor around will produce then again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The language server protocol might already account for this and prevent additional requests if the first one hasn't resolved yet. So I'll test that first, and if we DO get multiple requests at the same time, then I'll add some code to ignore code actions that arrive before the programs are finished with their first runs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tested, and I can't get these to queue up. There's a
waitAllProgramFirstRuns()
in the onInitialized, so the vscode doesn't even send codeAction events until that's resolved, so this line is really a "just in case" measure.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure the position is correct for a tag without any attribute. You probably should just always insert at the end of the tag instead of trying to find the first attribute.
Also maybe have some
const { component } = this.file.parser.ast;
for readability.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did it this way so we don't occasionally insert two consecutive whitespaces. It's easier to reason about the whitespace after an attribute. Although I do agree that we should add this to the very end of the tag, so I adjusted this to find the FINAL attribute first, and then after the
<component
object second if there are no attributesThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding an attribute or a tag should be done with a helper function BTW, so it's testable and reusable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. I'll do that.