-
Notifications
You must be signed in to change notification settings - Fork 40
Add ability to filter analzyer results on a set of paths #260
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 ability to filter analzyer results on a set of paths #260
Conversation
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. |
if (paths != null && paths.isNotEmpty) { | ||
final roots = await this.roots; | ||
final requestedUris = <Uri>{}; | ||
for (final path in paths) { |
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.
See validateRootConfig
under the lib/src/utils/cli_utils.dart
which will validate that a path is under one of the roots.
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.
Do you mean _isUnderRoot
?
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.
validateRootConfig
uses that function but does some additional stuff as well, and handles actually extracting the Root
object and the paths for you, given a single "root" configuration object (one entry in roots
).
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.
There is no validateRootConfig in the repo
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.
Oh hmmm, I think this is something that only exists in my "summaries" branch lol, hence the confusion 🤣
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.
yeah, 2a83a89.
You could roll that into this PR, its basically just extracting out some shared logic from the code that runs a command in a root given a configuration.
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.
Okay, I incorporated it, and added unit tests for it. I just incorporated the validateRootConfig
function, not the rest of that PR.
|
||
if (rootConfigs != null && rootConfigs.isEmpty) { | ||
// Empty list of roots means do nothing. | ||
return CallToolResult(content: [TextContent(text: 'No errors')]); |
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 think this should either return errors for the entire project or return an error response. This isn't really a valid request?
if (paths != null && paths.isNotEmpty) { | ||
final roots = await this.roots; | ||
final requestedUris = <Uri>{}; | ||
for (final path in paths) { |
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.
Oh hmmm, I think this is something that only exists in my "summaries" branch lol, hence the confusion 🤣
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.
Going to go ahead and approve this - feel free to integrate that helper method in or not, up to you. I am out next week and don't want to hold you up :)
Revisions updated by `dart tools/rev_sdk_deps.dart`. ai (https://github.com/dart-lang/ai/compare/72a9283..6b4b2bc): 6b4b2bc 2025-08-11 Greg Spencer Add ability to filter analyzer results on a set of paths (dart-lang/ai#260) ecosystem (https://github.com/dart-lang/ecosystem/compare/4543c38..68ff911): 68ff911 2025-08-08 Moritz Make health file name configurable (dart-lang/ecosystem#364) http (https://github.com/dart-lang/http/compare/afda310..6656f15): 6656f15 2025-08-07 Brian Quinlan Remove http-beta dependency (dart-lang/http#1806) b661894 2025-08-07 Brian Quinlan Prepare to release packages http/cronet_http/cupertino_http supporting request cancellation (dart-lang/http#1805) i18n (https://github.com/dart-lang/i18n/compare/25cdb1b..c28ad5e): c28ad5ea 2025-08-08 Moritz Configure gemini (dart-lang/i18n#1002) web (https://github.com/dart-lang/web/compare/f3c960f..72cdd84): 72cdd84 2025-08-07 Kevin Moore Fix pkg:web CI badge (dart-lang/web#441) Change-Id: I98392b4c4ef037c6ffa8754e8fd2d9a9e17a643e Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/444781 Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Auto-Submit: Devon Carew <devoncarew@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com>
Description
This adds the ability to filter the results of
analyze_files
with a set of paths.Tests
Related Issues
analyze_files
should filter results to the paths supplied. #259