wayland-rs icon indicating copy to clipboard operation
wayland-rs copied to clipboard

feat: let delegate_noop can handle constGeneric

Open Decodetalkers opened this issue 1 year ago • 8 comments

the macro of delegate_noop cannot handle like struct A<const X: unsize>, such kind of struct, so I want to fix it

Decodetalkers avatar Nov 29 '23 09:11 Decodetalkers

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (fdd88c4) 73.03% compared to head (2fca6dc) 73.03%.

:exclamation: Current head 2fca6dc differs from pull request most recent head bd57467. Consider uploading reports for the commit bd57467 to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #679   +/-   ##
=======================================
  Coverage   73.03%   73.03%           
=======================================
  Files          48       48           
  Lines        7841     7841           
=======================================
  Hits         5727     5727           
  Misses       2114     2114           
Flag Coverage Δ
main 58.32% <ø> (ø)
test-- 78.07% <ø> (ø)
test--server_system 61.07% <ø> (ø)
test-client_system- 68.90% <ø> (ø)
test-client_system-server_system 51.32% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Nov 29 '23 13:11 codecov[bot]

Thanks, the principle looks good, I'll take some time later to carefully review the macro.

More generally for this change, would you mind:

  • Add some text in the documentation of the macro explaining how to use it
  • Introduce the same change in wayland-server as well which has a similar macro
  • Add relevant entries to the changelog of the two crates

elinorbgr avatar Nov 29 '23 13:11 elinorbgr

Thanks, the principle looks good, I'll take some time later to carefully review the macro.

More generally for this change, would you mind:

* Add some text in the documentation of the macro explaining how to use it

* Introduce the same change in `wayland-server` as well which has a similar macro

* Add relevant entries to the changelog of the two crates

Sure, I am happy to do this

Decodetalkers avatar Nov 29 '23 14:11 Decodetalkers

Does the macro as it is here support types with both const-generic and regular generics? Something like App<const U: usize, T> ?

elinorbgr avatar Nov 30 '23 09:11 elinorbgr

I just let const before :, so the words after : will also work I think

Decodetalkers avatar Nov 30 '23 11:11 Decodetalkers

I'm not sure about that, it seems to me that the pattern you used, $(@< $( const $lt:tt $( : $clt:tt $(+ $dlt:tt )* )? ),+ >)? would match a list like const U: usize, const V: usize, ..., but not a list mixing const arguments and regular type arguments?

elinorbgr avatar Dec 04 '23 13:12 elinorbgr

Ah.. I see, I fogot that generic types can be muti types..

Decodetalkers avatar Dec 04 '23 14:12 Decodetalkers

Ok, now it is ok.. I try add muti generic type,

like

struct A<const X: usize, Y: Sized, Z> {
     it: Z
}

it seems ok

Decodetalkers avatar Dec 04 '23 15:12 Decodetalkers