Add support for serde(skip)
#14
Conversation
|
@robin-nitrokey, @sosthene-nitrokey, could this be merged? It would help us address linux-credentials/libwebauthn#74. TIA! |
|
Generally yes, but I’m not sure how we should handle indexing with skipped fields. Not incrementing the index for a skipped field makes sense, but it would silently change the serialization format so we would need to be very careful when upgrading. Alternatively, we could move to explicit indices (#17). @AlfioEmanueleFresta Do you have a preference regarding the index? |
|
Thanks @robin-nitrokey! I agree explicit indexing sounds like the best option. Our use case is to skip unused indexes in CTAP spec models (eg. see linux-credentials/libwebauthn#74), so either incrementing the index automatically, or explicit indexing, would be needed. |
|
Okay. I think we could release a v0.1.2 that implements |
…75) Fixes #74, causing an error with `change_pin_hid` with Solo2 keys. Unconditional skips aren't supported by `serde-indexed` - this includes either `serde(skip_serializing)` and `serde(skip)`. This change works around the limitation by using `skip_serializing_if` alongside a method which returns always true. Long term fix is trussed-dev/serde-indexed#14 (unmerged since Feb 2024) - pinged authors and contributors to see if this can be merged.
|
That sounds good! I would also merge all the other PRs for 0.1.2. Also note that once serde adds support for |
src/parse.rs
Outdated
| if !matches!(skip_serializing_if, Skip::Always) { | ||
| idx += 1; | ||
| } |
There was a problem hiding this comment.
Per the discussion in this thread, we should increment the index even if the field is skipped.
There was a problem hiding this comment.
I'll add an option to be able to not increment. (Default would be to increment).
sosthene-nitrokey
force-pushed
the
skip
branch
2 times, most recently
from
6612176 to
2c3d400
Compare
src/parse.rs
Outdated
| pub enum Skip { | ||
| None, | ||
| If(syn::ExprPath), | ||
| Always, | ||
| } |
There was a problem hiding this comment.
| pub enum Skip { | |
| None, | |
| If(syn::ExprPath), | |
| Always, | |
| } | |
| pub enum Skip { | |
| Never, | |
| If(syn::ExprPath), | |
| Always, | |
| } |
Skip::None could also be read as skip None values.
skipcauses the index to not be increased to match deserialization from upstreamserde.