@codex review
/gemini review

Thanks for the review request, @DanTup! I've thoroughly reviewed the changes in this pull request.

This PR introduces a new service method, Editor.getActiveLocation(), to the Dart Tooling Daemon (DTD), which allows clients to query the currently active editor location. This addresses the issue mentioned in the PR description by providing a direct way to retrieve editor selection information.

Here's a summary of my findings:

General Feedback

Overall, the changes are well-structured and logically sound. The introduction of the ActiveLocation interface and the refactoring of ActiveLocationChangedEvent are good design decisions, promoting reusability and clarity. The new service method Editor.getActiveLocation is correctly implemented and thoroughly tested.

Detailed Review

src/extension/dart/tooling_daemon.ts

• Type Imports: The import of ActiveLocation instead of ActiveLocationChangedEvent for type definition is correct and aligns with the new interface structure.
• Debounce Refactoring: Removing the activeLocationDebounceTimeMs class member and directly using config.dtdEditorActiveLocationDelay simplifies the code and reduces redundancy.
• Method Renaming: Renaming sendActiveLocationChange to updateActiveLocation improves clarity, as it better describes the method's role in preparing and then triggering the sending of location data.
• Logic Extraction: Extracting the ActiveLocation object creation into a new private method getActiveLocation enhances modularity and readability by separating data retrieval from event handling.
• EditorServices Update: Adding public activeLocation: ActiveLocation to EditorServices with an initial empty state correctly stores the latest active location for the new service method.
• Service Registration: The registration of the new Editor.getActiveLocation service method is correctly implemented, returning the stored activeLocation with the appropriate type.

src/shared/services/tooling_daemon.ts

• Type Consistency: Updating the sendActiveLocation parameter type from ActiveLocationChangedEvent to ActiveLocation and then wrapping it with kind: EventKind.activeLocationChanged before sending ensures type safety and consistency with the new definitions.
• Service Overloads: The addition of specific overloads for registerService and callMethod for Editor.getActiveLocation provides stronger typing and better API definition.

src/shared/services/tooling_daemon_services.ts

• ServiceMethod Enum: The addition of editorGetActiveLocation to the ServiceMethod enum correctly defines the new service method name.
• Interface Design: Introducing the ActiveLocation interface to define the core data structure and having ActiveLocationChangedEvent extend it is a good design pattern, promoting clear separation of concerns.

src/test/dart/tooling_daemon/tooling_daemon.test.ts

• Test Improvements: The addition of dtdDebounceMs and setConfigForTest to reduce debounce time makes the tests more reliable and efficient. The use of the simplify helper function improves test readability.
• New Test Coverage: The new test suite for should return the current active location is a crucial addition, directly verifying the functionality of the Editor.getActiveLocation service method across single and multi-selection scenarios.

Conclusion

The pull request effectively implements the new Editor.getActiveLocation service method, adhering to good design principles and ensuring comprehensive test coverage. The changes are clean and enhance the DTD's capabilities. I recommend merging this PR.

Codex Review: Didn't find any major issues. Keep it up!