Merge pull request #94 from marcusklaas/fixes

Fix two TODO items

src/expr.rs

@@ -105,14 +105,8 @@ impl<'a> FmtVisitor<'a> {
             let args: Vec<_> = args.iter().map(|e| (self.rewrite_expr(e,
                                                                       remaining_width,
                                                                       offset), String::new())).collect();
-            // TODO move this into write_list
-            let tactics = if args.iter().any(|&(ref s, _)| s.contains('\n')) {
-                ListTactic::Vertical
-            } else {
-                ListTactic::HorizontalVertical
-            };
             let fmt = ListFormatting {
-                tactic: tactics,
+                tactic: ListTactic::HorizontalVertical,
                 separator: ",",
                 trailing_separator: SeparatorTactic::Never,
                 indent: offset,
@@ -161,13 +155,8 @@ impl<'a> FmtVisitor<'a> {
 
         // FIXME comments
         let field_strs: Vec<_> = field_strs.into_iter().map(|s| (s, String::new())).collect();
-        let tactics = if field_strs.iter().any(|&(ref s, _)| s.contains('\n')) {
-            ListTactic::Vertical
-        } else {
-            ListTactic::HorizontalVertical
-        };
         let fmt = ListFormatting {
-            tactic: tactics,
+            tactic: ListTactic::HorizontalVertical,
             separator: ",",
             trailing_separator: if base.is_some() {
                     SeparatorTactic::Never

src/items.rs

@@ -341,8 +341,7 @@ impl<'a> FmtVisitor<'a> {
         // The fix is comments in the AST or a span for the closing paren.
         let snippet = self.snippet(codemap::mk_sp(prev_end, next_span_start));
         let snippet = snippet.trim();
-        let snippet = &snippet[..snippet.find(terminator)
-                                    .unwrap_or(snippet.find(separator).unwrap_or(snippet.len()))];
+        let snippet = &snippet[..snippet.find(terminator).unwrap_or(snippet.len())];
         let snippet = snippet.trim();
         result.push(snippet.to_owned());
 

src/lib.rs

@@ -64,6 +64,23 @@ const SKIP_ANNOTATION: &'static str = "rustfmt_skip";
 
 static mut CONFIG: Option<config::Config> = None;
 
+// Macro for deriving implementations of Decodable for enums
+macro_rules! impl_enum_decodable {
+    ( $e:ident, $( $x:ident ),* ) => {
+        impl Decodable for $e {
+            fn decode<D: Decoder>(d: &mut D) -> Result<Self, D::Error> {
+                let s = try!(d.read_str());
+                match &*s {
+                    $(
+                        stringify!($x) => Ok($e::$x),
+                    )*
+                    _ => Err(d.error("Bad variant")),
+                }
+            }
+        }
+    };
+}
+
 #[derive(Copy, Clone)]
 pub enum WriteMode {
     Overwrite,
@@ -81,16 +98,7 @@ pub enum NewlineStyle {
     Unix, // \n
 }
 
-impl Decodable for NewlineStyle {
-    fn decode<D: Decoder>(d: &mut D) -> Result<Self, D::Error> {
-        let s = try!(d.read_str());
-        match &*s {
-            "Windows" => Ok(NewlineStyle::Windows),
-            "Unix" => Ok(NewlineStyle::Unix),
-            _ => Err(d.error("Bad variant")),
-        }
-    }
-}
+impl_enum_decodable!(NewlineStyle, Windows, Unix);
 
 #[derive(Copy, Clone, Eq, PartialEq, Debug)]
 pub enum BraceStyle {
@@ -101,17 +109,7 @@ pub enum BraceStyle {
     SameLineWhere,
 }
 
-impl Decodable for BraceStyle {
-    fn decode<D: Decoder>(d: &mut D) -> Result<Self, D::Error> {
-        let s = try!(d.read_str());
-        match &*s {
-            "AlwaysNextLine" => Ok(BraceStyle::AlwaysNextLine),
-            "PreferSameLine" => Ok(BraceStyle::PreferSameLine),
-            "SameLineWhere" => Ok(BraceStyle::SameLineWhere),
-            _ => Err(d.error("Bad variant")),
-        }
-    }
-}
+impl_enum_decodable!(BraceStyle, AlwaysNextLine, PreferSameLine, SameLineWhere);
 
 // How to indent a function's return type.
 #[derive(Copy, Clone, Eq, PartialEq, Debug)]
@@ -122,17 +120,7 @@ pub enum ReturnIndent {
     WithWhereClause,
 }
 
-// TODO could use a macro for all these Decodable impls.
-impl Decodable for ReturnIndent {
-    fn decode<D: Decoder>(d: &mut D) -> Result<Self, D::Error> {
-        let s = try!(d.read_str());
-        match &*s {
-            "WithArgs" => Ok(ReturnIndent::WithArgs),
-            "WithWhereClause" => Ok(ReturnIndent::WithWhereClause),
-            _ => Err(d.error("Bad variant")),
-        }
-    }
-}
+impl_enum_decodable!(ReturnIndent, WithArgs, WithWhereClause);
 
 // Formatting which depends on the AST.
 fn fmt_ast<'a>(krate: &ast::Crate, codemap: &'a CodeMap) -> ChangeSet<'a> {

src/lists.rs

@@ -79,7 +79,8 @@ pub fn write_list<'b>(items: &[(String, String)], formatting: &ListFormatting<'b
     if tactic == ListTactic::HorizontalVertical {
         debug!("write_list: total_width: {}, total_sep_len: {}, h_width: {}",
                total_width, total_sep_len, formatting.h_width);
-        tactic = if fits_single {
+        tactic = if fits_single &&
+                    !items.iter().any(|&(ref s, _)| s.contains('\n')) {
             ListTactic::Horizontal
         } else {
             ListTactic::Vertical