From dc2544712c19720cd82f70461f6ad5525dc2db7e Mon Sep 17 00:00:00 2001
From: Nick Cameron <ncameron@mozilla.com>
Date: Fri, 14 Aug 2015 20:00:22 +1200
Subject: [PATCH 1/7] Rewrite match expressions

---
 src/expr.rs           | 174 +++++++++++++++++++++++++++++++++++++++++-
 src/rewrite.rs        |  10 +++
 src/utils.rs          |  20 +++++
 tests/target/match.rs |  42 ++++++++++
 4 files changed, 245 insertions(+), 1 deletion(-)
 create mode 100644 tests/target/match.rs

diff --git a/src/expr.rs b/src/expr.rs
index 2fbdcb7d6c9..ce5dfb8cddf 100644
--- a/src/expr.rs
+++ b/src/expr.rs
@@ -12,7 +12,7 @@ use rewrite::{Rewrite, RewriteContext};
 use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic};
 use string::{StringFormat, rewrite_string};
 use StructLitStyle;
-use utils::{span_after, make_indent, extra_offset};
+use utils::{span_after, make_indent, extra_offset, first_line_width, last_line_width};
 use visitor::FmtVisitor;
 use config::BlockIndentStyle;
 use comment::{FindUncommented, rewrite_comment};
@@ -99,6 +99,9 @@ impl Rewrite for ast::Expr {
                               width,
                               offset)
             }
+            ast::Expr_::ExprMatch(ref cond, ref arms, _) => {
+                rewrite_match(context, cond, arms, width, offset)
+            }
             ast::Expr_::ExprPath(ref qself, ref path) => {
                 rewrite_path(context, qself.as_ref(), path, width, offset)
             }
@@ -303,6 +306,175 @@ fn rewrite_if_else(context: &RewriteContext,
     Some(result)
 }
 
+fn rewrite_match(context: &RewriteContext,
+                 cond: &ast::Expr,
+                 arms: &[ast::Arm],
+                 width: usize,
+                 offset: usize)
+                 -> Option<String> {
+    // TODO comments etc. (I am somewhat surprised we don't need handling for these).
+
+    // `match `cond` {`
+    let cond_str = try_opt!(cond.rewrite(context, width - 8, offset + 6));
+    let mut result = format!("match {} {{", cond_str);
+
+    let block_indent = context.block_indent;
+    let nested_context = context.nested_context();
+    let arm_indent_str = make_indent(nested_context.block_indent);
+
+    for arm in arms {
+        result.push('\n');
+        result.push_str(&arm_indent_str);
+        result.push_str(&&try_opt!(arm.rewrite(&nested_context,
+                                               context.config.max_width -
+                                                   nested_context.block_indent,
+                                               nested_context.block_indent)));
+    }
+
+    result.push('\n');
+    result.push_str(&make_indent(block_indent));
+    result.push('}');
+    Some(result)
+}
+
+// Match arms.
+impl Rewrite for ast::Arm {
+    fn rewrite(&self, context: &RewriteContext, width: usize, offset: usize) -> Option<String> {
+        let &ast::Arm { ref attrs, ref pats, ref guard, ref body } = self;
+        let indent_str = make_indent(offset);
+
+        // TODO attrs
+
+        // Patterns
+        let pat_strs = pats.iter().map(|p| p.rewrite(context,
+                                                     // 5 = ` => {`
+                                                     width - 5,
+                                                     offset + context.config.tab_spaces)).collect::<Vec<_>>();
+        if pat_strs.iter().any(|p| p.is_none()) {
+            return None;
+        }
+        let pat_strs = pat_strs.into_iter().map(|p| p.unwrap()).collect::<Vec<_>>();
+                                  
+        let mut total_width = pat_strs.iter().fold(0, |a, p| a + p.len());
+        // Add ` | `.len().
+        total_width += (pat_strs.len() - 1) * 3;
+
+        let mut vertical = total_width > width - 5 || pat_strs.iter().any(|p| p.contains('\n'));
+        if !vertical {
+            // If the patterns were previously stacked, keep them stacked.
+            // FIXME should be an option.
+            let pat_span = mk_sp(pats[0].span.lo, pats[pats.len() - 1].span.hi);
+            let pat_str = context.codemap.span_to_snippet(pat_span).unwrap();
+            vertical = pat_str.find('\n').is_some();
+        }
+
+
+        let pats_width = if vertical {
+            pat_strs[pat_strs.len() - 1].len()
+        } else {
+            total_width
+        };
+
+        let mut pats_str = String::new();
+        for p in pat_strs {
+            if pats_str.len() > 0 {
+                if vertical {
+                    pats_str.push_str(" |\n");
+                    pats_str.push_str(&indent_str);
+                } else {
+                    pats_str.push_str(" | ");
+                }
+            }
+            pats_str.push_str(&p);
+        }
+
+        // TODO probably want to compute the guard width first, then the rest
+        // TODO also, only subtract the guard width from the last pattern.
+        // If guard.
+        let guard_str = try_opt!(rewrite_guard(context, guard, width, offset, pats_width));
+
+        let pats_str = format!("{}{}", pats_str, guard_str);
+        // Where the next text can start.
+        let mut line_start = last_line_width(&pats_str);
+        if pats_str.find('\n').is_none() {
+            line_start += offset;
+        }
+
+        let comma = if let ast::ExprBlock(_) = body.node {
+            String::new()
+        } else {
+            ",".to_owned()
+        };
+
+        // Let's try and get the arm body on the same line as the condition.
+        // 4 = ` => `.len()
+        if context.config.max_width > line_start + comma.len() + 4 {
+            let budget = context.config.max_width - line_start - comma.len() - 4;
+            if let Some(ref body_str) = body.rewrite(context,
+                                                     budget,
+                                                     offset + context.config.tab_spaces) {
+                if first_line_width(body_str) <= budget {
+                    return Some(format!("{} => {}{}", pats_str, body_str, comma));
+                }
+            }
+        }
+
+        // We have to push the body to the next line.
+        if comma.len() > 0 {
+            // We're trying to fit a block in, but it still failed, give up.
+            return None;
+        }
+
+        let body_str = try_opt!(body.rewrite(context,
+                                             width - context.config.tab_spaces,
+                                             offset + context.config.tab_spaces));
+        Some(format!("{} =>\n{}{}",
+                     pats_str,
+                     make_indent(offset + context.config.tab_spaces),
+                     body_str))
+    }
+}
+
+// The `if ...` guard on a match arm.
+fn rewrite_guard(context: &RewriteContext,
+                 guard: &Option<ptr::P<ast::Expr>>,
+                 width: usize,
+                 offset: usize,
+                 // The amount of space used up on this line for the pattern in
+                 // the arm.
+                 pattern_width: usize)
+                 -> Option<String> {
+    if let &Some(ref guard) = guard {
+        // 4 = ` if `, 5 = ` => {`
+        let overhead = pattern_width + 4 + 5;
+        if overhead < width {
+            let cond_str = guard.rewrite(context,
+                                         width - overhead,
+                                         offset + context.config.tab_spaces);
+            if let Some(cond_str) = cond_str {
+                return Some(format!(" if {}", cond_str));
+            }
+        }
+
+        // Not enough space to put the guard after the pattern, try a newline.
+        let overhead = context.config.tab_spaces + 4 + 5;
+        if overhead < width {
+            let cond_str = guard.rewrite(context,
+                                         width - overhead,
+                                         offset + context.config.tab_spaces);
+            if let Some(cond_str) = cond_str {
+                return Some(format!("\n{}if {}",
+                                    make_indent(offset + context.config.tab_spaces),
+                                    cond_str));
+            }
+        }
+
+        None
+    } else {
+        Some(String::new())
+    }
+}
+
 fn rewrite_pat_expr(context: &RewriteContext,
                     pat: Option<&ast::Pat>,
                     expr: &ast::Expr,
diff --git a/src/rewrite.rs b/src/rewrite.rs
index 33e4d5fbbf3..d30d81a4885 100644
--- a/src/rewrite.rs
+++ b/src/rewrite.rs
@@ -30,3 +30,13 @@ pub struct RewriteContext<'a> {
     pub config: &'a Config,
     pub block_indent: usize,
 }
+
+impl<'a> RewriteContext<'a> {
+    pub fn nested_context(&self) -> RewriteContext<'a> {
+        RewriteContext {
+            codemap: self.codemap,
+            config: self.config,
+            block_indent: self.block_indent + self.config.tab_spaces,
+        }
+    }
+}
diff --git a/src/utils.rs b/src/utils.rs
index a57ffb008ea..f80e1185bae 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -82,6 +82,25 @@ pub fn format_mutability(mutability: ast::Mutability) -> &'static str {
     }
 }
 
+// The width of the first line in s.
+#[inline]
+pub fn first_line_width(s: &str) -> usize {
+    match s.find('\n') {
+        Some(n) => n,
+        None => s.len(),
+    }
+}
+
+// The width of the last line in s.
+#[inline]
+pub fn last_line_width(s: &str) -> usize {
+    match s.rfind('\n') {
+        Some(n) => s.len() - n,
+        None => s.len(),
+    }
+}
+
+#[inline]
 fn is_skip(meta_item: &MetaItem) -> bool {
     match meta_item.node {
         MetaItem_::MetaWord(ref s) => *s == SKIP_ANNOTATION,
@@ -95,6 +114,7 @@ pub fn contains_skip(attrs: &[Attribute]) -> bool {
 }
 
 // Find the end of a TyParam
+#[inline]
 pub fn end_typaram(typaram: &ast::TyParam) -> BytePos {
     typaram.bounds.last().map(|bound| match *bound {
         ast::RegionTyParamBound(ref lt) => lt.span,
diff --git a/tests/target/match.rs b/tests/target/match.rs
new file mode 100644
index 00000000000..9d8a484028b
--- /dev/null
+++ b/tests/target/match.rs
@@ -0,0 +1,42 @@
+// Match expressions.
+
+fn foo() {
+    // A match expression.
+    match x {
+        // Some comment.
+        a => foo(),
+        b if 0 < 42 => foo(),
+        c => { // Another comment.
+            // Comment.
+            an_expression;
+            foo()
+        }
+        // Perhaps this should introduce braces?
+        Foo(ref bar) =>
+            aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa,
+        Pattern1 | Pattern2 | Pattern3 => false,
+        Paternnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn |
+        Paternnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnnn => {
+            blah
+        }
+        Patternnnnnnnnnnnnnnnnnnn |
+        Patternnnnnnnnnnnnnnnnnnn |
+        Patternnnnnnnnnnnnnnnnnnn |
+        Patternnnnnnnnnnnnnnnnnnn => meh,
+
+        Patternnnnnnnnnnnnnnnnnnn |
+        Patternnnnnnnnnnnnnnnnnnn if looooooooooooooooooong_guard => meh,
+
+        Patternnnnnnnnnnnnnnnnnnnnnnnnn |
+        Patternnnnnnnnnnnnnnnnnnnnnnnnn
+            if looooooooooooooooooooooooooooooooooooooooong_guard => meh,
+        _ => {}
+    }
+
+    let whatever = match something {
+        /// DOC COMMENT!
+        Some(_) => 42,
+        #[an_attribute]
+        None => 0,
+    };
+}

From d10629d8a5149e224126d2e05943122763f2f4cc Mon Sep 17 00:00:00 2001
From: Nick Cameron <ncameron@mozilla.com>
Date: Sun, 16 Aug 2015 16:13:55 +1200
Subject: [PATCH 2/7] Allow `{}` to remain.

---
 src/expr.rs          |  5 +++++
 tests/source/expr.rs | 13 +++++++++++++
 tests/target/expr.rs | 26 ++++++++++++++++++--------
 tests/target/loop.rs |  3 +--
 4 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/src/expr.rs b/src/expr.rs
index ce5dfb8cddf..3fb8f85295c 100644
--- a/src/expr.rs
+++ b/src/expr.rs
@@ -112,6 +112,11 @@ impl Rewrite for ast::Expr {
 
 impl Rewrite for ast::Block {
     fn rewrite(&self, context: &RewriteContext, width: usize, offset: usize) -> Option<String> {
+        let user_str = context.codemap.span_to_snippet(self.span).unwrap();
+        if user_str == "{}" && width > 1 {
+            return Some(user_str);
+        }
+
         let mut visitor = FmtVisitor::from_codemap(context.codemap, context.config);
         visitor.block_indent = context.block_indent;
 
diff --git a/tests/source/expr.rs b/tests/source/expr.rs
index e7862b83e50..6b3b3daf0a9 100644
--- a/tests/source/expr.rs
+++ b/tests/source/expr.rs
@@ -91,3 +91,16 @@ fn baz() {
         // Regular unsafe block
     }
 }
+
+// Test some empty blocks.
+fn qux() {
+    {}
+    // FIXME this one could be done better.
+    { /* a block with a comment */ }
+    {
+
+    }
+    {
+        // A block with a comment.
+    }
+}
diff --git a/tests/target/expr.rs b/tests/target/expr.rs
index a1875a0ec26..356c5b7dbbd 100644
--- a/tests/target/expr.rs
+++ b/tests/target/expr.rs
@@ -22,15 +22,13 @@ fn foo() -> bool {
              aaaaa)))))))));
 
     {
-        for _ in 0..10 {
-        }
+        for _ in 0..10 {}
     }
 
     {
         {
             {
-                {
-                }
+                {}
             }
         }
     }
@@ -47,8 +45,7 @@ fn foo() -> bool {
     }
 
     if let Some(x) = (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +
-                      aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {
-    }
+                      aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {}
 
     if let (some_very_large,
             tuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuple) = 1 + 2 + 3 {
@@ -56,8 +53,7 @@ fn foo() -> bool {
 
     if let (some_very_large,
             tuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuple) = 1111 +
-                                                                                         2222 {
-    }
+                                                                                         2222 {}
 
     if let (some_very_large, tuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuuple) =
            1 + 2 + 3 {
@@ -121,3 +117,17 @@ fn baz() {
         // Regular unsafe block
     }
 }
+
+// Test some empty blocks.
+fn qux() {
+    {}
+    // FIXME this one could be done better.
+    { /* a block with a comment */
+    }
+    {
+
+    }
+    {
+        // A block with a comment.
+    }
+}
diff --git a/tests/target/loop.rs b/tests/target/loop.rs
index e1f2ccc91a4..fea5bfe2cb2 100644
--- a/tests/target/loop.rs
+++ b/tests/target/loop.rs
@@ -13,8 +13,7 @@ fn main() {
     }
 
     'a: while loooooooooooooooooooooooooooooooooong_variable_name + another_value >
-              some_other_value {
-    }
+              some_other_value {}
 
     while aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa > bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb {
     }

From a43e2b5ae837604511557133f5e7554184465677 Mon Sep 17 00:00:00 2001
From: Nick Cameron <ncameron@mozilla.com>
Date: Sun, 16 Aug 2015 15:58:17 +1200
Subject: [PATCH 3/7] Formatting

---
 src/comment.rs |  6 +++---
 src/expr.rs    | 26 ++++++++++++-------------
 src/filemap.rs | 20 +++++++++----------
 src/imports.rs | 17 +++++++++--------
 src/issues.rs  | 10 +++++-----
 src/items.rs   | 21 +++++++++-----------
 src/lib.rs     |  4 ++--
 src/lists.rs   |  7 ++++---
 src/modules.rs |  2 +-
 src/types.rs   | 52 +++++++++++++++++++++++++-------------------------
 src/utils.rs   |  6 +++---
 src/visitor.rs |  2 +-
 12 files changed, 86 insertions(+), 87 deletions(-)

diff --git a/src/comment.rs b/src/comment.rs
index 46c698ecbbb..14ca718e318 100644
--- a/src/comment.rs
+++ b/src/comment.rs
@@ -127,8 +127,8 @@ impl FindUncommented for str {
                     if b != c {
                         needle_iter = pat.chars();
                     }
-                },
-                None => return Some(i - pat.len())
+                }
+                None => return Some(i - pat.len()),
             }
 
             if possible_comment && (b == '/' || b == '*') {
@@ -145,7 +145,7 @@ impl FindUncommented for str {
         // Handle case where the pattern is a suffix of the search string
         match needle_iter.next() {
             Some(_) => None,
-            None => Some(self.len() - pat.len())
+            None => Some(self.len() - pat.len()),
         }
     }
 }
diff --git a/src/expr.rs b/src/expr.rs
index 3fb8f85295c..db646dafaba 100644
--- a/src/expr.rs
+++ b/src/expr.rs
@@ -30,7 +30,7 @@ impl Rewrite for ast::Expr {
                     ast::Lit_::LitStr(ref is, _) => {
                         rewrite_string_lit(context, &is, l.span, width, offset)
                     }
-                    _ => context.codemap.span_to_snippet(self.span).ok()
+                    _ => context.codemap.span_to_snippet(self.span).ok(),
                 }
             }
             ast::Expr_::ExprCall(ref callee, ref args) => {
@@ -105,7 +105,7 @@ impl Rewrite for ast::Expr {
             ast::Expr_::ExprPath(ref qself, ref path) => {
                 rewrite_path(context, qself.as_ref(), path, width, offset)
             }
-            _ => context.codemap.span_to_snippet(self.span).ok()
+            _ => context.codemap.span_to_snippet(self.span).ok(),
         }
     }
 }
@@ -199,7 +199,7 @@ impl<'a> Loop<'a> {
             keyword: "while ",
             matcher: match pat {
                 Some(..) => "let ",
-                None => ""
+                None => "",
             },
             connector: " =",
         }
@@ -237,7 +237,7 @@ impl<'a> Rewrite for Loop<'a> {
                                                     self.connector,
                                                     inner_width,
                                                     inner_offset)),
-            None => String::new()
+            None => String::new(),
         };
 
         // FIXME: this drops any comment between "loop" and the block.
@@ -250,7 +250,7 @@ impl<'a> Rewrite for Loop<'a> {
 fn rewrite_label(label: Option<ast::Ident>) -> String {
     match label {
         Some(ident) => format!("{}: ", ident),
-        None => "".to_owned()
+        None => "".to_owned(),
     }
 }
 
@@ -262,9 +262,8 @@ fn rewrite_range(context: &RewriteContext,
                  offset: usize)
                  -> Option<String> {
     let left_string = match left {
-        // 2 = ..
         Some(expr) => try_opt!(expr.rewrite(context, width - 2, offset)),
-        None => String::new()
+        None => String::new(),
     };
 
     let right_string = match right {
@@ -273,7 +272,7 @@ fn rewrite_range(context: &RewriteContext,
             let max_width = (width - 2).checked_sub(left_string.len()).unwrap_or(0);
             try_opt!(expr.rewrite(context, max_width, offset + 2 + left_string.len()))
         }
-        None => String::new()
+        None => String::new(),
     };
 
     Some(format!("{}..{}", left_string, right_string))
@@ -354,12 +353,13 @@ impl Rewrite for ast::Arm {
         let pat_strs = pats.iter().map(|p| p.rewrite(context,
                                                      // 5 = ` => {`
                                                      width - 5,
-                                                     offset + context.config.tab_spaces)).collect::<Vec<_>>();
+                                                     offset + context.config.tab_spaces))
+                           .collect::<Vec<_>>();
         if pat_strs.iter().any(|p| p.is_none()) {
             return None;
         }
         let pat_strs = pat_strs.into_iter().map(|p| p.unwrap()).collect::<Vec<_>>();
-                                  
+
         let mut total_width = pat_strs.iter().fold(0, |a, p| a + p.len());
         // Add ` | `.len().
         total_width += (pat_strs.len() - 1) * 3;
@@ -496,7 +496,7 @@ fn rewrite_pat_expr(context: &RewriteContext,
                                                   pat_offset));
             format!("{}{}{}", matcher, pat_string, connector)
         }
-        None => String::new()
+        None => String::new(),
     };
 
     // Consider only the last line of the pat string.
@@ -804,7 +804,7 @@ fn rewrite_binary_op(context: &RewriteContext,
     let used_width = result.len() + 1;
     let remaining_width = match result.rfind('\n') {
         Some(idx) => (offset + width + idx).checked_sub(used_width).unwrap_or(0),
-        None => width.checked_sub(used_width).unwrap_or(0)
+        None => width.checked_sub(used_width).unwrap_or(0),
     };
 
     // Get "full width" rhs and see if it fits on the current line. This
@@ -836,7 +836,7 @@ fn rewrite_unary_op(context: &RewriteContext,
         ast::UnOp::UnUniq => "box ",
         ast::UnOp::UnDeref => "*",
         ast::UnOp::UnNot => "!",
-        ast::UnOp::UnNeg => "-"
+        ast::UnOp::UnNeg => "-",
     };
 
     let subexpr = try_opt!(expr.rewrite(context, width - operator_str.len(), offset));
diff --git a/src/filemap.rs b/src/filemap.rs
index ff50522eec6..8bb5a7285bb 100644
--- a/src/filemap.rs
+++ b/src/filemap.rs
@@ -58,18 +58,18 @@ fn write_file(text: &StringBuffer,
         where T: Write
     {
         match config.newline_style {
-                NewlineStyle::Unix => write!(writer, "{}", text),
-                NewlineStyle::Windows => {
-                    for (c, _) in text.chars() {
-                        match c {
-                            '\n' => try!(write!(writer, "\r\n")),
-                            '\r' => continue,
-                            c => try!(write!(writer, "{}", c)),
-                        }
+            NewlineStyle::Unix => write!(writer, "{}", text),
+            NewlineStyle::Windows => {
+                for (c, _) in text.chars() {
+                    match c {
+                        '\n' => try!(write!(writer, "\r\n")),
+                        '\r' => continue,
+                        c => try!(write!(writer, "{}", c)),
                     }
-                    Ok(())
-                },
+                }
+                Ok(())
             }
+        }
     }
 
     match mode {
diff --git a/src/imports.rs b/src/imports.rs
index 392e6fb838a..7d7300fb562 100644
--- a/src/imports.rs
+++ b/src/imports.rs
@@ -15,7 +15,8 @@ use rewrite::{Rewrite, RewriteContext};
 use syntax::ast;
 use syntax::codemap::Span;
 
-// TODO (some day) remove unused imports, expand globs, compress many single imports into a list import
+// TODO (some day) remove unused imports, expand globs, compress many single
+// imports into a list import.
 
 impl Rewrite for ast::ViewPath {
     // Returns an empty string when the ViewPath is empty (like foo::bar::{})
@@ -39,10 +40,10 @@ impl Rewrite for ast::ViewPath {
                 let path_str = try_opt!(path.rewrite(context, width - ident_str.len() - 4, offset));
 
                 Some(if path.segments.last().unwrap().identifier == ident {
-                         path_str
-                     } else {
-                         format!("{} as {}", path_str, ident_str)
-                     })
+                        path_str
+                    } else {
+                        format!("{} as {}", path_str, ident_str)
+                    })
             }
         }
     }
@@ -81,7 +82,7 @@ pub fn rewrite_use_list(width: usize,
     match path_list.len() {
         0 => return None,
         1 => return Some(rewrite_single_use_list(path_str, path_list[0])),
-        _ => ()
+        _ => (),
     }
 
     // 2 = ::
@@ -161,7 +162,7 @@ fn move_self_to_front(items: &mut Vec<ListItem>) -> bool {
         Some(pos) => {
             items[0] = items.remove(pos);
             true
-        },
-        None => false
+        }
+        None => false,
     }
 }
diff --git a/src/issues.rs b/src/issues.rs
index 34d5b8ab2c4..e9f8255cc91 100644
--- a/src/issues.rs
+++ b/src/issues.rs
@@ -29,7 +29,7 @@ impl ReportTactic {
         match *self {
             ReportTactic::Always => true,
             ReportTactic::Unnumbered => true,
-            ReportTactic::Never => false
+            ReportTactic::Never => false,
         }
     }
 }
@@ -113,7 +113,7 @@ impl BadIssueSeeker {
         match self.state {
             Seeking::Issue { todo_idx, fixme_idx } => {
                 self.state = self.inspect_issue(c, todo_idx, fixme_idx);
-            },
+            }
             Seeking::Number { issue, part } => {
                 let result = self.inspect_number(c, issue, part);
 
@@ -198,19 +198,19 @@ impl BadIssueSeeker {
                 } else {
                     part = NumberPart::Pound;
                 }
-            },
+            }
             NumberPart::Pound => {
                 if c == '#' {
                     part = NumberPart::Number;
                 }
-            },
+            }
             NumberPart::Number => {
                 if c >= '0' && c <= '9' {
                     part = NumberPart::CloseParen;
                 } else {
                     return IssueClassification::Bad(issue);
                 }
-            },
+            }
             NumberPart::CloseParen => {}
         }
 
diff --git a/src/items.rs b/src/items.rs
index 458a7b0b336..79d374f85ec 100644
--- a/src/items.rs
+++ b/src/items.rs
@@ -411,10 +411,8 @@ impl<'a> FmtVisitor<'a> {
 
                     result.push('(');
 
-                    let indent = self.block_indent
-                                 + vis.len()
-                                 + field.node.name.to_string().len()
-                                 + 1; // Open paren
+                    let indent = self.block_indent + vis.len() + field.node.name.to_string().len() +
+                                 1; // Open paren
 
                     let comma_cost = if self.config.enum_trailing_comma {
                         1
@@ -449,7 +447,7 @@ impl<'a> FmtVisitor<'a> {
                 }
 
                 result
-            },
+            }
             ast::VariantKind::StructVariantKind(ref struct_def) => {
                 // TODO Should limit the width, as we have a trailing comma
                 self.format_struct("",
@@ -491,7 +489,7 @@ impl<'a> FmtVisitor<'a> {
 
         let is_tuple = match struct_def.fields[0].node.kind {
             ast::StructFieldKind::NamedField(..) => false,
-            ast::StructFieldKind::UnnamedField(..) => true
+            ast::StructFieldKind::UnnamedField(..) => true,
         };
 
         let (opener, terminator) = if is_tuple {
@@ -506,7 +504,7 @@ impl<'a> FmtVisitor<'a> {
                                             offset + header_str.len(),
                                             codemap::mk_sp(span.lo,
                                                            struct_def.fields[0].span.lo)),
-            None => opener.to_owned()
+            None => opener.to_owned(),
         };
         result.push_str(&generics_str);
 
@@ -632,7 +630,7 @@ impl<'a> FmtVisitor<'a> {
         };
         let vis = match field.node.kind {
             ast::StructFieldKind::NamedField(_, vis) |
-            ast::StructFieldKind::UnnamedField(vis) => format_visibility(vis)
+            ast::StructFieldKind::UnnamedField(vis) => format_visibility(vis),
         };
         let typ = pprust::ty_to_string(&field.node.ty);
 
@@ -645,7 +643,7 @@ impl<'a> FmtVisitor<'a> {
 
         match name {
             Some(name) => format!("{}{}{}: {}", attr_str, vis, name, typ),
-            None => format!("{}{}{}", attr_str, vis, typ)
+            None => format!("{}{}{}", attr_str, vis, typ),
         }
     }
 
@@ -799,8 +797,7 @@ fn rewrite_explicit_self(explicit_self: &ast::ExplicitSelf, args: &[ast::Arg]) -
 
             // this hacky solution caused by absence of `Mutability` in `SelfValue`.
             let mut_str = {
-                if let ast::Pat_::PatIdent(ast::BindingMode::BindByValue(mutability), _, _)
-                        = args[0].pat.node {
+                if let ast::Pat_::PatIdent(ast::BindingMode::BindByValue(mutability), _, _) = args[0].pat.node {
                     format_mutability(mutability)
                 } else {
                     panic!("there is a bug or change in structure of AST, aborting.");
@@ -809,7 +806,7 @@ fn rewrite_explicit_self(explicit_self: &ast::ExplicitSelf, args: &[ast::Arg]) -
 
             Some(format!("{}self", mut_str))
         }
-        _ => None
+        _ => None,
     }
 }
 
diff --git a/src/lib.rs b/src/lib.rs
index cd43be56a99..9ec401b129d 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -92,7 +92,7 @@ impl FromStr for WriteMode {
             "replace" => Ok(WriteMode::Replace),
             "display" => Ok(WriteMode::Display),
             "overwrite" => Ok(WriteMode::Overwrite),
-            _ => Err(())
+            _ => Err(()),
         }
     }
 }
@@ -243,7 +243,7 @@ fn fmt_lines(file_map: &mut FileMap, config: &Config) -> FormatReport {
 
         for (c, b) in text.chars() {
             if c == '\r' {
-                continue;
+                continuecontinue
             }
 
             // Add warnings for bad todos/ fixmes
diff --git a/src/lists.rs b/src/lists.rs
index 8f1d8a743c2..eadefe2e0fb 100644
--- a/src/lists.rs
+++ b/src/lists.rs
@@ -213,7 +213,8 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> St
             let comment = item.post_comment.as_ref().unwrap();
             // Use block-style only for the last item or multiline comments.
             let block_style = !formatting.ends_with_newline && last ||
-                              comment.trim().contains('\n') || comment.trim().len() > width;
+                              comment.trim().contains('\n') ||
+                              comment.trim().len() > width;
 
             let formatted_comment = rewrite_comment(comment, block_style, width, offset);
 
@@ -381,7 +382,7 @@ fn comment_len(comment: &Option<String>) -> usize {
             } else {
                 text_len
             }
-        },
-        &None => 0
+        }
+        &None => 0,
     }
 }
diff --git a/src/modules.rs b/src/modules.rs
index 2f40fd33a55..681bb1c8ade 100644
--- a/src/modules.rs
+++ b/src/modules.rs
@@ -67,6 +67,6 @@ fn module_file(id: ast::Ident,
 
     match parser::Parser::default_submod_path(id, &dir_path, codemap).result {
         Ok(parser::ModulePathSuccess { path, .. }) => path,
-        Err(_) => panic!("Couldn't find module {}", id)
+        Err(_) => panic!("Couldn't find module {}", id),
     }
 }
diff --git a/src/types.rs b/src/types.rs
index ea8787bd846..10ab7b246cd 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -157,7 +157,7 @@ fn get_path_separator(codemap: &CodeMap,
         if c == ':' {
             return "::"
         } else if c.is_whitespace() || c == '<' {
-            continue;
+            continuecontinue
         } else {
             return "";
         }
@@ -235,7 +235,7 @@ fn rewrite_segment(segment: &ast::PathSegment,
         ast::PathParameters::ParenthesizedParameters(ref data) => {
             let output = match data.output {
                 Some(ref ty) => format!(" -> {}", pprust::ty_to_string(&*ty)),
-                None => String::new()
+                None => String::new(),
             };
 
             let list_lo = span_after(codemap::mk_sp(*span_lo, span_hi), "(", context.codemap);
@@ -267,7 +267,7 @@ fn rewrite_segment(segment: &ast::PathSegment,
 
             format!("({}){}", write_list(&items.collect::<Vec<_>>(), &fmt), output)
         }
-        _ => String::new()
+        _ => String::new(),
     };
 
     Some(format!("{}{}", segment.identifier, params))
@@ -278,57 +278,57 @@ impl Rewrite for ast::WherePredicate {
         // TODO dead spans?
         // TODO assumes we'll always fit on one line...
         Some(match self {
-            &ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate{ref bound_lifetimes,
+                &ast::WherePredicate::BoundPredicate(ast::WhereBoundPredicate{ref bound_lifetimes,
                                                                           ref bounded_ty,
                                                                           ref bounds,
                                                                           ..}) => {
-                if bound_lifetimes.len() > 0 {
-                    let lifetime_str = bound_lifetimes.iter().map(|lt| {
+                    if bound_lifetimes.len() > 0 {
+                        let lifetime_str = bound_lifetimes.iter().map(|lt| {
                                            lt.rewrite(context, width, offset).unwrap()
                                        }).collect::<Vec<_>>().join(", ");
-                    let type_str = pprust::ty_to_string(bounded_ty);
+                        let type_str = pprust::ty_to_string(bounded_ty);
                     // 8 = "for<> : ".len()
-                    let used_width = lifetime_str.len() + type_str.len() + 8;
-                    let bounds_str = bounds.iter().map(|ty_bound| {
+                        let used_width = lifetime_str.len() + type_str.len() + 8;
+                        let bounds_str = bounds.iter().map(|ty_bound| {
                                          ty_bound.rewrite(context,
                                                           width - used_width,
                                                           offset + used_width)
                                                  .unwrap()
                                      }).collect::<Vec<_>>().join(" + ");
 
-                    format!("for<{}> {}: {}", lifetime_str, type_str, bounds_str)
-                } else {
-                    let type_str = pprust::ty_to_string(bounded_ty);
+                        format!("for<{}> {}: {}", lifetime_str, type_str, bounds_str)
+                    } else {
+                        let type_str = pprust::ty_to_string(bounded_ty);
                     // 2 = ": ".len()
-                    let used_width = type_str.len() + 2;
-                    let bounds_str = bounds.iter().map(|ty_bound| {
+                        let used_width = type_str.len() + 2;
+                        let bounds_str = bounds.iter().map(|ty_bound| {
                                          ty_bound.rewrite(context,
                                                           width - used_width,
                                                           offset + used_width)
                                                  .unwrap()
                                      }).collect::<Vec<_>>().join(" + ");
 
-                    format!("{}: {}", type_str, bounds_str)
+                        format!("{}: {}", type_str, bounds_str)
+                    }
                 }
-            }
-            &ast::WherePredicate::RegionPredicate(ast::WhereRegionPredicate{ref lifetime,
+                &ast::WherePredicate::RegionPredicate(ast::WhereRegionPredicate{ref lifetime,
                                                                             ref bounds,
                                                                             ..}) => {
-                format!("{}: {}",
+                    format!("{}: {}",
                         pprust::lifetime_to_string(lifetime),
                         bounds.iter().map(pprust::lifetime_to_string)
                               .collect::<Vec<_>>().join(" + "))
-            }
-            &ast::WherePredicate::EqPredicate(ast::WhereEqPredicate{ref path, ref ty, ..}) => {
-                let ty_str = pprust::ty_to_string(ty);
+                }
+                &ast::WherePredicate::EqPredicate(ast::WhereEqPredicate{ref path, ref ty, ..}) => {
+                    let ty_str = pprust::ty_to_string(ty);
                 // 3 = " = ".len()
-                let used_width = 3 + ty_str.len();
-                let path_str = try_opt!(path.rewrite(context,
+                    let used_width = 3 + ty_str.len();
+                    let path_str = try_opt!(path.rewrite(context,
                                                      width - used_width,
                                                      offset + used_width));
-                format!("{} = {}", path_str, ty_str)
-            }
-        })
+                    format!("{} = {}", path_str, ty_str)
+                }
+            })
     }
 }
 
diff --git a/src/utils.rs b/src/utils.rs
index f80e1185bae..a8f7a7c7b1d 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -21,7 +21,7 @@ pub fn extra_offset(text: &str, offset: usize) -> usize {
     match text.rfind('\n') {
         // 1 for newline character
         Some(idx) => text.len() - idx - 1 - offset,
-        None => text.len()
+        None => text.len(),
     }
 }
 
@@ -70,7 +70,7 @@ pub fn make_indent(width: usize) -> String {
 pub fn format_visibility(vis: Visibility) -> &'static str {
     match vis {
         Visibility::Public => "pub ",
-        Visibility::Inherited => ""
+        Visibility::Inherited => "",
     }
 }
 
@@ -78,7 +78,7 @@ pub fn format_visibility(vis: Visibility) -> &'static str {
 pub fn format_mutability(mutability: ast::Mutability) -> &'static str {
     match mutability {
         ast::Mutability::MutMutable => "mut ",
-        ast::Mutability::MutImmutable => ""
+        ast::Mutability::MutImmutable => "",
     }
 }
 
diff --git a/src/visitor.rs b/src/visitor.rs
index e107a32abf8..86b1197822b 100644
--- a/src/visitor.rs
+++ b/src/visitor.rs
@@ -351,7 +351,7 @@ impl<'a> FmtVisitor<'a> {
                 let prev_span = codemap::mk_sp(self.last_pos, span.lo);
                 let span_end = match self.snippet(prev_span).rfind('\n') {
                     Some(offset) => self.last_pos + BytePos(offset as u32),
-                    None => span.lo
+                    None => span.lo,
                 };
                 self.format_missing(span_end);
                 self.last_pos = span.hi;

From ae1eec81af19fae54b7417148f841675d2ccf770 Mon Sep 17 00:00:00 2001
From: Nick Cameron <ncameron@mozilla.com>
Date: Mon, 17 Aug 2015 09:41:45 +1200
Subject: [PATCH 4/7] Rewrite match expressions (continued).

---
 src/expr.rs           | 105 ++++++++++++++++++++++++++++++++++++------
 src/utils.rs          |   2 +-
 tests/target/match.rs |  13 +++++-
 3 files changed, 103 insertions(+), 17 deletions(-)

diff --git a/src/expr.rs b/src/expr.rs
index db646dafaba..face73d72f0 100644
--- a/src/expr.rs
+++ b/src/expr.rs
@@ -113,7 +113,7 @@ impl Rewrite for ast::Expr {
 impl Rewrite for ast::Block {
     fn rewrite(&self, context: &RewriteContext, width: usize, offset: usize) -> Option<String> {
         let user_str = context.codemap.span_to_snippet(self.span).unwrap();
-        if user_str == "{}" && width > 1 {
+        if user_str == "{}" && width >= 2 {
             return Some(user_str);
         }
 
@@ -316,7 +316,9 @@ fn rewrite_match(context: &RewriteContext,
                  width: usize,
                  offset: usize)
                  -> Option<String> {
-    // TODO comments etc. (I am somewhat surprised we don't need handling for these).
+    if arms.len() == 0 {
+        return None;
+    }
 
     // `match `cond` {`
     let cond_str = try_opt!(cond.rewrite(context, width - 8, offset + 6));
@@ -326,28 +328,101 @@ fn rewrite_match(context: &RewriteContext,
     let nested_context = context.nested_context();
     let arm_indent_str = make_indent(nested_context.block_indent);
 
-    for arm in arms {
+    let open_brace_pos = span_after(mk_sp(cond.span.hi, arm_start_pos(&arms[0])),
+                                    "{",
+                                    context.codemap);
+
+    for (i, arm) in arms.iter().enumerate() {
+        // Make sure we get the stuff between arms.
+        let missed_str = if i == 0 {
+            context.codemap.span_to_snippet(mk_sp(open_brace_pos + BytePos(1),
+                                                  arm_start_pos(arm))).unwrap()
+        } else {
+            context.codemap.span_to_snippet(mk_sp(arm_end_pos(&arms[i-1]),
+                                                  arm_start_pos(arm))).unwrap()
+        };
+        let missed_str = match missed_str.find_uncommented(",") {
+            Some(n) => &missed_str[n+1..],
+            None => &missed_str[..],
+        };
+        // first = first non-whitespace byte index.
+        let first = missed_str.find(|c: char| !c.is_whitespace()).unwrap_or(missed_str.len());
+        if missed_str[..first].chars().filter(|c| c == &'\n').count() >= 2 {
+            // There were multiple line breaks which got trimmed to nothing
+            // that means there should be some vertical white space. Lets
+            // replace that with just one blank line.
+            result.push('\n');
+        }
+        let missed_str = missed_str.trim();
+        if missed_str.len() > 0 {
+            result.push('\n');
+            result.push_str(&arm_indent_str);
+            result.push_str(missed_str);
+        }
         result.push('\n');
         result.push_str(&arm_indent_str);
-        result.push_str(&&try_opt!(arm.rewrite(&nested_context,
-                                               context.config.max_width -
-                                                   nested_context.block_indent,
-                                               nested_context.block_indent)));
+
+        let arm_str = arm.rewrite(&nested_context,
+                                  context.config.max_width -
+                                      nested_context.block_indent,
+                                  nested_context.block_indent);
+        if let Some(ref arm_str) = arm_str {
+            result.push_str(arm_str);
+        } else {
+            // We couldn't format the arm, just reproduce the source.
+            let snippet = context.codemap.span_to_snippet(mk_sp(arm_start_pos(arm),
+                                                                arm_end_pos(arm))).unwrap();
+            result.push_str(&snippet);
+        }
     }
 
+    // We'll miss any comments etc. between the last arm and the end of the
+    // match expression, but meh.
+
     result.push('\n');
     result.push_str(&make_indent(block_indent));
     result.push('}');
     Some(result)
 }
 
+fn arm_start_pos(arm: &ast::Arm) -> BytePos {
+    let &ast::Arm { ref attrs, ref pats, .. } = arm;
+    if attrs.len() > 0 {
+        return attrs[0].span.lo
+    }
+
+    pats[0].span.lo
+}
+
+fn arm_end_pos(arm: &ast::Arm) -> BytePos {
+    arm.body.span.hi
+}
+
 // Match arms.
 impl Rewrite for ast::Arm {
     fn rewrite(&self, context: &RewriteContext, width: usize, offset: usize) -> Option<String> {
         let &ast::Arm { ref attrs, ref pats, ref guard, ref body } = self;
         let indent_str = make_indent(offset);
 
-        // TODO attrs
+        // FIXME this is all a bit grotty, would be nice to abstract out the
+        // treatment of attributes.
+        let attr_str = if attrs.len() > 0 {
+            // We only use this visitor for the attributes, should we use it for
+            // more?
+            let mut attr_visitor = FmtVisitor::from_codemap(context.codemap, context.config);
+            attr_visitor.block_indent = context.block_indent;
+            attr_visitor.last_pos = attrs[0].span.lo;
+            if attr_visitor.visit_attrs(attrs) {
+                // Attributes included a skip instruction.
+                let snippet = context.codemap.span_to_snippet(mk_sp(attrs[0].span.lo,
+                                                                    body.span.hi)).unwrap();
+                return Some(snippet);
+            }
+            attr_visitor.format_missing(pats[0].span.lo);
+            attr_visitor.buffer.to_string()
+        } else {
+            String::new()
+        };
 
         // Patterns
         let pat_strs = pats.iter().map(|p| p.rewrite(context,
@@ -393,9 +468,6 @@ impl Rewrite for ast::Arm {
             pats_str.push_str(&p);
         }
 
-        // TODO probably want to compute the guard width first, then the rest
-        // TODO also, only subtract the guard width from the last pattern.
-        // If guard.
         let guard_str = try_opt!(rewrite_guard(context, guard, width, offset, pats_width));
 
         let pats_str = format!("{}{}", pats_str, guard_str);
@@ -419,13 +491,17 @@ impl Rewrite for ast::Arm {
                                                      budget,
                                                      offset + context.config.tab_spaces) {
                 if first_line_width(body_str) <= budget {
-                    return Some(format!("{} => {}{}", pats_str, body_str, comma));
+                    return Some(format!("{}{} => {}{}",
+                                        attr_str.trim_left(),
+                                        pats_str,
+                                        body_str,
+                                        comma));
                 }
             }
         }
 
         // We have to push the body to the next line.
-        if comma.len() > 0 {
+        if comma.len() == 0 {
             // We're trying to fit a block in, but it still failed, give up.
             return None;
         }
@@ -433,7 +509,8 @@ impl Rewrite for ast::Arm {
         let body_str = try_opt!(body.rewrite(context,
                                              width - context.config.tab_spaces,
                                              offset + context.config.tab_spaces));
-        Some(format!("{} =>\n{}{}",
+        Some(format!("{}{} =>\n{}{},",
+                     attr_str.trim_left(),
                      pats_str,
                      make_indent(offset + context.config.tab_spaces),
                      body_str))
diff --git a/src/utils.rs b/src/utils.rs
index a8f7a7c7b1d..00e18c3a65b 100644
--- a/src/utils.rs
+++ b/src/utils.rs
@@ -95,7 +95,7 @@ pub fn first_line_width(s: &str) -> usize {
 #[inline]
 pub fn last_line_width(s: &str) -> usize {
     match s.rfind('\n') {
-        Some(n) => s.len() - n,
+        Some(n) => s.len() - n - 1,
         None => s.len(),
     }
 }
diff --git a/tests/target/match.rs b/tests/target/match.rs
index 9d8a484028b..04bc7c87127 100644
--- a/tests/target/match.rs
+++ b/tests/target/match.rs
@@ -28,15 +28,24 @@ fn foo() {
         Patternnnnnnnnnnnnnnnnnnn if looooooooooooooooooong_guard => meh,
 
         Patternnnnnnnnnnnnnnnnnnnnnnnnn |
-        Patternnnnnnnnnnnnnnnnnnnnnnnnn
-            if looooooooooooooooooooooooooooooooooooooooong_guard => meh,
+        Patternnnnnnnnnnnnnnnnnnnnnnnnn if looooooooooooooooooooooooooooooooooooooooong_guard =>
+            meh,
+
+        // Test that earlier patterns can take the guard space
+        (aaaa, bbbbb, ccccccc, aaaaa, bbbbbbbb, cccccc, aaaa, bbbbbbbb, cccccc, dddddd) |
+        Patternnnnnnnnnnnnnnnnnnnnnnnnn if loooooooooooooooooooooooooooooooooooooooooong_guard => {}
+
         _ => {}
     }
 
     let whatever = match something {
         /// DOC COMMENT!
         Some(_) => 42,
+        // COmment on an attribute.
         #[an_attribute]
+        // Comment after an attribute.
         None => 0,
+        #[rustfmt_skip]
+        Blurb     =>     {                  }
     };
 }

From 81f2e449d7b31652e08722f85d44dcc1b31694c6 Mon Sep 17 00:00:00 2001
From: Nick Cameron <ncameron@mozilla.com>
Date: Sun, 16 Aug 2015 16:43:59 +1200
Subject: [PATCH 5/7] Handle span error with `continue`

This should be properly addressed by #184, but requires a change to the rustc parser, so this patch just works around the issue.
---
 src/expr.rs          | 10 ++++++++++
 src/lib.rs           |  5 ++++-
 tests/source/loop.rs |  3 +++
 tests/target/loop.rs |  3 +++
 4 files changed, 20 insertions(+), 1 deletion(-)

diff --git a/src/expr.rs b/src/expr.rs
index face73d72f0..2e526895e25 100644
--- a/src/expr.rs
+++ b/src/expr.rs
@@ -105,6 +105,16 @@ impl Rewrite for ast::Expr {
             ast::Expr_::ExprPath(ref qself, ref path) => {
                 rewrite_path(context, qself.as_ref(), path, width, offset)
             }
+            // FIXME #184 Note that this formatting is broken due to a bad span
+            // from the parser.
+            // `continue`
+            ast::Expr_::ExprAgain(ref opt_ident) => {
+                let id_str = match *opt_ident {
+                    Some(ident) => format!(" {}", ident),
+                    None => String::new(),
+                };
+                Some(format!("continue{}", id_str))
+            }
             _ => context.codemap.span_to_snippet(self.span).ok(),
         }
     }
diff --git a/src/lib.rs b/src/lib.rs
index 9ec401b129d..11d9e76bac8 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -11,7 +11,8 @@
 #![feature(rustc_private)]
 #![feature(str_escape)]
 #![feature(str_char)]
-
+#![feature(custom_attribute)]
+#![allow(unused_attributes)]
 
 // TODO we're going to allocate a whole bunch of temp Strings, is it worth
 // keeping some scratch mem for this and running our own StrPool?
@@ -227,6 +228,8 @@ fn fmt_ast(krate: &ast::Crate, codemap: &CodeMap, config: &Config) -> FileMap {
 // Formatting done on a char by char or line by line basis.
 // TODO warn on bad license
 // TODO other stuff for parity with make tidy
+// FIXME skipping due to `continue`, #184.
+#[rustfmt_skip]
 fn fmt_lines(file_map: &mut FileMap, config: &Config) -> FormatReport {
     let mut truncate_todo = Vec::new();
     let mut report = FormatReport { file_error_map: HashMap::new() };
diff --git a/tests/source/loop.rs b/tests/source/loop.rs
index 36e6ab43de8..c7f7da71831 100644
--- a/tests/source/loop.rs
+++ b/tests/source/loop.rs
@@ -21,5 +21,8 @@ let x = loop { do_forever(); };
     while let  Some(i) =     x.find('s')
     {
         x.update();
+        // FIXME #184
+        // continue;
+        // continue 'foo;
     }
 }
diff --git a/tests/target/loop.rs b/tests/target/loop.rs
index fea5bfe2cb2..648fe826e8f 100644
--- a/tests/target/loop.rs
+++ b/tests/target/loop.rs
@@ -25,5 +25,8 @@ fn main() {
 
     while let Some(i) = x.find('s') {
         x.update();
+        // FIXME #184
+        // continue;
+        // continue 'foo;
     }
 }

From df0fd0e1190c5d79bbc9c493ce739c8f8416d688 Mon Sep 17 00:00:00 2001
From: Nick Cameron <ncameron@mozilla.com>
Date: Mon, 17 Aug 2015 09:55:26 +1200
Subject: [PATCH 6/7] reformatting/rebasing

---
 src/types.rs | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/types.rs b/src/types.rs
index 10ab7b246cd..834548a20fc 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -146,6 +146,8 @@ impl<'a> fmt::Display for SegmentParam<'a> {
 // We'd really rather not do this, but there doesn't seem to be an alternative
 // at this point.
 // FIXME: fails with spans containing comments with the characters < or :
+// FIXME #184 skip due to continue.
+#[rustfmt_skip]
 fn get_path_separator(codemap: &CodeMap,
                       path_start: BytePos,
                       segment_start: BytePos)
@@ -155,7 +157,7 @@ fn get_path_separator(codemap: &CodeMap,
 
     for c in snippet.chars().rev() {
         if c == ':' {
-            return "::"
+            return "::";
         } else if c.is_whitespace() || c == '<' {
             continuecontinue
         } else {
@@ -188,9 +190,8 @@ fn rewrite_segment(segment: &ast::PathSegment,
     let offset = offset + ident_len;
 
     let params = match segment.parameters {
-        ast::PathParameters::AngleBracketedParameters(ref data) if data.lifetimes.len() > 0 ||
-                                                                   data.types.len() > 0 ||
-                                                                   data.bindings.len() > 0 => {
+        ast::PathParameters::AngleBracketedParameters(ref data) if data.lifetimes.len() > 0 || data.types.len() > 0 ||
+            data.bindings.len() > 0 => {
             let param_list = data.lifetimes.iter()
                                            .map(SegmentParam::LifeTime)
                                            .chain(data.types.iter()

From 43ad7ad7a0f6139c25606435828d835c26091fdf Mon Sep 17 00:00:00 2001
From: Nick Cameron <ncameron@mozilla.com>
Date: Wed, 19 Aug 2015 15:15:54 +1200
Subject: [PATCH 7/7] Re-jig binop formatting

and misc other fixes from the reviews.
---
 src/expr.rs           | 88 +++++++++++++++++++++++++------------------
 src/lib.rs            |  2 +-
 src/types.rs          |  7 ++--
 tests/target/expr.rs  | 10 +++--
 tests/target/match.rs |  6 ++-
 5 files changed, 67 insertions(+), 46 deletions(-)

diff --git a/src/expr.rs b/src/expr.rs
index 2e526895e25..0cdaf060714 100644
--- a/src/expr.rs
+++ b/src/expr.rs
@@ -165,7 +165,7 @@ impl Rewrite for ast::Block {
     }
 }
 
-// TODO(#18): implement pattern formatting
+// FIXME(#18): implement pattern formatting
 impl Rewrite for ast::Pat {
     fn rewrite(&self, context: &RewriteContext, _: usize, _: usize) -> Option<String> {
         context.codemap.span_to_snippet(self.span).ok()
@@ -435,15 +435,11 @@ impl Rewrite for ast::Arm {
         };
 
         // Patterns
-        let pat_strs = pats.iter().map(|p| p.rewrite(context,
+        let pat_strs = try_opt!(pats.iter().map(|p| p.rewrite(context,
                                                      // 5 = ` => {`
                                                      width - 5,
                                                      offset + context.config.tab_spaces))
-                           .collect::<Vec<_>>();
-        if pat_strs.iter().any(|p| p.is_none()) {
-            return None;
-        }
-        let pat_strs = pat_strs.into_iter().map(|p| p.unwrap()).collect::<Vec<_>>();
+                                           .collect::<Option<Vec<_>>>());
 
         let mut total_width = pat_strs.iter().fold(0, |a, p| a + p.len());
         // Add ` | `.len().
@@ -488,10 +484,11 @@ impl Rewrite for ast::Arm {
         }
 
         let comma = if let ast::ExprBlock(_) = body.node {
-            String::new()
+            ""
         } else {
-            ",".to_owned()
+            ","
         };
+        let nested_indent = context.block_indent + context.config.tab_spaces;
 
         // Let's try and get the arm body on the same line as the condition.
         // 4 = ` => `.len()
@@ -499,7 +496,7 @@ impl Rewrite for ast::Arm {
             let budget = context.config.max_width - line_start - comma.len() - 4;
             if let Some(ref body_str) = body.rewrite(context,
                                                      budget,
-                                                     offset + context.config.tab_spaces) {
+                                                     nested_indent) {
                 if first_line_width(body_str) <= budget {
                     return Some(format!("{}{} => {}{}",
                                         attr_str.trim_left(),
@@ -518,7 +515,7 @@ impl Rewrite for ast::Arm {
 
         let body_str = try_opt!(body.rewrite(context,
                                              width - context.config.tab_spaces,
-                                             offset + context.config.tab_spaces));
+                                             nested_indent));
         Some(format!("{}{} =>\n{}{},",
                      attr_str.trim_left(),
                      pats_str,
@@ -533,16 +530,17 @@ fn rewrite_guard(context: &RewriteContext,
                  width: usize,
                  offset: usize,
                  // The amount of space used up on this line for the pattern in
-                 // the arm.
+                 // the arm (excludes offset).
                  pattern_width: usize)
                  -> Option<String> {
     if let &Some(ref guard) = guard {
+        // First try to fit the guard string on the same line as the pattern.
         // 4 = ` if `, 5 = ` => {`
         let overhead = pattern_width + 4 + 5;
         if overhead < width {
             let cond_str = guard.rewrite(context,
                                          width - overhead,
-                                         offset + context.config.tab_spaces);
+                                         offset + pattern_width + 4);
             if let Some(cond_str) = cond_str {
                 return Some(format!(" if {}", cond_str));
             }
@@ -653,7 +651,7 @@ fn rewrite_call(context: &RewriteContext,
                 -> Option<String> {
     debug!("rewrite_call, width: {}, offset: {}", width, offset);
 
-    // TODO using byte lens instead of char lens (and probably all over the place too)
+    // FIXME using byte lens instead of char lens (and probably all over the place too)
     // 2 is for parens
     let max_callee_width = try_opt!(width.checked_sub(2));
     let callee_str = try_opt!(callee.rewrite(context, max_callee_width, offset));
@@ -710,7 +708,7 @@ fn rewrite_paren(context: &RewriteContext,
                  -> Option<String> {
     debug!("rewrite_paren, width: {}, offset: {}", width, offset);
     // 1 is for opening paren, 2 is for opening+closing, we want to keep the closing
-    // paren on the same line as the subexpr
+    // paren on the same line as the subexpr.
     let subexpr_str = subexpr.rewrite(context, width-2, offset+1);
     debug!("rewrite_paren, subexpr_str: `{:?}`", subexpr_str);
     subexpr_str.map(|s| format!("({})", s))
@@ -880,20 +878,6 @@ fn rewrite_binary_op(context: &RewriteContext,
 
     let operator_str = context.codemap.span_to_snippet(op.span).unwrap();
 
-    // 1 = space between lhs expr and operator
-    let max_width = try_opt!(context.config.max_width.checked_sub(operator_str.len() + offset + 1));
-    let mut result = try_opt!(lhs.rewrite(context, max_width, offset));
-
-    result.push(' ');
-    result.push_str(&operator_str);
-
-    // 1 = space between operator and rhs
-    let used_width = result.len() + 1;
-    let remaining_width = match result.rfind('\n') {
-        Some(idx) => (offset + width + idx).checked_sub(used_width).unwrap_or(0),
-        None => width.checked_sub(used_width).unwrap_or(0),
-    };
-
     // Get "full width" rhs and see if it fits on the current line. This
     // usually works fairly well since it tends to place operands of
     // operations with high precendence close together.
@@ -901,15 +885,45 @@ fn rewrite_binary_op(context: &RewriteContext,
 
     // Second condition is needed in case of line break not caused by a
     // shortage of space, but by end-of-line comments, for example.
-    if rhs_result.len() > remaining_width || rhs_result.contains('\n') {
-        result.push('\n');
-        result.push_str(&make_indent(offset));
-    } else {
-        result.push(' ');
-    };
+    // Note that this is non-conservative, but its just to see if it's even
+    // worth trying to put everything on one line.
+    if rhs_result.len() + 2 + operator_str.len() < width && !rhs_result.contains('\n') {
+        // 1 = space between lhs expr and operator
+        if let Some(mut result) = lhs.rewrite(context,
+                                              width - 1 - operator_str.len(),
+                                              offset) {
 
-    result.push_str(&rhs_result);
-    Some(result)
+            result.push(' ');
+            result.push_str(&operator_str);
+            result.push(' ');
+
+            let remaining_width = width.checked_sub(last_line_width(&result)).unwrap_or(0);
+
+            if rhs_result.len() <= remaining_width {
+                result.push_str(&rhs_result);
+                return Some(result);
+            }
+
+            if let Some(rhs_result) = rhs.rewrite(context,
+                                                  remaining_width,
+                                                  offset + result.len()) {
+                if rhs_result.len() <= remaining_width {
+                    result.push_str(&rhs_result);
+                    return Some(result);
+                }
+            }
+        }
+    }
+
+    // We have to use multiple lines.
+
+    // Re-evaluate the lhs because we have more space now:
+    let budget = try_opt!(context.config.max_width.checked_sub(offset + 1 + operator_str.len()));
+    Some(format!("{} {}\n{}{}",
+                 try_opt!(lhs.rewrite(context, budget, offset)),
+                 operator_str,
+                 make_indent(offset),
+                 rhs_result))
 }
 
 fn rewrite_unary_op(context: &RewriteContext,
diff --git a/src/lib.rs b/src/lib.rs
index 11d9e76bac8..4efd919649a 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -246,7 +246,7 @@ fn fmt_lines(file_map: &mut FileMap, config: &Config) -> FormatReport {
 
         for (c, b) in text.chars() {
             if c == '\r' {
-                continuecontinue
+                continue;
             }
 
             // Add warnings for bad todos/ fixmes
diff --git a/src/types.rs b/src/types.rs
index 834548a20fc..3bdb08cc0d2 100644
--- a/src/types.rs
+++ b/src/types.rs
@@ -159,7 +159,7 @@ fn get_path_separator(codemap: &CodeMap,
         if c == ':' {
             return "::";
         } else if c.is_whitespace() || c == '<' {
-            continuecontinue
+            continue;
         } else {
             return "";
         }
@@ -190,8 +190,9 @@ fn rewrite_segment(segment: &ast::PathSegment,
     let offset = offset + ident_len;
 
     let params = match segment.parameters {
-        ast::PathParameters::AngleBracketedParameters(ref data) if data.lifetimes.len() > 0 || data.types.len() > 0 ||
-            data.bindings.len() > 0 => {
+        ast::PathParameters::AngleBracketedParameters(ref data) if data.lifetimes.len() > 0 ||
+                                                                   data.types.len() > 0 ||
+                                                                   data.bindings.len() > 0 => {
             let param_list = data.lifetimes.iter()
                                            .map(SegmentParam::LifeTime)
                                            .chain(data.types.iter()
diff --git a/tests/target/expr.rs b/tests/target/expr.rs
index 356c5b7dbbd..dfbf0032011 100644
--- a/tests/target/expr.rs
+++ b/tests/target/expr.rs
@@ -5,8 +5,8 @@ fn foo() -> bool {
     let referenced = &5;
 
     let very_long_variable_name = (a + first + simple + test);
-    let very_long_variable_name = (a + first + simple + test + AAAAAAAAAAAAA + BBBBBBBBBBBBBBBBB +
-                                   b + c);
+    let very_long_variable_name = (a + first + simple + test + AAAAAAAAAAAAA +
+                                   BBBBBBBBBBBBBBBBB + b + c);
 
     //FIXME this exceeds width limit. Needs assignments reformatting
     let is_internalxxxx = self.codemap.span_to_filename(s) == self.codemap.span_to_filename(m.inner);
@@ -15,10 +15,12 @@ fn foo() -> bool {
                    (bbbbbb - function_call(x, *very_long_pointer, y)) + 1000;
 
     some_ridiculously_loooooooooooooooooooooong_function(10000 * 30000000000 +
-                                                         40000 / 1002200000000 - 50000 * sqrt(-1),
+                                                         40000 / 1002200000000 -
+                                                         50000 * sqrt(-1),
                                                          trivial_value);
     (((((((((aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +
-             a + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +
+             a +
+             aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa +
              aaaaa)))))))));
 
     {
diff --git a/tests/target/match.rs b/tests/target/match.rs
index 04bc7c87127..e5f1fad967a 100644
--- a/tests/target/match.rs
+++ b/tests/target/match.rs
@@ -36,12 +36,16 @@ fn foo() {
         Patternnnnnnnnnnnnnnnnnnnnnnnnn if loooooooooooooooooooooooooooooooooooooooooong_guard => {}
 
         _ => {}
+        ast::PathParameters::AngleBracketedParameters(ref data) if data.lifetimes.len() > 0 ||
+                                                                   data.types.len() > 0 ||
+                                                                   data.bindings.len() > 0 => {
+        }
     }
 
     let whatever = match something {
         /// DOC COMMENT!
         Some(_) => 42,
-        // COmment on an attribute.
+        // Comment on an attribute.
         #[an_attribute]
         // Comment after an attribute.
         None => 0,