Split ListTactic decision logic from write_list

src/config.rs

@@ -90,6 +90,7 @@ configuration_option_enum! { LicensePolicy:
     FileLicense,
 }
 
+// TODO: this is not necessary any more
 configuration_option_enum! { MultilineStyle:
     // Use horizontal layout if it fits in one line, fall back to vertical
     PreferSingle,
@@ -260,9 +261,9 @@ create_config! {
     max_width: usize, 100, "Maximum width of each line";
     ideal_width: usize, 80, "Ideal width of each line";
     tab_spaces: usize, 4, "Number of spaces per tab";
-    fn_call_width: usize, 50,
+    fn_call_width: usize, 55,
         "Maximum width of the args of a function call before faling back to vertical formatting";
-    struct_lit_width: usize, 12,
+    struct_lit_width: usize, 16,
         "Maximum width in the body of a struct lit before faling back to vertical formatting";
     newline_style: NewlineStyle, NewlineStyle::Unix, "Unix or Windows line endings";
     fn_brace_style: BraceStyle, BraceStyle::SameLineWhere, "Brace style for functions";

src/expr.rs

@@ -13,7 +13,8 @@ use std::borrow::Borrow;
 
 use Indent;
 use rewrite::{Rewrite, RewriteContext};
-use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic};
+use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic,
+            DefinitiveListTactic, definitive_tactic};
 use string::{StringFormat, rewrite_string};
 use utils::{span_after, extra_offset, last_line_width, wrap_str, binary_search};
 use visitor::FmtVisitor;
@@ -147,7 +148,13 @@ impl Rewrite for ast::Expr {
                 Some(format!("break{}", id_str))
             }
             ast::Expr_::ExprClosure(capture, ref fn_decl, ref body) => {
-                rewrite_closure(capture, fn_decl, body, self.span, context, width, offset)
+                rewrite_closure(capture,
+                                fn_decl,
+                                body,
+                                self.span,
+                                context,
+                                width,
+                                offset)
             }
             ast::Expr_::ExprField(..) |
             ast::Expr_::ExprTupField(..) |
@@ -223,9 +230,9 @@ pub fn rewrite_array<'a, I>(expr_iter: I,
                     .collect::<Vec<_>>();
 
     let tactic = if items.iter().any(|li| li.item.len() > 10 || li.is_multiline()) {
-        ListTactic::HorizontalVertical
+        definitive_tactic(&items, ListTactic::HorizontalVertical, max_item_width)
     } else {
-        ListTactic::Mixed
+        DefinitiveListTactic::Mixed
     };
 
     let fmt = ListFormatting {
@@ -233,8 +240,7 @@ pub fn rewrite_array<'a, I>(expr_iter: I,
         separator: ",",
         trailing_separator: SeparatorTactic::Never,
         indent: offset + 1,
-        h_width: max_item_width,
+        width: max_item_width,
-        v_width: max_item_width,
         ends_with_newline: false,
         config: context.config,
     };
@@ -285,18 +291,25 @@ fn rewrite_closure(capture: ast::CaptureClause,
                                  },
                                  span_after(span, "|", context.codemap),
                                  body.span.lo);
+    let item_vec = arg_items.collect::<Vec<_>>();
+    let tactic = definitive_tactic(&item_vec,
+                                   ListTactic::HorizontalVertical,
+                                   horizontal_budget);
+    let budget = match tactic {
+        DefinitiveListTactic::Horizontal => horizontal_budget,
+        _ => budget,
+    };
 
     let fmt = ListFormatting {
-        tactic: ListTactic::HorizontalVertical,
+        tactic: tactic,
         separator: ",",
         trailing_separator: SeparatorTactic::Never,
         indent: argument_offset,
-        h_width: horizontal_budget,
+        width: budget,
-        v_width: budget,
         ends_with_newline: false,
         config: context.config,
     };
-    let list_str = try_opt!(write_list(&arg_items.collect::<Vec<_>>(), &fmt));
+    let list_str = try_opt!(write_list(&item_vec, &fmt));
     let mut prefix = format!("{}|{}|", mover, list_str);
 
     if !ret_str.is_empty() {
@@ -586,7 +599,11 @@ fn rewrite_if_else(context: &RewriteContext,
 
     // Try to format if-else on single line.
     if allow_single_line && context.config.single_line_if_else {
-        let trial = single_line_if_else(context, &pat_expr_string, if_block, else_block_opt, width);
+        let trial = single_line_if_else(context,
+                                        &pat_expr_string,
+                                        if_block,
+                                        else_block_opt,
+                                        width);
 
         if trial.is_some() {
             return trial;
@@ -1074,7 +1091,13 @@ pub fn rewrite_call<R>(context: &RewriteContext,
     where R: Rewrite
 {
     let closure = |callee_max_width| {
-        rewrite_call_inner(context, callee, callee_max_width, args, span, width, offset)
+        rewrite_call_inner(context,
+                           callee,
+                           callee_max_width,
+                           args,
+                           span,
+                           width,
+                           offset)
     };
 
     // 2 is for parens
@@ -1136,8 +1159,7 @@ fn rewrite_call_inner<R>(context: &RewriteContext,
                              span.lo,
                              span.hi);
 
-    let fmt = ListFormatting::for_fn(remaining_width, offset, context.config);
+    let list_str = match ::lists::format_fn_args(items, remaining_width, offset, context.config) {
-    let list_str = match write_list(&items.collect::<Vec<_>>(), &fmt) {
         Some(str) => str,
         None => return Err(Ordering::Less),
     };
@@ -1150,9 +1172,7 @@ fn rewrite_paren(context: &RewriteContext,
                  width: usize,
                  offset: Indent)
                  -> Option<String> {
-    debug!("rewrite_paren, width: {}, offset: {:?}",
+    debug!("rewrite_paren, width: {}, offset: {:?}", 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.
     let subexpr_str = subexpr.rewrite(context, try_opt!(width.checked_sub(2)), offset + 1);
@@ -1248,15 +1268,25 @@ fn rewrite_struct_lit<'a>(context: &RewriteContext,
                              },
                              span_after(span, "{", context.codemap),
                              span.hi);
+    let item_vec = items.collect::<Vec<_>>();
 
-    let mut tactic = match (context.config.struct_lit_style, fields.len()) {
+    let tactic = {
-        (StructLitStyle::Visual, 1) => ListTactic::HorizontalVertical,
+        let mut prelim_tactic = match (context.config.struct_lit_style, fields.len()) {
-        _ => context.config.struct_lit_multiline_style.to_list_tactic(),
+            (StructLitStyle::Visual, 1) => ListTactic::HorizontalVertical,
+            _ => context.config.struct_lit_multiline_style.to_list_tactic(),
+        };
+
+        if prelim_tactic == ListTactic::HorizontalVertical && fields.len() > 1 {
+            prelim_tactic = ListTactic::LimitedHorizontalVertical(context.config.struct_lit_width);
+        };
+
+        definitive_tactic(&item_vec, prelim_tactic, h_budget)
     };
 
-    if tactic == ListTactic::HorizontalVertical && fields.len() > 1 {
+    let budget = match tactic {
-        tactic = ListTactic::LimitedHorizontalVertical(context.config.struct_lit_width);
+        DefinitiveListTactic::Horizontal => h_budget,
-    }
+        _ => v_budget,
+    };
 
     let fmt = ListFormatting {
         tactic: tactic,
@@ -1267,12 +1297,11 @@ fn rewrite_struct_lit<'a>(context: &RewriteContext,
             context.config.struct_lit_trailing_comma
         },
         indent: indent,
-        h_width: h_budget,
+        width: budget,
-        v_width: v_budget,
         ends_with_newline: false,
         config: context.config,
     };
-    let fields_str = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
+    let fields_str = try_opt!(write_list(&item_vec, &fmt));
 
     let format_on_newline = || {
         let inner_indent = context.block_indent
@@ -1340,10 +1369,8 @@ fn rewrite_tuple_lit(context: &RewriteContext,
                              },
                              span.lo + BytePos(1), // Remove parens
                              span.hi - BytePos(1));
-
     let budget = try_opt!(width.checked_sub(2));
-    let fmt = ListFormatting::for_fn(budget, indent, context.config);
+    let list_str = try_opt!(::lists::format_fn_args(items, budget, indent, context.config));
-    let list_str = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
 
     Some(format!("({})", list_str))
 }

src/imports.rs

@@ -9,7 +9,7 @@
 // except according to those terms.
 
 use Indent;
-use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, ListTactic};
+use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic};
 use utils::span_after;
 use rewrite::{Rewrite, RewriteContext};
 
@@ -124,20 +124,6 @@ pub fn rewrite_use_list(width: usize,
     // 1 = }
     let remaining_width = width.checked_sub(supp_indent + 1).unwrap_or(0);
 
-    let fmt = ListFormatting {
-        tactic: ListTactic::Mixed,
-        separator: ",",
-        trailing_separator: SeparatorTactic::Never,
-        indent: offset + supp_indent,
-        h_width: remaining_width,
-        // FIXME This is too conservative, and will not use all width
-        // available
-        // (loose 1 column (";"))
-        v_width: remaining_width,
-        ends_with_newline: false,
-        config: context.config,
-    };
-
     let mut items = {
         // Dummy value, see explanation below.
         let mut items = vec![ListItem::from_str("")];
@@ -169,6 +155,21 @@ pub fn rewrite_use_list(width: usize,
         items[1..].sort_by(|a, b| a.item.cmp(&b.item));
     }
 
+    let tactic = ::lists::definitive_tactic(&items[first_index..],
+                                            ::lists::ListTactic::Mixed,
+                                            remaining_width);
+    let fmt = ListFormatting {
+        tactic: tactic,
+        separator: ",",
+        trailing_separator: SeparatorTactic::Never,
+        indent: offset + supp_indent,
+        // FIXME This is too conservative, and will not use all width
+        // available
+        // (loose 1 column (";"))
+        width: remaining_width,
+        ends_with_newline: false,
+        config: context.config,
+    };
     let list_str = try_opt!(write_list(&items[first_index..], &fmt));
 
     Some(if path_str.is_empty() {

src/items.rs

@@ -12,7 +12,8 @@
 
 use Indent;
 use utils::{format_mutability, format_visibility, contains_skip, span_after, end_typaram, wrap_str};
-use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, ListTactic};
+use lists::{write_list, itemize_list, ListItem, ListFormatting, SeparatorTactic, ListTactic,
+            DefinitiveListTactic, definitive_tactic};
 use expr::rewrite_assign_rhs;
 use comment::FindUncommented;
 use visitor::FmtVisitor;
@@ -499,13 +500,20 @@ impl<'a> FmtVisitor<'a> {
             BlockIndentStyle::Visual => arg_indent,
         };
 
+        let tactic = definitive_tactic(&arg_items,
+                                       self.config.fn_args_density.to_list_tactic(),
+                                       one_line_budget);
+        let budget = match tactic {
+            DefinitiveListTactic::Horizontal => one_line_budget,
+            _ => multi_line_budget,
+        };
+
         let fmt = ListFormatting {
-            tactic: self.config.fn_args_density.to_list_tactic(),
+            tactic: tactic,
             separator: ",",
             trailing_separator: SeparatorTactic::Never,
             indent: indent,
-            h_width: one_line_budget,
+            width: budget,
-            v_width: multi_line_budget,
             ends_with_newline: false,
             config: self.config,
         };
@@ -630,6 +638,7 @@ impl<'a> FmtVisitor<'a> {
                                              },
                                              span_after(field.span, "(", self.codemap),
                                              next_span_start);
+                    let item_vec = items.collect::<Vec<_>>();
 
                     result.push('(');
 
@@ -641,18 +650,20 @@ impl<'a> FmtVisitor<'a> {
                         0
                     };
                     let budget = self.config.max_width - indent.width() - comma_cost - 1; // 1 = )
+                    let tactic = definitive_tactic(&item_vec,
+                                                   ListTactic::HorizontalVertical,
+                                                   budget);
 
                     let fmt = ListFormatting {
-                        tactic: ListTactic::HorizontalVertical,
+                        tactic: tactic,
                         separator: ",",
                         trailing_separator: SeparatorTactic::Never,
                         indent: indent,
-                        h_width: budget,
+                        width: budget,
-                        v_width: budget,
                         ends_with_newline: true,
                         config: self.config,
                     };
-                    let list_str = match write_list(&items.collect::<Vec<_>>(), &fmt) {
+                    let list_str = match write_list(&item_vec, &fmt) {
                         Some(list_str) => list_str,
                         None => return,
                     };
@@ -766,9 +777,9 @@ impl<'a> FmtVisitor<'a> {
             result.push('\n');
             result.push_str(&indentation);
 
-            ListTactic::Vertical
+            DefinitiveListTactic::Vertical
         } else {
-            ListTactic::Horizontal
+            DefinitiveListTactic::Horizontal
         };
 
         // 1 = ,
@@ -778,13 +789,12 @@ impl<'a> FmtVisitor<'a> {
             separator: ",",
             trailing_separator: self.config.struct_trailing_comma,
             indent: offset.block_indent(self.config),
-            h_width: self.config.max_width,
+            width: budget,
-            v_width: budget,
             ends_with_newline: true,
             config: self.config,
         };
 
-        let list_str = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
+        let list_str = try_opt!(write_list(items, &fmt));
         result.push_str(&list_str);
 
         if break_line {
@@ -930,21 +940,15 @@ impl<'a> FmtVisitor<'a> {
         let ty_spans = tys.iter().map(span_for_ty_param);
 
         let items = itemize_list(self.codemap,
-                                 lt_spans.chain(ty_spans),
+                                 lt_spans.chain(ty_spans).zip(lt_strs.chain(ty_strs)),
                                  ">",
-                                 |sp| sp.lo,
+                                 |&(sp, _)| sp.lo,
-                                 |sp| sp.hi,
+                                 |&(sp, _)| sp.hi,
-                                 |_| String::new(),
+                                 // FIXME: don't clone
+                                 |&(_, ref str)| str.clone(),
                                  span_after(span, "<", self.codemap),
                                  span.hi);
-        let mut items = items.collect::<Vec<_>>();
+        let list_str = try_opt!(::lists::format_item_list(items, h_budget, offset, self.config));
-
-        for (item, ty) in items.iter_mut().zip(lt_strs.chain(ty_strs)) {
-            item.item = ty;
-        }
-
-        let fmt = ListFormatting::for_item(h_budget, offset, self.config);
-        let list_str = try_opt!(write_list(&items, &fmt));
 
         Some(format!("<{}>", list_str))
     }
@@ -990,18 +994,20 @@ impl<'a> FmtVisitor<'a> {
                                  |pred| pred.rewrite(&context, budget, offset).unwrap(),
                                  span_start,
                                  span_end);
+        let item_vec = items.collect::<Vec<_>>();
+        // FIXME: we don't need to collect here if the where_layout isnt horizontalVertical
+        let tactic = definitive_tactic(&item_vec, self.config.where_layout, budget);
 
         let fmt = ListFormatting {
-            tactic: self.config.where_layout,
+            tactic: tactic,
             separator: ",",
             trailing_separator: SeparatorTactic::Never,
             indent: offset,
-            h_width: budget,
+            width: budget,
-            v_width: budget,
             ends_with_newline: true,
             config: self.config,
         };
-        let preds_str = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
+        let preds_str = try_opt!(write_list(&item_vec, &fmt));
 
         // 9 = " where ".len() + " {".len()
         if density == Density::Tall || preds_str.contains('\n') ||

src/lib.rs

@@ -119,8 +119,7 @@ impl Indent {
 
     pub fn to_string(&self, config: &Config) -> String {
         let (num_tabs, num_spaces) = if config.hard_tabs {
-            (self.block_indent / config.tab_spaces,
+            (self.block_indent / config.tab_spaces, self.alignment)
-             self.alignment)
         } else {
             (0, self.block_indent + self.alignment)
         };

src/lists.rs

@@ -14,7 +14,7 @@ use std::iter::Peekable;
 use syntax::codemap::{self, CodeMap, BytePos};
 
 use Indent;
-use utils::{round_up_to_power_of_two, wrap_str};
+use utils::wrap_str;
 use comment::{FindUncommented, rewrite_comment, find_comment_end};
 use config::Config;
 
@@ -44,46 +44,69 @@ pub enum SeparatorTactic {
 impl_enum_decodable!(SeparatorTactic, Always, Never, Vertical);
 
 // TODO having some helpful ctors for ListFormatting would be nice.
+// FIXME: this should have only 1 width param
 pub struct ListFormatting<'a> {
-    pub tactic: ListTactic,
+    pub tactic: DefinitiveListTactic,
     pub separator: &'a str,
     pub trailing_separator: SeparatorTactic,
     pub indent: Indent,
-    // Available width if we layout horizontally.
+    pub width: usize,
-    pub h_width: usize,
-    // Available width if we layout vertically
-    pub v_width: usize,
     // Non-expressions, e.g. items, will have a new line at the end of the list.
     // Important for comment styles.
     pub ends_with_newline: bool,
     pub config: &'a Config,
 }
 
-impl<'a> ListFormatting<'a> {
+pub fn format_fn_args<I>(items: I, width: usize, offset: Indent, config: &Config) -> Option<String>
-    pub fn for_fn(width: usize, offset: Indent, config: &'a Config) -> ListFormatting<'a> {
+    where I: Iterator<Item = ListItem>
-        ListFormatting {
+{
-            tactic: ListTactic::LimitedHorizontalVertical(config.fn_call_width),
+    list_helper(items,
-            separator: ",",
+                width,
-            trailing_separator: SeparatorTactic::Never,
+                offset,
-            indent: offset,
+                config,
-            h_width: width,
+                ListTactic::LimitedHorizontalVertical(config.fn_call_width))
-            v_width: width,
+}
-            ends_with_newline: false,
-            config: config,
-        }
-    }
 
-    pub fn for_item(width: usize, offset: Indent, config: &'a Config) -> ListFormatting<'a> {
+pub fn format_item_list<I>(items: I,
-        ListFormatting {
+                           width: usize,
-            tactic: ListTactic::HorizontalVertical,
+                           offset: Indent,
-            separator: ",",
+                           config: &Config)
-            trailing_separator: SeparatorTactic::Never,
+                           -> Option<String>
-            indent: offset,
+    where I: Iterator<Item = ListItem>
-            h_width: width,
+{
-            v_width: width,
+    list_helper(items,
-            ends_with_newline: false,
+                width,
-            config: config,
+                offset,
-        }
+                config,
+                ListTactic::HorizontalVertical)
+}
+
+fn list_helper<I>(items: I,
+                  width: usize,
+                  offset: Indent,
+                  config: &Config,
+                  tactic: ListTactic)
+                  -> Option<String>
+    where I: Iterator<Item = ListItem>
+{
+    let item_vec: Vec<_> = items.collect();
+    let tactic = definitive_tactic(&item_vec, tactic, width);
+    let fmt = ListFormatting {
+        tactic: tactic,
+        separator: ",",
+        trailing_separator: SeparatorTactic::Never,
+        indent: offset,
+        width: width,
+        ends_with_newline: false,
+        config: config,
+    };
+
+    write_list(&item_vec, &fmt)
+}
+
+impl AsRef<ListItem> for ListItem {
+    fn as_ref(&self) -> &ListItem {
+        self
     }
 }
 
@@ -116,76 +139,67 @@ impl ListItem {
     }
 }
 
-// Format a list of commented items into a string.
+#[derive(Eq, PartialEq, Debug, Copy, Clone)]
-// FIXME: this has grown into a monstrosity
+pub enum DefinitiveListTactic {
-// TODO: add unit tests
+    Vertical,
-pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> Option<String> {
+    Horizontal,
-    if items.is_empty() {
+    Mixed,
-        return Some(String::new());
+}
-    }
 
-    let mut tactic = formatting.tactic;
+pub fn definitive_tactic<'t, I, T>(items: I,
+                                   tactic: ListTactic,
+                                   width: usize)
+                                   -> DefinitiveListTactic
+    where I: IntoIterator<Item = T> + Clone,
+          T: AsRef<ListItem>
+{
+    let pre_line_comments = items.clone()
+                                 .into_iter()
+                                 .any(|item| item.as_ref().has_line_pre_comment());
 
-    // Conservatively overestimates because of the changing separator tactic.
+    let limit = match tactic {
-    let sep_count = if formatting.trailing_separator == SeparatorTactic::Always {
+        _ if pre_line_comments => return DefinitiveListTactic::Vertical,
-        items.len()
+        ListTactic::Mixed => return DefinitiveListTactic::Mixed,
-    } else {
+        ListTactic::Horizontal => return DefinitiveListTactic::Horizontal,
-        items.len() - 1
+        ListTactic::Vertical => return DefinitiveListTactic::Vertical,
+        ListTactic::LimitedHorizontalVertical(limit) => ::std::cmp::min(width, limit),
+        ListTactic::HorizontalVertical => width,
     };
+
+    let (sep_count, total_width) = calculate_width(items.clone());
+    let sep_len = ", ".len(); // FIXME: make more generic?
+    let total_sep_len = sep_len * sep_count.checked_sub(1).unwrap_or(0);
+    let real_total = total_width + total_sep_len;
+
+    if real_total <= limit && !pre_line_comments &&
+       !items.into_iter().any(|item| item.as_ref().is_multiline()) {
+        DefinitiveListTactic::Horizontal
+    } else {
+        DefinitiveListTactic::Vertical
+    }
+}
+
+// Format a list of commented items into a string.
+// TODO: add unit tests
+pub fn write_list<'b, I, T>(items: I, formatting: &ListFormatting<'b>) -> Option<String>
+    where I: IntoIterator<Item = T>,
+          T: AsRef<ListItem>
+{
+    let tactic = formatting.tactic;
     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 let ListTactic::LimitedHorizontalVertical(limit) = tactic {
-        if total_width > limit {
-            tactic = ListTactic::Vertical;
-        } else {
-            tactic = ListTactic::HorizontalVertical;
-        }
-    }
-    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 && !items.iter().any(ListItem::is_multiline) {
-            ListTactic::Horizontal
-        } else {
-            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;
-    }
-
-    // Switch to vertical mode if we find non-block comments.
-    if items.iter().any(ListItem::has_line_pre_comment) {
-        tactic = ListTactic::Vertical;
-    }
 
     // Now that we know how we will layout, we can decide for sure if there
     // will be a trailing separator.
     let trailing_separator = needs_trailing_separator(formatting.trailing_separator, tactic);
-
+    let mut result = String::new();
-    // Create a buffer for the result.
+    let mut iter = items.into_iter().enumerate().peekable();
-    // TODO could use a StringBuffer or rope for this
-    let alloc_width = if tactic == ListTactic::Horizontal {
-        total_width + total_sep_len
-    } else {
-        total_width + items.len() * (formatting.indent.width() + 1)
-    };
-    let mut result = String::with_capacity(round_up_to_power_of_two(alloc_width));
 
     let mut line_len = 0;
     let indent_str = &formatting.indent.to_string(formatting.config);
-    for (i, item) in items.iter().enumerate() {
+    while let Some((i, item)) = iter.next() {
+        let item = item.as_ref();
         let first = i == 0;
-        let last = i == items.len() - 1;
+        let last = iter.peek().is_none();
         let separate = !last || trailing_separator;
         let item_sep_len = if separate {
             sep_len
@@ -195,17 +209,17 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> Op
         let item_width = item.item.len() + item_sep_len;
 
         match tactic {
-            ListTactic::Horizontal if !first => {
+            DefinitiveListTactic::Horizontal if !first => {
                 result.push(' ');
             }
-            ListTactic::Vertical if !first => {
+            DefinitiveListTactic::Vertical if !first => {
                 result.push('\n');
                 result.push_str(indent_str);
             }
-            ListTactic::Mixed => {
+            DefinitiveListTactic::Mixed => {
                 let total_width = total_item_width(item) + item_sep_len;
 
-                if line_len > 0 && line_len + total_width > formatting.v_width {
+                if line_len > 0 && line_len + total_width > formatting.width {
                     result.push('\n');
                     result.push_str(indent_str);
                     line_len = 0;
@@ -224,9 +238,9 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> Op
         // Pre-comments
         if let Some(ref comment) = item.pre_comment {
             // Block style in non-vertical mode.
-            let block_mode = tactic != ListTactic::Vertical;
+            let block_mode = tactic != DefinitiveListTactic::Vertical;
             // Width restriction is only relevant in vertical mode.
-            let max_width = formatting.v_width;
+            let max_width = formatting.width;
             let comment = try_opt!(rewrite_comment(comment,
                                                    block_mode,
                                                    max_width,
@@ -234,7 +248,7 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> Op
                                                    formatting.config));
             result.push_str(&comment);
 
-            if tactic == ListTactic::Vertical {
+            if tactic == DefinitiveListTactic::Vertical {
                 result.push('\n');
                 result.push_str(indent_str);
             } else {
@@ -244,16 +258,16 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> Op
 
         let item_str = try_opt!(wrap_str(&item.item[..],
                                          formatting.config.max_width,
-                                         formatting.v_width,
+                                         formatting.width,
                                          formatting.indent));
         result.push_str(&item_str);
 
         // Post-comments
-        if tactic != ListTactic::Vertical && item.post_comment.is_some() {
+        if tactic != DefinitiveListTactic::Vertical && item.post_comment.is_some() {
             let comment = item.post_comment.as_ref().unwrap();
             let formatted_comment = try_opt!(rewrite_comment(comment,
                                                              true,
-                                                             formatting.v_width,
+                                                             formatting.width,
                                                              Indent::empty(),
                                                              formatting.config));
 
@@ -265,9 +279,9 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> Op
             result.push_str(formatting.separator);
         }
 
-        if tactic == ListTactic::Vertical && item.post_comment.is_some() {
+        if tactic == DefinitiveListTactic::Vertical && item.post_comment.is_some() {
             // 1 = space between item and comment.
-            let width = formatting.v_width.checked_sub(item_width + 1).unwrap_or(1);
+            let width = formatting.width.checked_sub(item_width + 1).unwrap_or(1);
             let mut offset = formatting.indent;
             offset.alignment += item_width + 1;
             let comment = item.post_comment.as_ref().unwrap();
@@ -286,7 +300,7 @@ pub fn write_list<'b>(items: &[ListItem], formatting: &ListFormatting<'b>) -> Op
             result.push_str(&formatted_comment);
         }
 
-        if !last && tactic == ListTactic::Vertical && item.new_lines {
+        if !last && tactic == DefinitiveListTactic::Vertical && item.new_lines {
             result.push('\n');
         }
     }
@@ -449,25 +463,34 @@ pub fn itemize_list<'a, T, I, F1, F2, F3>(codemap: &'a CodeMap,
     }
 }
 
-fn needs_trailing_separator(separator_tactic: SeparatorTactic, list_tactic: ListTactic) -> bool {
+fn needs_trailing_separator(separator_tactic: SeparatorTactic,
+                            list_tactic: DefinitiveListTactic)
+                            -> bool {
     match separator_tactic {
         SeparatorTactic::Always => true,
-        SeparatorTactic::Vertical => list_tactic == ListTactic::Vertical,
+        SeparatorTactic::Vertical => list_tactic == DefinitiveListTactic::Vertical,
         SeparatorTactic::Never => false,
     }
 }
 
-fn calculate_width(items: &[ListItem]) -> usize {
+/// Returns the count and total width of the list items.
-    items.iter().map(total_item_width).fold(0, |a, l| a + l)
+fn calculate_width<'li, I, T>(items: I) -> (usize, usize)
+    where I: IntoIterator<Item = T>,
+          T: AsRef<ListItem>
+{
+    items.into_iter()
+         .map(|item| total_item_width(item.as_ref()))
+         .fold((0, 0), |acc, l| (acc.0 + 1, acc.1 + l))
 }
 
 fn total_item_width(item: &ListItem) -> usize {
-    comment_len(&item.pre_comment) + comment_len(&item.post_comment) + item.item.len()
+    comment_len(item.pre_comment.as_ref().map(|x| &(*x)[..])) +
+    comment_len(item.post_comment.as_ref().map(|x| &(*x)[..])) + item.item.len()
 }
 
-fn comment_len(comment: &Option<String>) -> usize {
+fn comment_len(comment: Option<&str>) -> usize {
-    match *comment {
+    match comment {
-        Some(ref s) => {
+        Some(s) => {
             let text_len = s.trim().len();
             if text_len > 0 {
                 // We'll put " /*" before and " */" after inline comments.

src/macros.rs

@@ -89,7 +89,12 @@ pub fn rewrite_macro(mac: &ast::Mac,
     match style {
         MacroStyle::Parens => {
             // Format macro invocation as function call.
-            rewrite_call(context, &macro_name, &expr_vec, mac.span, width, offset)
+            rewrite_call(context,
+                         &macro_name,
+                         &expr_vec,
+                         mac.span,
+                         width,
+                         offset)
         }
         MacroStyle::Brackets => {
             // Format macro invocation as array literal.

src/types.rs

@@ -13,7 +13,7 @@ use syntax::print::pprust;
 use syntax::codemap::{self, Span, BytePos, CodeMap};
 
 use Indent;
-use lists::{itemize_list, write_list, ListFormatting};
+use lists::itemize_list;
 use rewrite::{Rewrite, RewriteContext};
 use utils::{extra_offset, span_after, format_mutability, wrap_str};
 
@@ -206,9 +206,7 @@ fn rewrite_segment(segment: &ast::PathSegment,
                                  .collect::<Vec<_>>();
 
             let next_span_lo = param_list.last().unwrap().get_span().hi + BytePos(1);
-            let list_lo = span_after(codemap::mk_sp(*span_lo, span_hi),
+            let list_lo = span_after(codemap::mk_sp(*span_lo, span_hi), "<", context.codemap);
-                                     "<",
-                                     context.codemap);
             let separator = get_path_separator(context.codemap, *span_lo, list_lo);
 
             // 1 for <
@@ -232,9 +230,10 @@ fn rewrite_segment(segment: &ast::PathSegment,
                                      },
                                      list_lo,
                                      span_hi);
-
+            let list_str = try_opt!(::lists::format_item_list(items,
-            let fmt = ListFormatting::for_item(list_width, offset + extra_offset, context.config);
+                                                              list_width,
-            let list_str = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
+                                                              offset + extra_offset,
+                                                              context.config));
 
             // Update position of last bracket.
             *span_lo = next_span_lo;
@@ -263,9 +262,7 @@ fn rewrite_segment(segment: &ast::PathSegment,
                                      |ty| ty.rewrite(context, budget, offset).unwrap(),
                                      list_lo,
                                      span_hi);
-
+            let list_str = try_opt!(::lists::format_fn_args(items, budget, offset, context.config));
-            let fmt = ListFormatting::for_fn(budget, offset, context.config);
-            let list_str = try_opt!(write_list(&items.collect::<Vec<_>>(), &fmt));
 
             format!("({}){}", list_str, output)
         }