update how we calculate import prefixes #1012
Conversation
| return referencedFrom._prefixFor(protoFilePath); | ||
| } | ||
|
|
||
| String _prefixFor(String protoFilePath) { |
There was a problem hiding this comment.
This argument is called usage below in enum_generator.dart, but referencedFrom here. It represents the same thing in both functions (and the same value is passed to both) so it would be easier to read if the same thing would be called the same everywhere, consistently.
I think protoFilePath is a better name, but no strong opinions as long as it's consistent.
There was a problem hiding this comment.
I had some trouble in the call sites keeping straight the two FileGenerators; one we were trying to get an import prefix for, and one was the context in which that prefix would be used.
referencedFrom and protoFilePath actually switch roles here - one is for the container, and the other is for the imported file.
I updated this method - and a few similar call sites - to used a named 'FileGenerator context' param. I think that makes the relationship explicit and a bit more clear.
There was a problem hiding this comment.
Thanks for the review!
Re: the checked-in generated files - this PR doesn't change the files here much (see protoc_plugin/lib/src/gen/google/protobuf/compiler/plugin.pb.dart for the only real change). This will affect other projects much more; something like https://github.com/dart-lang/labs/tree/main/pkgs/googleapis_firestore_v1 will see more diffs.
| return referencedFrom._prefixFor(protoFilePath); | ||
| } | ||
|
|
||
| String _prefixFor(String protoFilePath) { |
There was a problem hiding this comment.
I had some trouble in the call sites keeping straight the two FileGenerators; one we were trying to get an import prefix for, and one was the context in which that prefix would be used.
referencedFrom and protoFilePath actually switch roles here - one is for the container, and the other is for the imported file.
I updated this method - and a few similar call sites - to used a named 'FileGenerator context' param. I think that makes the relationship explicit and a bit more clear.
Revisions updated by `dart tools/rev_sdk_deps.dart`. ecosystem (https://github.com/dart-lang/ecosystem/compare/815d4ba..8cebaf0): 8cebaf0 2025-05-28 Kevin Moore team lints: update to latest pkg:lints (dart-lang/ecosystem#359) protobuf (https://github.com/dart-lang/protobuf/compare/d940c8d..c69077d): c69077d 2025-05-30 Devon Carew revert the change to not generate empty enum files (google/protobuf.dart#1016) 08e7410 2025-05-29 Devon Carew ignore unused_imports for json files (google/protobuf.dart#1013) efdabd7 2025-05-29 Devon Carew update how we calculate import prefixes (google/protobuf.dart#1012) 60dfaed 2025-05-28 Devon Carew rev package:protoc_plugin to 22.3.0 (google/protobuf.dart#1011) 86dfa6b 2025-05-27 Devon Carew move generated test code from out/ to test/gen/ (google/protobuf.dart#1009) test (https://github.com/dart-lang/test/compare/42a6d33..e2ddae9): e2ddae9f 2025-06-01 dependabot[bot] Bump the github-actions group with 2 updates (dart-lang/test#2506) tools (https://github.com/dart-lang/tools/compare/a0dda7e..04c6849): 04c68495 2025-06-01 dependabot[bot] Bump actions/setup-node from 3 to 4 in the github-actions group (dart-lang/tools#2106) webdev (https://github.com/dart-lang/webdev/compare/5dbb30e..64492b2): 64492b2d 2025-05-27 Nate Biggs Fix e2e 'evaluate' test to use new Closure toString format. (dart-lang/webdev#2624) webdriver (https://github.com/google/webdriver.dart/compare/f52afbf..b8f511d): b8f511d 2025-06-02 dependabot[bot] Bump nanasess/setup-chromedriver (google/webdriver.dart#329) Change-Id: If832327efab4615677528599ac51cecf4bb09fdf Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/432362 Reviewed-by: Konstantin Shcheglov <scheglov@google.com> Commit-Queue: Konstantin Shcheglov <scheglov@google.com> Auto-Submit: Devon Carew <devoncarew@google.com>
This makes the import prefixes a little more understandable in the library that they're used in (e.g,
$0-$9and mostly contiguous, not$0-$40and very sparse).