Cleanup a1ae45add88e424710073711935a3605cafe9ef9 a bit.

Emmanuel Gil Peyrot created

Change summary

src/bind.rs          | 11 ++----
src/blocking.rs      |  1 
src/data_forms.rs    | 29 +++++++++-------
src/disco.rs         |  4 +-
src/ibr.rs           | 12 +++---
src/jingle.rs        | 10 ++--
src/jingle_ft.rs     | 80 ++++++++++++---------------------------------
src/jingle_s5b.rs    | 17 +++------
src/mam.rs           | 14 +------
src/presence.rs      | 12 ++----
src/pubsub/event.rs  | 22 ++++--------
src/pubsub/pubsub.rs | 12 +++---
src/rsm.rs           | 13 ++-----
src/sasl.rs          |  1 
src/stanza_error.rs  |  5 +-
src/time.rs          |  6 +--
src/util/macros.rs   | 12 ++----
src/xhtml.rs         | 10 ++--
18 files changed, 97 insertions(+), 174 deletions(-)

Detailed changes

src/bind.rs 🔗

@@ -63,13 +63,10 @@ impl From<BindQuery> for Element {
     fn from(bind: BindQuery) -> Element {
         Element::builder("bind")
             .ns(ns::BIND)
-            .append_all((match bind.resource {
-                None => vec![],
-                Some(resource) => vec![Element::builder("resource")
+            .append_all(bind.resource.map(|resource|
+                Element::builder("resource")
                     .ns(ns::BIND)
-                    .append(resource)
-                    .build()],
-            }).into_iter())
+                    .append(resource)))
             .build()
     }
 }
@@ -129,7 +126,7 @@ impl From<BindResponse> for Element {
     fn from(bind: BindResponse) -> Element {
         Element::builder("bind")
             .ns(ns::BIND)
-            .append(Element::builder("jid").ns(ns::BIND).append(bind.jid).build())
+            .append(Element::builder("jid").ns(ns::BIND).append(bind.jid))
             .build()
     }
 }

src/blocking.rs 🔗

@@ -55,7 +55,6 @@ macro_rules! generate_blocking_element {
                              Element::builder("item")
                                      .ns(ns::BLOCKING)
                                      .attr("jid", jid)
-                                     .build()
                          }))
                         .build()
             }

src/data_forms.rs 🔗

@@ -7,7 +7,7 @@
 use crate::util::error::Error;
 use crate::media_element::MediaElement;
 use crate::ns;
-use minidom::{Element, Node};
+use minidom::Element;
 use std::convert::TryFrom;
 
 generate_element!(
@@ -152,12 +152,12 @@ impl From<Field> for Element {
             .attr("var", field.var)
             .attr("type", field.type_)
             .attr("label", field.label)
-            .append_all((if field.required {
-                Some(Element::builder("required").ns(ns::DATA_FORMS).build())
+            .append_all(if field.required {
+                Some(Element::builder("required").ns(ns::DATA_FORMS))
             } else {
                 None
-            }).into_iter())
-            .append_all(field.options.iter().cloned().map(Element::from).map(Node::Element).into_iter())
+            })
+            .append_all(field.options.iter().cloned().map(Element::from))
             .append_all(
                 field
                     .values
@@ -166,10 +166,9 @@ impl From<Field> for Element {
                         Element::builder("value")
                             .ns(ns::DATA_FORMS)
                             .append(value)
-                            .build()
                     })
             )
-            .append_all(field.media.iter().cloned().map(Element::from).map(Node::Element))
+            .append_all(field.media.iter().cloned().map(Element::from))
             .build()
     }
 }
@@ -283,12 +282,16 @@ impl From<DataForm> for Element {
                     .ns(ns::DATA_FORMS)
                     .append(text)
             }))
-            .append_all((if let Some(form_type) = form.form_type {
-                vec![Element::builder("field").ns(ns::DATA_FORMS).attr("var", "FORM_TYPE").attr("type", "hidden").append(Element::builder("value").ns(ns::DATA_FORMS).append(form_type).build()).build()]
-            } else {
-                vec![]
-            }).into_iter())
-            .append_all(form.fields.iter().cloned().map(Element::from).map(Node::Element))
+            .append_all(form.form_type.map(|form_type| {
+                Element::builder("field")
+                    .ns(ns::DATA_FORMS)
+                    .attr("var", "FORM_TYPE")
+                    .attr("type", "hidden")
+                    .append(Element::builder("value")
+                        .ns(ns::DATA_FORMS)
+                        .append(form_type))
+            }))
+            .append_all(form.fields.iter().cloned().map(Element::from))
             .build()
     }
 }

src/disco.rs 🔗

@@ -9,7 +9,7 @@ use crate::util::error::Error;
 use crate::iq::{IqGetPayload, IqResultPayload};
 use crate::ns;
 use jid::Jid;
-use minidom::{Element, Node};
+use minidom::Element;
 use std::convert::TryFrom;
 
 generate_element!(
@@ -180,7 +180,7 @@ impl From<DiscoInfoResult> for Element {
             .attr("node", disco.node)
             .append_all(disco.identities.into_iter())
             .append_all(disco.features.into_iter())
-            .append_all(disco.extensions.iter().cloned().map(Element::from).map(Node::Element))
+            .append_all(disco.extensions.iter().cloned().map(Element::from))
             .build()
     }
 }

src/ibr.rs 🔗

@@ -8,7 +8,7 @@ use crate::data_forms::DataForm;
 use crate::util::error::Error;
 use crate::iq::{IqGetPayload, IqResultPayload, IqSetPayload};
 use crate::ns;
-use minidom::{Element, Node};
+use minidom::Element;
 use std::collections::HashMap;
 use std::convert::TryFrom;
 
@@ -93,23 +93,23 @@ impl From<Query> for Element {
     fn from(query: Query) -> Element {
         Element::builder("query")
             .ns(ns::REGISTER)
-            .append_all((if query.registered {
+            .append_all(if query.registered {
                 Some(Element::builder("registered").ns(ns::REGISTER))
             } else {
                 None
-            }).into_iter())
+            })
             .append_all(
                 query
                     .fields
                     .into_iter()
                     .map(|(name, value)| Element::builder(name).ns(ns::REGISTER).append(value))
             )
-            .append_all((if query.remove {
+            .append_all(if query.remove {
                 Some(Element::builder("remove").ns(ns::REGISTER))
             } else {
                 None
-            }).into_iter())
-            .append_all(query.form.map(Element::from).map(Node::Element).into_iter())
+            })
+            .append_all(query.form.map(Element::from))
             .build()
     }
 }

src/jingle.rs 🔗

@@ -8,7 +8,7 @@ use crate::util::error::Error;
 use crate::iq::IqSetPayload;
 use crate::ns;
 use jid::Jid;
-use minidom::{Element, Node};
+use minidom::Element;
 use std::collections::BTreeMap;
 use std::str::FromStr;
 use std::convert::TryFrom;
@@ -351,7 +351,8 @@ impl From<Reason> for Element {
             Reason::UnsupportedApplications => "unsupported-applications",
             Reason::UnsupportedTransports => "unsupported-transports",
         })
-        .build()
+            .ns(ns::JINGLE)
+            .build()
     }
 }
 
@@ -419,7 +420,6 @@ impl From<ReasonElement> for Element {
                         .ns(ns::JINGLE)
                         .attr("xml:lang", lang)
                         .append(text)
-                        .build()
                 }))
             .build()
     }
@@ -542,8 +542,8 @@ impl From<Jingle> for Element {
             .attr("initiator", jingle.initiator)
             .attr("responder", jingle.responder)
             .attr("sid", jingle.sid)
-            .append_all(jingle.contents.into_iter())
-            .append_all(jingle.reason.map(Element::from).map(Node::Element).into_iter())
+            .append_all(jingle.contents)
+            .append_all(jingle.reason.map(Element::from))
             .build()
     }
 }

src/jingle_ft.rs 🔗

@@ -193,65 +193,27 @@ impl TryFrom<Element> for File {
 
 impl From<File> for Element {
     fn from(file: File) -> Element {
-        let mut root = Element::builder("file").ns(ns::JINGLE_FT);
-        if let Some(date) = file.date {
-            root = root.append(
-                Node::Element(
-                    Element::builder("date")
-                        .ns(ns::JINGLE_FT)
-                        .append(date)
-                        .build()
-                )
-            );
-        }
-        if let Some(media_type) = file.media_type {
-            root = root.append(
-                Node::Element(
-                    Element::builder("media-type")
-                        .ns(ns::JINGLE_FT)
-                        .append(media_type)
-                        .build()
-                )
-            );
-        }
-        if let Some(name) = file.name {
-            root = root.append(
-                Node::Element(
-                    Element::builder("name")
-                        .ns(ns::JINGLE_FT)
-                        .append(name)
-                        .build()
-                )
-            );
-        }
-        for (lang, desc) in file.descs.into_iter() {
-            root = root.append(
-                Node::Element(
-                    Element::builder("desc")
-                        .ns(ns::JINGLE_FT)
-                        .attr("xml:lang", lang)
-                        .append(desc.0)
-                        .build()
-                )
-            );
-        }
-        if let Some(size) = file.size {
-            root = root.append(
-                Node::Element(
-                    Element::builder("size")
-                        .ns(ns::JINGLE_FT)
-                        .append(format!("{}", size))
-                        .build()
-                )
-            );
-        }
-        if let Some(range) = file.range {
-            root = root.append(range);
-        }
-        for hash in file.hashes {
-            root = root.append(hash);
-        }
-        root.build()
+        Element::builder("file")
+            .ns(ns::JINGLE_FT)
+            .append_all(file.date.map(|date|
+                Element::builder("date")
+                    .append(date)))
+            .append_all(file.media_type.map(|media_type|
+                Element::builder("media-type")
+                    .append(media_type)))
+            .append_all(file.name.map(|name|
+                Element::builder("name")
+                    .append(name)))
+            .append_all(file.descs.into_iter().map(|(lang, desc)|
+                Element::builder("desc")
+                    .attr("xml:lang", lang)
+                    .append(desc.0)))
+            .append_all(file.size.map(|size|
+                Element::builder("size")
+                    .append(format!("{}", size))))
+            .append_all(file.range)
+            .append_all(file.hashes)
+            .build()
     }
 }
 

src/jingle_s5b.rs 🔗

@@ -251,27 +251,22 @@ impl From<Transport> for Element {
                 TransportPayload::Candidates(candidates) => candidates
                     .into_iter()
                     .map(Element::from)
-                    .collect::<Vec<_>>()
-                    .into_iter(),
+                    .collect::<Vec<_>>(),
                 TransportPayload::Activated(cid) => vec![Element::builder("activated")
                     .ns(ns::JINGLE_S5B)
                     .attr("cid", cid)
-                    .build()]
-                    .into_iter(),
+                    .build()],
                 TransportPayload::CandidateError => vec![Element::builder("candidate-error")
                     .ns(ns::JINGLE_S5B)
-                    .build()]
-                    .into_iter(),
+                    .build()],
                 TransportPayload::CandidateUsed(cid) => vec![Element::builder("candidate-used")
                     .ns(ns::JINGLE_S5B)
                     .attr("cid", cid)
-                    .build()]
-                    .into_iter(),
+                    .build()],
                 TransportPayload::ProxyError => vec![Element::builder("proxy-error")
                     .ns(ns::JINGLE_S5B)
-                    .build()]
-                    .into_iter(),
-                TransportPayload::None => vec![].into_iter(),
+                    .build()],
+                TransportPayload::None => vec![],
             })
             .build()
     }

src/mam.rs 🔗

@@ -171,17 +171,9 @@ fn serialise_jid_list(name: &str, jids: Vec<Jid>) -> ::std::option::IntoIter<Nod
                 .append_all(
                     jids.into_iter()
                         .map(|jid|
-                            Node::Element(
-                                Element::builder("jid")
-                                    .ns(ns::MAM)
-                                    .append(Node::Text(String::from(jid)))
-                                    .build()
-                                    .into()
-                            )
-                        )
-                        .into_iter(),
-                )
-                .build()
+                            Element::builder("jid")
+                                .ns(ns::MAM)
+                                .append(String::from(jid))))
                 .into(),
         ).into_iter()
     }

src/presence.rs 🔗

@@ -324,18 +324,14 @@ impl From<Presence> for Element {
                                 },
                             )
                             .append(status)
-                            .build()
                     })
             )
-            .append_all((if presence.priority == 0 {
+            .append_all(if presence.priority == 0 {
                 None
             } else {
-                Some(
-                    Element::builder("priority")
-                        .append(format!("{}", presence.priority))
-                        .build()
-                )
-            }).into_iter())
+                Some(Element::builder("priority")
+                    .append(format!("{}", presence.priority)))
+            })
             .append_all(presence.payloads.into_iter())
             .build()
     }

src/pubsub/event.rs 🔗

@@ -10,7 +10,7 @@ use crate::util::error::Error;
 use crate::ns;
 use crate::pubsub::{ItemId, NodeName, Subscription, SubscriptionId, Item as PubSubItem};
 use jid::Jid;
-use minidom::{Element, Node};
+use minidom::Element;
 use std::convert::TryFrom;
 
 /// Event wrapper for a PubSub `<item/>`.
@@ -198,8 +198,7 @@ impl From<PubSubEvent> for Element {
             PubSubEvent::Configuration { node, form } => Element::builder("configuration")
                 .ns(ns::PUBSUB_EVENT)
                 .attr("node", node)
-                .append_all(form.map(Element::from).map(Node::Element).into_iter())
-                .build(),
+                .append_all(form.map(Element::from)),
             PubSubEvent::Delete { node, redirect } => Element::builder("purge")
                 .ns(ns::PUBSUB_EVENT)
                 .attr("node", node)
@@ -207,14 +206,11 @@ impl From<PubSubEvent> for Element {
                     Element::builder("redirect")
                         .ns(ns::PUBSUB_EVENT)
                         .attr("uri", redirect)
-                        .build()
-                }))
-                .build(),
+                })),
             PubSubEvent::PublishedItems { node, items } => Element::builder("items")
                 .ns(ns::PUBSUB_EVENT)
                 .attr("node", node)
-                .append_all(items.into_iter())
-                .build(),
+                .append_all(items.into_iter()),
             PubSubEvent::RetractedItems { node, items } => Element::builder("items")
                 .ns(ns::PUBSUB_EVENT)
                 .attr("node", node)
@@ -225,14 +221,11 @@ impl From<PubSubEvent> for Element {
                             Element::builder("retract")
                                 .ns(ns::PUBSUB_EVENT)
                                 .attr("id", id)
-                                .build()
                         })
-                )
-                .build(),
+                ),
             PubSubEvent::Purge { node } => Element::builder("purge")
                 .ns(ns::PUBSUB_EVENT)
-                .attr("node", node)
-                .build(),
+                .attr("node", node),
             PubSubEvent::Subscription {
                 node,
                 expiry,
@@ -245,8 +238,7 @@ impl From<PubSubEvent> for Element {
                 .attr("expiry", expiry)
                 .attr("jid", jid)
                 .attr("subid", subid)
-                .attr("subscription", subscription)
-                .build(),
+                .attr("subscription", subscription),
         };
         Element::builder("event")
             .ns(ns::PUBSUB_EVENT)

src/pubsub/pubsub.rs 🔗

@@ -219,11 +219,11 @@ impl From<SubscribeOptions> for Element {
     fn from(subscribe_options: SubscribeOptions) -> Element {
         Element::builder("subscribe-options")
             .ns(ns::PUBSUB)
-            .append_all((if subscribe_options.required {
-                vec![Element::builder("required").ns(ns::PUBSUB).build()]
+            .append_all(if subscribe_options.required {
+                Some(Element::builder("required").ns(ns::PUBSUB))
             } else {
-                vec![]
-            }).into_iter())
+                None
+            })
             .build()
     }
 }
@@ -473,7 +473,7 @@ impl From<PubSub> for Element {
     fn from(pubsub: PubSub) -> Element {
         Element::builder("pubsub")
             .ns(ns::PUBSUB)
-            .append_all((match pubsub {
+            .append_all(match pubsub {
                 PubSub::Create { create, configure } => {
                     let mut elems = vec![Element::from(create)];
                     if let Some(configure) = configure {
@@ -498,7 +498,7 @@ impl From<PubSub> for Element {
                 PubSub::Subscription(subscription) => vec![Element::from(subscription)],
                 PubSub::Subscriptions(subscriptions) => vec![Element::from(subscriptions)],
                 PubSub::Unsubscribe(unsubscribe) => vec![Element::from(unsubscribe)],
-            }).into_iter())
+            })
             .build()
     }
 }

src/rsm.rs 🔗

@@ -6,7 +6,7 @@
 
 use crate::util::error::Error;
 use crate::ns;
-use minidom::{Element, Node};
+use minidom::Element;
 use std::convert::TryFrom;
 
 /// Requests paging through a potentially big set of items (represented by an
@@ -76,23 +76,20 @@ impl From<SetQuery> for Element {
                 Element::builder("max")
                     .ns(ns::RSM)
                     .append(format!("{}", max))
-                    .build()
             }))
             .append_all(
                 set.after
-                    .map(|after| Element::builder("after").ns(ns::RSM).append(after).build()),
+                    .map(|after| Element::builder("after").ns(ns::RSM).append(after))
             )
             .append_all(set.before.map(|before| {
                 Element::builder("before")
                     .ns(ns::RSM)
                     .append(before)
-                    .build()
             }))
             .append_all(set.index.map(|index| {
                 Element::builder("index")
                     .ns(ns::RSM)
                     .append(format!("{}", index))
-                    .build()
             }))
             .build()
     }
@@ -158,20 +155,18 @@ impl From<SetResult> for Element {
                 .ns(ns::RSM)
                 .attr("index", set.first_index)
                 .append(first)
-                .build()
         });
         Element::builder("set")
             .ns(ns::RSM)
-            .append_all(first.map(Element::from).map(Node::Element).into_iter())
+            .append_all(first)
             .append_all(
                 set.last
-                    .map(|last| Element::builder("last").ns(ns::RSM).append(last).build()),
+                    .map(|last| Element::builder("last").ns(ns::RSM).append(last)),
             )
             .append_all(set.count.map(|count| {
                 Element::builder("count")
                     .ns(ns::RSM)
                     .append(format!("{}", count))
-                    .build()
             }))
             .build()
     }

src/sasl.rs 🔗

@@ -212,7 +212,6 @@ impl From<Failure> for Element {
                             .ns(ns::SASL)
                             .attr("xml:lang", lang)
                             .append(text)
-                            .build()
                     })
             )
             .build()

src/stanza_error.rs 🔗

@@ -9,7 +9,7 @@ use crate::message::MessagePayload;
 use crate::ns;
 use crate::presence::PresencePayload;
 use jid::Jid;
-use minidom::{Element, Node};
+use minidom::Element;
 use std::collections::BTreeMap;
 use std::convert::TryFrom;
 
@@ -300,10 +300,9 @@ impl From<StanzaError> for Element {
                         .ns(ns::XMPP_STANZAS)
                         .attr("xml:lang", lang)
                         .append(text)
-                        .build()
                 })
             )
-            .append_all(err.other.map(Element::from).map(Node::Element).into_iter())
+            .append_all(err.other)
             .build()
     }
 }

src/time.rs 🔗

@@ -78,11 +78,9 @@ impl From<TimeResult> for Element {
         Element::builder("time")
             .ns(ns::TIME)
             .append(Element::builder("tzo")
-                    .append(format!("{}", time.0.timezone()))
-                    .build())
+                    .append(format!("{}", time.0.timezone())))
             .append(Element::builder("utc")
-                    .append(time.0.with_timezone(FixedOffset::east(0)).format("%FT%TZ"))
-                    .build())
+                    .append(time.0.with_timezone(FixedOffset::east(0)).format("%FT%TZ")))
             .build()
     }
 }

src/util/macros.rs 🔗

@@ -570,12 +570,10 @@ macro_rules! finish_parse_elem {
 
 macro_rules! generate_serialiser {
     ($builder:ident, $parent:ident, $elem:ident, Required, String, ($name:tt, $ns:ident)) => {
-        $builder.append(::minidom::Node::Element(
+        $builder.append(
             ::minidom::Element::builder($name)
                 .ns(crate::ns::$ns)
                 .append(::minidom::Node::Text($parent.$elem))
-                .build()
-            )
         )
     };
     ($builder:ident, $parent:ident, $elem:ident, Option, String, ($name:tt, $ns:ident)) => {
@@ -583,8 +581,7 @@ macro_rules! generate_serialiser {
                 ::minidom::Element::builder($name)
                     .ns(crate::ns::$ns)
                     .append(::minidom::Node::Text(elem))
-                    .build()
-            }).into_iter()
+            })
         )
     };
     ($builder:ident, $parent:ident, $elem:ident, Option, $constructor:ident, ($name:tt, $ns:ident)) => {
@@ -592,8 +589,7 @@ macro_rules! generate_serialiser {
                 ::minidom::Element::builder($name)
                     .ns(crate::ns::$ns)
                     .append(::minidom::Node::Element(::minidom::Element::from(elem)))
-                    .build()
-            }).into_iter()
+            })
         )
     };
     ($builder:ident, $parent:ident, $elem:ident, Vec, $constructor:ident, ($name:tt, $ns:ident)) => {
@@ -736,7 +732,7 @@ macro_rules! impl_pubsub_item {
                     .ns(ns::$ns)
                     .attr("id", item.0.id)
                     .attr("publisher", item.0.publisher)
-                    .append_all(item.0.payload.map(::minidom::Node::Element).into_iter())
+                    .append_all(item.0.payload)
                     .build()
             }
         }

src/xhtml.rs 🔗

@@ -98,13 +98,13 @@ impl From<XhtmlIm> for Element {
     fn from(wrapper: XhtmlIm) -> Element {
         Element::builder("html")
             .ns(ns::XHTML_IM)
-            .append_all(wrapper.bodies.into_iter().map(|(ref lang, ref body)| {
+            .append_all(wrapper.bodies.into_iter().map(|(lang, body)| {
                 if lang.is_empty() {
                     assert!(body.xml_lang.is_none());
                 } else {
-                    assert_eq!(Some(lang), body.xml_lang.as_ref());
+                    assert_eq!(Some(lang), body.xml_lang);
                 }
-                Element::from(body.clone())
+                Element::from(body)
             }))
             .build()
     }
@@ -346,11 +346,11 @@ impl From<Tag> for Element {
     }
 }
 
-fn children_to_nodes(children: Vec<Child>) -> Vec<Node> {
+fn children_to_nodes(children: Vec<Child>) -> impl IntoIterator<Item = Node> {
     children.into_iter().map(|child| match child {
         Child::Tag(tag) => Node::Element(Element::from(tag)),
         Child::Text(text) => Node::Text(text),
-    }).collect::<Vec<_>>()
+    })
 }
 
 fn children_to_html(children: Vec<Child>) -> String {