flutter_rust_bridge icon indicating copy to clipboard operation
flutter_rust_bridge copied to clipboard

init suport array

Open Cupnfish opened this issue 2 years ago • 7 comments

Close #572

Checklist

  • [x] An issue to be fixed by this PR is listed above.
  • [x] New tests are added to ensure new features are working. End-to-end tests are usually in the ./frb_example/pure_dart example, more specifically, rust/src/api.rs and dart/lib/main.dart.
  • [x] The code generator is run and the code is formatted (e.g. via just refresh_all).
  • [ ] If this PR adds/changes features, documentations (in the ./book folder) are updated.
  • [x] CI is passing.

Remark for PR creator: If the PR is submitted but the owner (fzyzcjy) does not reply for a few days, maybe he just did not see it, so please ping him.

Cupnfish avatar Aug 08 '22 08:08 Cupnfish

P.S. As a side remark

After upgrade to 1.37.2, it breaks arrays like... https://github.com/fzyzcjy/flutter_rust_bridge/issues/572

We already have some support on arrays: http://cjycode.com/flutter_rust_bridge/feature/lang_vec.html

fzyzcjy avatar Aug 08 '22 08:08 fzyzcjy

👀

fzyzcjy avatar Aug 23 '22 05:08 fzyzcjy

@fzyzcjy I see that the CI error is related to a memory leak, and I was wondering how the memory check part works locally?

Cupnfish avatar Aug 28 '22 15:08 Cupnfish

how the memory check part works locally?

Look at CI script, and mimic it to run locally. https://github.com/fzyzcjy/flutter_rust_bridge/blob/master/.github/workflows/ci.yaml

fzyzcjy avatar Aug 28 '22 23:08 fzyzcjy

Btw I am a bit curious: We already have http://cjycode.com/flutter_rust_bridge/feature/lang_vec.html#t-n (https://github.com/fzyzcjy/flutter_rust_bridge/pull/442/files), i.e. array support. So could you please add a bit of doc showing what this PR does in addition to the old code?

fzyzcjy avatar Aug 28 '22 23:08 fzyzcjy

Btw I am a bit curious: We already have http://cjycode.com/flutter_rust_bridge/feature/lang_vec.html#t-n, i.e. array support. So could you please add a bit of doc showing what this PR does in addition to the old code?

The previous implementation only supported arrays of the results returned from the rust side , but not parameters , this time it mainly supported parameters as well , which I thought was supported by reading the documentation before , but it didn't support them , so the documentation didn't actually need to be changed this time because what the documentation said misled people into thinking that arrays were originally supported , but in fact only the values returned from the rust side were supported .

Cupnfish avatar Aug 29 '22 05:08 Cupnfish

I see. Looking forward to merging this PR!

fzyzcjy avatar Aug 29 '22 05:08 fzyzcjy

@fzyzcjy Learned from previous experiences and mistakes, the new refactoring has been committed for review.

Cupnfish avatar Oct 03 '22 17:10 Cupnfish

/cc @Desdaemon @Roms1383 - do you have interest in making a review?

fzyzcjy avatar Oct 04 '22 05:10 fzyzcjy

/cc @Desdaemon @Roms1383 - do you have interest in making a review?

next time I'd be honoured but still fully focused on the other PR

Roms1383 avatar Oct 06 '22 08:10 Roms1383

Good job! Busy on https://github.com/fzyzcjy/flutter_smooth, will probably merge today if no further problems :)

fzyzcjy avatar Oct 07 '22 02:10 fzyzcjy