binrw icon indicating copy to clipboard operation
binrw copied to clipboard

Add `bw(if(expr))` and `brw(if(expr))` to skip writing conditionally

Open Holzhaus opened this issue 2 years ago • 2 comments

It would be great if it would be possible to skip writing of individual fields conditionally, similar to what br(if(...)) does for reading.

API could look like this:

#[binrw]
#[brw(little)]
struct Data {
    version: u16,
    some_field: u16,
    #[br(if(version >= 5, 0))]
    #[bw(if(version >= 5))]
    another_field: u32,
}

And for brevity:

#[binrw]
#[brw(little)]
struct Data {
    version: u16,
    some_field: u16,
    #[brw(if(version >= 5, 0))]
    another_field: u32,
}

Holzhaus avatar Mar 28 '22 20:03 Holzhaus

@Holzhaus the currently recommended pattern for this is using the behavior of optionals in binrw (which is that on None nothing is written).

so one potential option is using:

#[bw(map = |field| (version >= 5).then(|| field))]]

or if that's a bit too verbose/polluting, you can get even more fancy and have the following helper function:

fn write_if<T>(condition: impl FnOnce() -> bool) -> impl FnOnce(T) -> Option<T> {
    move |field| condition().then(move || field)
}

Which can then be used like so:

#[bw(map = write_if(|| version >= 5))]

I do think your idea make sense, however, as that makes sense for bidirectionality of the attribute. I had originally gotten it in my head that it didn't make sense as "not involving Default for writing doesn't make sense", however having given it more thought it sorta does? The only concern being that populating the field with a non-default value then being confused it isn't written seems possible.

(Also I should note that presently if you use Option<T> as your field type, the bidirectionality already works, with the caveat that you likely need validation to ensure you don't have Some(T) for versions where that's invalid)

jam1garner avatar Apr 16 '22 19:04 jam1garner

I had been hoping to use if for conditionally writing a constant. Something like #[bw(calc = 0, if(version > 1))]. I eventually used an Option<u32> in the struct. Nevertheless, conditional constants might be another reason to support bw(if…).

stevecheckoway avatar Apr 17 '22 19:04 stevecheckoway