[pigeon] Improve java generation#214
Conversation
There was a problem hiding this comment.
ArrayList and HashMap are directly lifted from the platform channels documentation as the datatypes the standard message codec is creating. I don't imagine it's safe or possible just to cast from one to the other:
https://flutter.dev/docs/development/platform-integration/platform-channels
There was a problem hiding this comment.
From what I can see in Flutter's code, they don't care about specific type anywhere:
- https://github.com/flutter/engine/blob/36769af3103819b6e977b71f80761091955fa5d9/shell/platform/android/io/flutter/plugin/common/StandardMessageCodec.java#L260
- https://github.com/flutter/engine/blob/36769af3103819b6e977b71f80761091955fa5d9/shell/platform/android/io/flutter/plugin/common/JSONUtil.java#L71
There was a problem hiding this comment.
Also I don't see scenario where this conversion code would suddenly start caring about type of Lists. They are just iterating through it and storing it into transfer format.
There was a problem hiding this comment.
Ok, let me give it a closer look today. It looks like I'm going to have some time set aside for Pigeon work today.
|
@gaaclarke What should I do to move this forward? Any changes I need to make to the code? |
|
@matejdro no, thank you. I have an outstanding PR for unit tests for generated java code that I wanted to land first. If i keep getting pulled away from that I'd like to test it personally at least. Hopefully I can get around to it this week, thanks for your patience. |
|
@gaaclarke Anything new? |
This resolves java side of the flutter/flutter#58913
gaaclarke
left a comment
There was a problem hiding this comment.
Sorry, I was hoping to have some extra testing capabilities for this PR. It got stalled longer than I hoped. This looks good. I might have to tweak it since I'm running into weird null safety CI issues locally.
e49ef9d to
27fc2de
Compare
Made a bunch of minor improvements to the Java generation part of the pigeon. Those should make usage of pigeon from java side a bit more convenient.
Please see commit messages for full list of improvements.