Skip to content

Use SetterInput for more setters #578

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

griff
Copy link
Contributor

@griff griff commented Aug 11, 2025

This changes more of the setters both manually implemented and generated to accept a SetterInput.

This makes it useful to implement SetterInput on your own types to support using them with setters.

This is especially useful when you have plain rust structs the represent the capnp-struct and you just want convert one to the other and is my main use case.

I don't know if this is a breaking change like how originally implementing SetterInput was.

Copy link

codecov bot commented Aug 11, 2025

Codecov Report

❌ Patch coverage is 65.38462% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 50.75%. Comparing base (ab342b3) to head (083b39c).
⚠️ Report is 192 commits behind head on master.

Files with missing lines Patch % Lines
capnpc/src/codegen.rs 0.00% 9 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #578      +/-   ##
==========================================
- Coverage   51.64%   50.75%   -0.90%     
==========================================
  Files          69       70       +1     
  Lines       33735    32315    -1420     
==========================================
- Hits        17422    16400    -1022     
+ Misses      16313    15915     -398     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@griff griff force-pushed the feat/setter-input-everywhere branch from a112ca3 to f02e9d4 Compare August 11, 2025 09:47
@dwrensha
Copy link
Member

Thanks. I can see how this could be useful, but I'm a bit worried about adding more complexity to the user-facing interfaces in the generated code.

@griff
Copy link
Contributor Author

griff commented Aug 11, 2025

What do you mean by "adding more complexity"? You can already implement SetterInput for your own structs and use that where the setter supports it so Data and Text. this only makes SetterInput useful in other places.

But I can see making it more useful means more people using what might be considered an internal trait. If it is truly internal have you considered sealing it so others can't implement it then?

Also how would you then see making it easier to convert between generated types and plain rust structs?

@griff griff force-pushed the feat/setter-input-everywhere branch from f02e9d4 to 083b39c Compare August 15, 2025 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants