[pigeon] Deduces the package name for dart test file imports. #2467
Conversation
cd4e44d to
ebebcd8
Compare
ebebcd8 to
77a01d9
Compare
| } | ||
|
|
||
| final String text = File(pubspecPath).readAsStringSync(); | ||
| final RegExp nameFinder = RegExp(r'^name:\s*(.*)'); |
There was a problem hiding this comment.
We should use https://pub.dev/packages/yaml to extract this robustly. This won't handle all valid YAML correctly.
| return match.group(1); | ||
| } | ||
|
|
||
| String _posixify(String input) { |
There was a problem hiding this comment.
This definitely needs a comment. input is very vague—in fact, nothing about this function signature indicates its about paths at all—and the fact that this would convert a relative path to an absolute path is non-obvious.
| 'import \'${_escapeForDartSingleQuotedString(relativeDartPath)}\';'); | ||
| } else { | ||
| final String path = relativeDartPath.replaceFirstMapped( | ||
| RegExp(r'.*/lib/(.*?)'), (Match match) => match.group(1).toString()); |
There was a problem hiding this comment.
Won't (.*?) always be empty?
There was a problem hiding this comment.
You know what, you might be right in theory. In practice it's doing the (relatively) correct operation. It's asserted to work in the unit test. It's almost like there is an implied $ at the end of the regex for some reason. I added explicit ^ and $ characters to avoid confusion. I'll file a dart bug.
There was a problem hiding this comment.
I don't think there's a Dart bug here, what you wrote is just a weird way of writing an empty replacement. Replacing .*/lib/(.*)$ with '$1' and replacing .*/lib/ with '' are the same thing.
There was a problem hiding this comment.
Oh yea, weird, I see now. Funny how that worked out.
c4cd52e to
31ee52f
Compare
31ee52f to
a923dfc
Compare
| 'import \'${_escapeForDartSingleQuotedString(relativeDartPath)}\';'); | ||
| } else { | ||
| final String path = relativeDartPath.replaceFirstMapped( | ||
| RegExp(r'^.*/lib/(.*?)$'), (Match match) => match.group(1).toString()); |
There was a problem hiding this comment.
This still seems like a really complicated way of writing it. Why not just replaceFirst(RegExp(r'^.*/lib/'), '')
There was a problem hiding this comment.
That should work. It's a different way than I was looking at it. Done.
fixes flutter/flutter#97744
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.