-
Notifications
You must be signed in to change notification settings - Fork 3
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
Feature/wasm pack #10
Conversation
Cargo.toml
Outdated
@@ -28,15 +28,19 @@ bindgen = "0.69.4" | |||
[features] | |||
std = [] | |||
plaintext-before-extension = [] | |||
serde_support = ["serde", "std", "arrayvec/serde"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename this feature to just serde than make the array
["dep:serde", "std", "arrayvec/serde"]
also it is necessary to enable std to use serde? I don't think you need std to use serde
Cargo.toml
Outdated
arrayvec = "0.7.4" | ||
arrayvec = { version = "0.7.4", features = [ | ||
"serde", | ||
], optional = true, default-features = false } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is arrayvec really optional? You seem to use it without a feature gate in the code.
I think you just want
arrayvec = { version = "0.7.4", default-features = false }
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The arrayvec/serde
feature flag will enable the serde feature on arrayvec when necessary
feature = "serde_support", | ||
derive(serde::Deserialize), | ||
serde(bound(deserialize = "'de: 'a")) | ||
)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These can probably be the same cfg_attr. You don't need a separate one for each derive.
#[cfg_attr(
feature = "serde_support",
derive(serde::Serialize, serde::Deserialize),
serde(bound(deserialize = "'de: 'a"))
)]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this was not possible
src/user_data/data_information.rs
Outdated
#[derive(Debug, PartialEq)] | ||
pub struct DataInformationBlock { | ||
pub data_information_field: DataInformationField, | ||
#[serde(skip_serializing, skip_deserializing)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should also be in a cfg_attr
src/user_data/mod.rs
Outdated
StatusField::from_bits(bits) | ||
.ok_or_else(|| serde::de::Error::custom("Invalid bits for StatusField")) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can derive these with cfg_attr in the bitflags macro.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
somehow this was not working. Where do I need to put it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what error does it show? I am doing this cargo check and it shows no error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, you also need to add bitflags/serde to the serde feature flag
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made a pr with this #11
wasm/src/lib.rs
Outdated
|
||
#[cfg(feature = "wee_alloc")] | ||
#[global_allocator] | ||
static ALLOC: wee_alloc::WeeAlloc = wee_alloc::WeeAlloc::INIT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't use weealloc, its very broken. See issues:
bitflags = "2.4.2" | ||
arrayvec = "0.7.4" | ||
arrayvec = { version = "0.7.4", optional = false, default-features = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to say optional = false
. It has no effect so you can leave it.
86f1ead
to
4d7107d
Compare
I've added wasm package.
I am looking into using serde crate. Need to know if this is correct.
Serde should be avilable in cli and wasm but not as an option in the lib