search

Portable-Network-Archive / liblzma-rs

Bindings to liblzma in Rust (xz streams in Rust)
Apache License 2.0
18 stars 5 forks source link

Added interface for work with lzma_properties_decode #126

Closed ikrivosheev closed 3 months ago

ikrivosheev commented 3 months ago

Hello! Thank you for working on the library. I have to decode properties using lzma_properties_decode: https://tukaani.org/xz/liblzma-api/filter_8h.html#a88d2e864b2039ac82802cc202278d478.

I'll make a PR, but I have a question. At the moment library has struct Filters. I want add methods like: <method>_raw(&mut self, properties: &[u8]) -> &mut Filters. Is it ok?

ChanTsune commented 3 months ago

Hi @ikrivosheev!

Thank you for checking in advance before the PR!

I agree with adding ways to make it easier to use from Rust. However, since lzma_properties_decode may return an error if an inappropriate argument is given, I think it is better to use Result<&mut Self, Error> as the return type of the method! Additionally, adding raw to a method name might give the impression that the operation is unsafe, so if the operation is not unsafe, it might not be necessary to include raw. Of course, I think if this operation directly or indirectly causes a panic, it would be appropriate to treat it as an unsafe method and name it raw.

ikrivosheev commented 3 months ago

Hi @ikrivosheev!

Thank you for checking in advance before the PR!

I agree with adding ways to make it easier to use from Rust. However, since lzma_properties_decode may return an error if an inappropriate argument is given, I think it is better to use Result<&mut Self, Error> as the return type of the method! Additionally, adding raw to a method name might give the impression that the operation is unsafe, so if the operation is not unsafe, it might not be necessary to include raw. Of course, I think if this operation directly or indirectly causes a panic, it would be appropriate to treat it as an unsafe method and name it raw.

Great, I'll make a draft pr.

ikrivosheev commented 3 months ago

@ChanTsune I've made a PR. Could you review it?