Merge pull request #68 from marcusklaas/import-breaks

Keep import lists on a single line when possible

src/imports.rs

@@ -23,10 +23,11 @@ impl<'a> FmtVisitor<'a> {
     // Basically just pretty prints a multi-item import.
     pub fn rewrite_use_list(&mut self,
                             block_indent: usize,
-                            budget: usize, // excluding indentation
+                            one_line_budget: usize, // excluding indentation
+                            multi_line_budget: usize,
                             path: &ast::Path,
                             path_list: &[ast::PathListItem],
-                            visibility: ast::Visibility) -> Option<String> {
+                            visibility: ast::Visibility) -> String {
         let path_str = pprust::path_to_string(&path);
 
         let vis = match visibility {
@@ -41,10 +42,16 @@ impl<'a> FmtVisitor<'a> {
         // 2 = } + ;
         let used_width = indent + 2;
 
-        let remaining_budget = if used_width >= budget {
-            return None;
+        // Break as early as possible when we've blown our budget.
+        let remaining_line_budget = if used_width > one_line_budget {
+            0
         } else {
-            budget - used_width
+            one_line_budget - used_width
+        };
+        let remaining_multi_budget = if used_width > multi_line_budget {
+            0
+        } else {
+            multi_line_budget - used_width
         };
 
         let fmt = ListFormatting {
@@ -52,8 +59,8 @@ impl<'a> FmtVisitor<'a> {
             separator: ",",
             trailing_separator: SeparatorTactic::Never,
             indent: block_indent + indent,
-            h_width: remaining_budget,
-            v_width: remaining_budget,
+            h_width: remaining_line_budget,
+            v_width: remaining_multi_budget,
         };
 
         // TODO handle any comments inbetween items.
@@ -79,10 +86,10 @@ impl<'a> FmtVisitor<'a> {
                 ast::PathListItem_::PathListMod{ .. } => None,
             }
         })).collect();
-        Some(if path_str.len() == 0 {
+        if path_str.len() == 0 {
             format!("{}use {{{}}};", vis, write_list(&items, &fmt))
         } else {
             format!("{}use {}::{{{}}};", vis, path_str, write_list(&items, &fmt))
-        })
+        }
     }
 }

src/lists.rs

@@ -58,18 +58,24 @@ pub fn write_list<'b>(items: &[(String, String)], formatting: &ListFormatting<'b
     };
     let sep_len = formatting.separator.len();
     let total_sep_len = (sep_len + 1) * sep_count;
-
     let total_width = calculate_width(items);
+    let fits_single = total_width + total_sep_len <= formatting.h_width;
 
     // Check if we need to fallback from horizontal listing, if possible.
     if tactic == ListTactic::HorizontalVertical {
         debug!("write_list: total_width: {}, total_sep_len: {}, h_width: {}",
                total_width, total_sep_len, formatting.h_width);
-        if total_width + total_sep_len > formatting.h_width {
-            tactic = ListTactic::Vertical;
+        tactic = if fits_single {
+            ListTactic::Horizontal
         } else {
-            tactic = ListTactic::Horizontal;
-        }
+            ListTactic::Vertical
+        };
+    }
+
+    // Check if we can fit everything on a single line in mixed mode.
+    // The horizontal tactic does not break after v_width columns.
+    if tactic == ListTactic::Mixed && fits_single {
+        tactic = ListTactic::Horizontal;
     }
 
     // Now that we know how we will layout, we can decide for sure if there

src/visitor.rs

@@ -149,15 +149,16 @@ impl<'a, 'v> visit::Visitor<'v> for FmtVisitor<'a> {
                 match vp.node {
                     ast::ViewPath_::ViewPathList(ref path, ref path_list) => {
                         let block_indent = self.block_indent;
-                        let budget = IDEAL_WIDTH - block_indent;
-                        if let Some(new_str) = self.rewrite_use_list(block_indent,
-                                                                     budget,
-                                                                     path,
-                                                                     path_list,
-                                                                     item.vis) {
-                            self.changes.push_str_span(item.span, &new_str);
-                            self.last_pos = item.span.hi;
-                        }
+                        let one_line_budget = MAX_WIDTH - block_indent;
+                        let multi_line_budget = IDEAL_WIDTH - block_indent;
+                        let new_str = self.rewrite_use_list(block_indent,
+                                                            one_line_budget,
+                                                            multi_line_budget,
+                                                            path,
+                                                            path_list,
+                                                            item.vis);
+                        self.changes.push_str_span(item.span, &new_str);
+                        self.last_pos = item.span.hi;
                     }
                     ast::ViewPath_::ViewPathGlob(_) => {
                         // FIXME convert to list?

tests/idem/imports.rs

@@ -1,20 +1,20 @@
 // Imports.
 
 // Long import.
-use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic,
-                  ItemDefaultImpl};
+use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic, ItemDefaultImpl};
+use exceedingly::looooooooooooooooooooooooooooooooooooooooooooooooooooooooooong::import::path::{ItemA,
+                                                                                                ItemB};
 
 use {Foo, Bar};
 use Foo::{Bar, Baz};
-pub use syntax::ast::{Expr_, Expr, ExprAssign, ExprCall, ExprMethodCall,
-                      ExprPath};
+pub use syntax::ast::{Expr_, Expr, ExprAssign, ExprCall, ExprMethodCall, ExprPath};
 
 mod Foo {
-    pub use syntax::ast::{Expr_, ExprEval, ToExpr, ExprMethodCall, ToExprPath};
+    pub use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod, ItemStatic, ItemDefaultImpl};
 
     mod Foo2 {
-        pub use syntax::ast::{Expr_, ExprEval, ToExpr, ExprMethodCall,
-                              ToExprPath};
+        pub use syntax::ast::{ItemForeignMod, ItemImpl, ItemMac, ItemMod,
+                              ItemStatic, ItemDefaultImpl};
     }
 }