data_forms: Switch to Into/TryFrom.

src/data_forms.rs

@@ -53,70 +53,72 @@ pub struct DataForm {
     pub fields: Vec<Field>,
 }
 
-pub fn parse_data_form(root: &Element) -> Result<DataForm, Error> {
+impl<'a> TryFrom<&'a Element> for DataForm {
-    if !root.is("x", ns::DATA_FORMS) {
+    type Error = Error;
-        return Err(Error::ParseError("This is not a data form element."));
-    }
 
-    let type_: DataFormType = match root.attr("type") {
+    fn try_from(elem: &'a Element) -> Result<DataForm, Error> {
-        Some(type_) => type_.parse()?,
+        if !elem.is("x", ns::DATA_FORMS) {
-        None => return Err(Error::ParseError("Type attribute on data form is mandatory.")),
+            return Err(Error::ParseError("This is not a data form element."));
-    };
-    let mut fields = vec!();
-    let mut form_type = None;
-    for field in root.children() {
-        if field.is("field", ns::DATA_FORMS) {
-            let var = field.attr("var").ok_or(Error::ParseError("Field must have a 'var' attribute."))?;
-            let field_type = field.attr("type").unwrap_or("text-single");
-            let label = field.attr("label").and_then(|label| label.parse().ok());
-            let mut values = vec!();
-            let mut media = vec!();
-            for element in field.children() {
-                if element.is("value", ns::DATA_FORMS) {
-                    values.push(element.text());
-                } else if element.is("media", ns::MEDIA_ELEMENT) {
-                    match MediaElement::try_from(element) {
-                        Ok(media_element) => media.push(media_element),
-                        Err(_) => (), // TODO: is it really nice to swallow this error?
-                    }
-                } else {
-                    return Err(Error::ParseError("Field child isn’t a value or media element."));
-                }
-            }
-            if var == "FORM_TYPE" && field_type == "hidden" {
-                if form_type != None {
-                    return Err(Error::ParseError("More than one FORM_TYPE in a data form."));
-                }
-                if values.len() != 1 {
-                    return Err(Error::ParseError("Wrong number of values in FORM_TYPE."));
-                }
-                form_type = Some(values[0].clone());
-            }
-            fields.push(Field {
-                var: var.to_owned(),
-                type_: field_type.to_owned(),
-                label: label,
-                values: values,
-                media: media,
-            });
-        } else {
-            return Err(Error::ParseError("Unknown field type in data form."));
         }
+
+        let type_: DataFormType = match elem.attr("type") {
+            Some(type_) => type_.parse()?,
+            None => return Err(Error::ParseError("Type attribute on data form is mandatory.")),
+        };
+        let mut fields = vec!();
+        let mut form_type = None;
+        for field in elem.children() {
+            if field.is("field", ns::DATA_FORMS) {
+                let var = field.attr("var").ok_or(Error::ParseError("Field must have a 'var' attribute."))?;
+                let field_type = field.attr("type").unwrap_or("text-single");
+                let label = field.attr("label").and_then(|label| label.parse().ok());
+                let mut values = vec!();
+                let mut media = vec!();
+                for element in field.children() {
+                    if element.is("value", ns::DATA_FORMS) {
+                        values.push(element.text());
+                    } else if element.is("media", ns::MEDIA_ELEMENT) {
+                        match MediaElement::try_from(element) {
+                            Ok(media_element) => media.push(media_element),
+                            Err(_) => (), // TODO: is it really nice to swallow this error?
+                        }
+                    } else {
+                        return Err(Error::ParseError("Field child isn’t a value or media element."));
+                    }
+                }
+                if var == "FORM_TYPE" && field_type == "hidden" {
+                    if form_type != None {
+                        return Err(Error::ParseError("More than one FORM_TYPE in a data form."));
+                    }
+                    if values.len() != 1 {
+                        return Err(Error::ParseError("Wrong number of values in FORM_TYPE."));
+                    }
+                    form_type = Some(values[0].clone());
+                }
+                fields.push(Field {
+                    var: var.to_owned(),
+                    type_: field_type.to_owned(),
+                    label: label,
+                    values: values,
+                    media: media,
+                });
+            } else {
+                return Err(Error::ParseError("Unknown field type in data form."));
+            }
+        }
+        Ok(DataForm { type_: type_, form_type: form_type, fields: fields })
     }
-    Ok(DataForm { type_: type_, form_type: form_type, fields: fields })
 }
 
 #[cfg(test)]
 mod tests {
-    use minidom::Element;
+    use super::*;
-    use error::Error;
-    use data_forms;
 
     #[test]
     fn test_simple() {
         let elem: Element = "<x xmlns='jabber:x:data' type='result'/>".parse().unwrap();
-        let form = data_forms::parse_data_form(&elem).unwrap();
+        let form = DataForm::try_from(&elem).unwrap();
-        assert_eq!(form.type_, data_forms::DataFormType::Result_);
+        assert_eq!(form.type_, DataFormType::Result_);
         assert!(form.form_type.is_none());
         assert!(form.fields.is_empty());
     }
@@ -124,7 +126,7 @@ mod tests {
     #[test]
     fn test_invalid() {
         let elem: Element = "<x xmlns='jabber:x:data'/>".parse().unwrap();
-        let error = data_forms::parse_data_form(&elem).unwrap_err();
+        let error = DataForm::try_from(&elem).unwrap_err();
         let message = match error {
             Error::ParseError(string) => string,
             _ => panic!(),
@@ -132,7 +134,7 @@ mod tests {
         assert_eq!(message, "Type attribute on data form is mandatory.");
 
         let elem: Element = "<x xmlns='jabber:x:data' type='coucou'/>".parse().unwrap();
-        let error = data_forms::parse_data_form(&elem).unwrap_err();
+        let error = DataForm::try_from(&elem).unwrap_err();
         let message = match error {
             Error::ParseError(string) => string,
             _ => panic!(),
@@ -143,7 +145,7 @@ mod tests {
     #[test]
     fn test_wrong_child() {
         let elem: Element = "<x xmlns='jabber:x:data' type='cancel'><coucou/></x>".parse().unwrap();
-        let error = data_forms::parse_data_form(&elem).unwrap_err();
+        let error = DataForm::try_from(&elem).unwrap_err();
         let message = match error {
             Error::ParseError(string) => string,
             _ => panic!(),

src/disco.rs

@@ -4,12 +4,14 @@
 // License, v. 2.0. If a copy of the MPL was not distributed with this
 // file, You can obtain one at http://mozilla.org/MPL/2.0/.
 
+use std::convert::TryFrom;
+
 use minidom::Element;
 
 use error::Error;
 use ns;
 
-use data_forms::{DataForm, DataFormType, parse_data_form};
+use data_forms::{DataForm, DataFormType};
 
 #[derive(Debug, Clone, PartialEq)]
 pub struct Feature {
@@ -74,7 +76,7 @@ pub fn parse_disco(root: &Element) -> Result<Disco, Error> {
                 name: name,
             });
         } else if child.is("x", ns::DATA_FORMS) {
-            let data_form = parse_data_form(child)?;
+            let data_form = DataForm::try_from(child)?;
             match data_form.type_ {
                 DataFormType::Result_ => (),
                 _ => return Err(Error::ParseError("Data form must have a 'result' type in disco#info.")),

src/mam.rs

@@ -11,7 +11,6 @@ use jid::Jid;
 
 use error::Error;
 
-use data_forms;
 use data_forms::DataForm;
 use rsm::Set;
 use forwarding;
@@ -62,7 +61,7 @@ pub fn parse_query(root: &Element) -> Result<Query, Error> {
     let mut set = None;
     for child in root.children() {
         if child.is("x", ns::DATA_FORMS) {
-            form = Some(data_forms::parse_data_form(child)?);
+            form = Some(DataForm::try_from(child)?);
         } else if child.is("set", ns::RSM) {
             set = Some(Set::try_from(child)?);
         } else {
@@ -177,7 +176,7 @@ pub fn serialise_query(query: &Query) -> Element {
                            .attr("node", query.node.clone())
                            .build();
     //if let Some(form) = query.form {
-    //    elem.append_child(data_forms::serialise(&form));
+    //    elem.append_child((&form).into());
     //}
     if let Some(ref set) = query.set {
         elem.append_child(set.into());

src/media_element.rs

@@ -55,7 +55,7 @@ impl<'a> TryFrom<&'a Element> for MediaElement {
 #[cfg(test)]
 mod tests {
     use super::*;
-    use data_forms;
+    use data_forms::DataForm;
 
     #[test]
     fn test_simple() {
@@ -177,7 +177,7 @@ mod tests {
   </field>
   [ ... ]
 </x>"#.parse().unwrap();
-        let form = data_forms::parse_data_form(&elem).unwrap();
+        let form = DataForm::try_from(&elem).unwrap();
         assert_eq!(form.fields.len(), 1);
         assert_eq!(form.fields[0].var, "ocr");
         assert_eq!(form.fields[0].media[0].width, Some(290));