[L] Change in dart/sdk[main]: [analysis_server] Add `kind`s to LSP document highlights
Danny Tuppeny (Gerrit)
Danny Tuppeny added 2 comments![]( "Open in Gerrit")
## KindsPlease check this makes sense and you agree with it (and feel free to nit-pick if it's not how you'd have written it).
Using Write/Read for things like type names feels slightly odd to me, but between what the spec says and TypeScript's behaviour, I think it makes sense to copy.
class _DartDocumentHighlightsVisitor extends GeneralizingAstVisitor<void> {There were a lot of tests updated, and the kinds are in a map separate to the code, so it's probably easier to review the specific choices about what is Read/Write/Text here in the implementation than by reviewing the tests.
Related details
- Brian Wilkerson
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brian Wilkerson (Gerrit)
Brian Wilkerson voted and added 2 comments![]( "Open in Gerrit")
Votes added by Brian Wilkerson
| Code-Review | +1 |
2 comments
## KindsPlease check this makes sense and you agree with it (and feel free to nit-pick if it's not how you'd have written it).
Using Write/Read for things like type names feels slightly odd to me, but between what the spec says and TypeScript's behaviour, I think it makes sense to copy.
I agree.
- `Write` - setters, assignments, declarations, argument namesI'm guessing that you're distinguishing between references to (a) actual setters and (b) things like local variables that appear on the left-hand side of an assignment or fields being assigned to in a constructor's initializer list.
If so, the distinction is valid, but we might want to spell it out more concretely.
Related details
- Danny Tuppeny
- Keerti Parthasarathy
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Danny Tuppeny (Gerrit)
Danny Tuppeny added 1 comment![]( "Open in Gerrit")
- `Write` - setters, assignments, declarations, argument namesI'm guessing that you're distinguishing between references to (a) actual setters and (b) things like local variables that appear on the left-hand side of an assignment or fields being assigned to in a constructor's initializer list.
If so, the distinction is valid, but we might want to spell it out more concretely.
I changed `setters, assignments` to `assignments (setters, variables, initializers)` though I'm not sure if that's exactly what you had in mind. If not, let me know (or if you want to suggest specific text I'm happy to go with it).
Related details
- Brian Wilkerson
- Keerti Parthasarathy
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Brian Wilkerson (Gerrit)
Brian Wilkerson voted and added 1 comment![]( "Open in Gerrit")
Votes added by Brian Wilkerson
| Code-Review | +1 |
| Commit-Queue | +1 |
1 comment
- `Write` - setters, assignments, declarations, argument namesDanny TuppenyI'm guessing that you're distinguishing between references to (a) actual setters and (b) things like local variables that appear on the left-hand side of an assignment or fields being assigned to in a constructor's initializer list.
If so, the distinction is valid, but we might want to spell it out more concretely.
I changed `setters, assignments` to `assignments (setters, variables, initializers)` though I'm not sure if that's exactly what you had in mind. If not, let me know (or if you want to suggest specific text I'm happy to go with it).
- Danny Tuppeny
- Keerti Parthasarathy
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keerti Parthasarathy (Gerrit)
Keerti Parthasarathy voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Danny Tuppeny
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Keerti Parthasarathy (Gerrit)
Keerti Parthasarathy voted Commit-Queue+2![]( "Open in Gerrit")
| Commit-Queue | +2 |
Related details
- Danny Tuppeny
Code-Owners
Code-Review
Review-Enforcement
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
dart-scoped@luci-project-accounts.iam.gserviceaccount.com (Gerrit)
dart-...@luci-project-accounts.iam.gserviceaccount.com submitted the change![]( "Open in Gerrit")
Change information
[analysis_server] Add `kind`s to LSP document highlights
Populates the `kind` on the Document Highlights we return. This allows colouring reads and writes differently (which it turns out VS Code does by default).
The options are a bit limited (Read/Write/Text) and using Read/Write for things like type names feels slightly odd, but the spec does seem to encourage this and it's what TypeScript is doing.
Fixes https://github.com/dart-lang/sdk/issues/62929
- M pkg/analysis_server/doc/design/features/documentHighlight.md
- M pkg/analysis_server/lib/src/computer/computer_document_highlights.dart
- M pkg/analysis_server/lib/src/lsp/handlers/handler_document_highlights.dart
- M pkg/analysis_server/test/lsp/document_highlights_test.dart
Code-Review: +1 by Brian Wilkerson, +1 by Keerti Parthasarathy
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |