search

RazrFalcon / xmlparser

A low-level, pull-based, zero-allocation XML 1.0 parser.
Apache License 2.0
130 stars 16 forks source link

Derive more auto-derivable traits #12

Closed jfrimmel closed 4 years ago

jfrimmel commented 4 years ago

Hello, I'm an active user of this crate and noticed, that some types of this crate don't have [Partial]Eq implemented, despite it being possible. I searched the code base and derived some more traits.

As this is only a simple addition, there shouln't be any fallout. Tests are passing locally.

RazrFalcon commented 4 years ago

Is there a reason to do this? I can understand why someone may want to compare some structs, but why to put them in a HashMap?

jfrimmel commented 4 years ago

Regarding Hash: the Rust API Guidelines suggest it, but I see your point, that nobody will likely ever put the types into a HashMap. But implementing it does no harm either.

What do you think, should I remove the Hash derive?

RazrFalcon commented 4 years ago

Ok, let's keep them. But reformat them to #[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]. And modify only public types, e.g. keep State as is.

jfrimmel commented 4 years ago

Okay, I'll amend the commit and force-push.

jfrimmel commented 4 years ago

Applied your feedback and force-pushed.

RazrFalcon commented 4 years ago

Thanks. Can you do the same for roxmltree patch?