diff --git a/src/bin/rustfmt.rs b/src/bin/rustfmt.rs index fac4f9f8714..dc4087a066a 100644 --- a/src/bin/rustfmt.rs +++ b/src/bin/rustfmt.rs @@ -18,12 +18,14 @@ extern crate env_logger; extern crate getopts; use rustfmt::{run, Input, Summary}; -use rustfmt::config::Config; +use rustfmt::file_lines::FileLines; +use rustfmt::config::{Config, WriteMode}; use std::{env, error}; use std::fs::{self, File}; use std::io::{self, ErrorKind, Read, Write}; use std::path::{Path, PathBuf}; +use std::str::FromStr; use getopts::{Matches, Options}; @@ -61,8 +63,8 @@ enum Operation { struct CliOptions { skip_children: bool, verbose: bool, - write_mode: Option, - file_lines: Option, + write_mode: Option, + file_lines: FileLines, // Default is all lines in all files. } impl CliOptions { @@ -71,29 +73,28 @@ impl CliOptions { options.skip_children = matches.opt_present("skip-children"); options.verbose = matches.opt_present("verbose"); - if let Some(write_mode) = matches.opt_str("write-mode") { - options.write_mode = Some(write_mode); + if let Some(ref write_mode) = matches.opt_str("write-mode") { + if let Ok(write_mode) = WriteMode::from_str(write_mode) { + options.write_mode = Some(write_mode); + } else { + return Err(FmtError::from(format!("Invalid write-mode: {}", write_mode))); + } } - if let Some(file_lines) = matches.opt_str("file-lines") { - options.file_lines = Some(file_lines); + if let Some(ref file_lines) = matches.opt_str("file-lines") { + options.file_lines = file_lines.parse()?; } Ok(options) } - fn apply_to(&self, config: &mut Config) -> FmtResult<()> { - let bool_to_str = |b| if b { "true" } else { "false" }; - config - .override_value("skip_children", bool_to_str(self.skip_children))?; - config.override_value("verbose", bool_to_str(self.verbose))?; - if let Some(ref write_mode) = self.write_mode { - config.override_value("write_mode", &write_mode)?; + fn apply_to(self, config: &mut Config) { + config.set().skip_children(self.skip_children); + config.set().verbose(self.verbose); + config.set().file_lines(self.file_lines); + if let Some(write_mode) = self.write_mode { + config.set().write_mode(write_mode); } - if let Some(ref file_lines) = self.file_lines { - config.override_value("file_lines", &file_lines)?; - } - Ok(()) } } @@ -221,11 +222,11 @@ fn execute(opts: &Options) -> FmtResult { &env::current_dir().unwrap())?; // write_mode is always Plain for Stdin. - config.override_value("write_mode", "Plain")?; + config.set().write_mode(WriteMode::Plain); // parse file_lines if let Some(ref file_lines) = matches.opt_str("file-lines") { - config.override_value("file-lines", file_lines)?; + config.set().file_lines(file_lines.parse()?); for f in config.file_lines().files() { if f != "stdin" { println!("Warning: Extra file listed in file_lines option '{}'", f); @@ -238,6 +239,12 @@ fn execute(opts: &Options) -> FmtResult { Operation::Format { files, config_path } => { let options = CliOptions::from_matches(&matches)?; + for f in options.file_lines.files() { + if !files.contains(&PathBuf::from(f)) { + println!("Warning: Extra file listed in file_lines option '{}'", f); + } + } + let mut config = Config::default(); let mut path = None; // Load the config path file if provided @@ -246,13 +253,6 @@ fn execute(opts: &Options) -> FmtResult { config = cfg_tmp; path = path_tmp; }; - options.apply_to(&mut config)?; - - for f in config.file_lines().files() { - if !files.contains(&PathBuf::from(f)) { - println!("Warning: Extra file listed in file_lines option '{}'", f); - } - } if options.verbose { if let Some(path) = path.as_ref() { @@ -282,7 +282,7 @@ fn execute(opts: &Options) -> FmtResult { config = config_tmp; } - options.apply_to(&mut config)?; + options.clone().apply_to(&mut config); error_summary.add(run(Input::File(file), &config)); } } diff --git a/src/comment.rs b/src/comment.rs index ad52ec67f33..22fa241677f 100644 --- a/src/comment.rs +++ b/src/comment.rs @@ -731,10 +731,8 @@ mod test { #[cfg_attr(rustfmt, rustfmt_skip)] fn format_comments() { let mut config: ::config::Config = Default::default(); - config.override_value("wrap_comments", "true") - .expect("Could not set wrap_comments to true"); - config.override_value("normalize_comments", "true") - .expect("Could not set normalize_comments to true"); + config.set().wrap_comments(true); + config.set().normalize_comments(true); let comment = rewrite_comment(" //test", true, diff --git a/src/config.rs b/src/config.rs index 9f74f2b69d6..d629d4176ca 100644 --- a/src/config.rs +++ b/src/config.rs @@ -11,8 +11,6 @@ extern crate toml; use std::cell::Cell; -use std::error; -use std::result; use file_lines::FileLines; use lists::{SeparatorTactic, ListTactic}; @@ -231,6 +229,21 @@ macro_rules! create_config { $(pub $i: Option<$ty>),+ } + // Macro hygiene won't allow us to make `set_$i()` methods on Config + // for each item, so this struct is used to give the API to set values: + // `config.get().option(false)`. It's pretty ugly. Consider replacing + // with `config.set_option(false)` if we ever get a stable/usable + // `concat_idents!()`. + pub struct ConfigSetter<'a>(&'a mut Config); + + impl<'a> ConfigSetter<'a> { + $( + pub fn $i(&mut self, value: $ty) { + (self.0).$i.1 = value; + } + )+ + } + impl Config { $( @@ -240,6 +253,10 @@ macro_rules! create_config { } )+ + pub fn set<'a>(&'a mut self) -> ConfigSetter<'a> { + ConfigSetter(self) + } + fn fill_from_parsed_config(mut self, parsed: PartialConfig) -> Config { $( if let Some(val) = parsed.$i { @@ -316,15 +333,19 @@ macro_rules! create_config { } pub fn override_value(&mut self, key: &str, val: &str) - -> result::Result<(), Box> { match key { $( - stringify!($i) => self.$i.1 = val.parse::<$ty>()?, + stringify!($i) => { + self.$i.1 = val.parse::<$ty>() + .expect(&format!("Failed to parse override for {} (\"{}\") as a {}", + stringify!($i), + val, + stringify!($ty))); + } )+ _ => panic!("Unknown config key in override: {}", key) } - Ok(()) } pub fn print_docs() { @@ -480,4 +501,13 @@ mod test { assert!(config.skip_children.0.get()); assert!(!config.disable_all_formatting.0.get()); } + + #[test] + fn test_config_set() { + let mut config = Config::default(); + config.set().verbose(false); + assert_eq!(config.verbose(), false); + config.set().verbose(true); + assert_eq!(config.verbose(), true); + } } diff --git a/src/lib.rs b/src/lib.rs index 8c112b5f8af..f31f3c0c3ee 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -708,9 +708,7 @@ mod test { #[test] fn indent_to_string_hard_tabs() { let mut config = Config::default(); - config - .override_value("hard_tabs", "true") - .expect("Could not set hard_tabs to true"); + config.set().hard_tabs(true); let indent = Indent::new(8, 4); // 2 tabs + 4 spaces diff --git a/tests/system.rs b/tests/system.rs index 91386a7f768..de4b1c46267 100644 --- a/tests/system.rs +++ b/tests/system.rs @@ -20,7 +20,7 @@ use std::path::{Path, PathBuf}; use rustfmt::*; use rustfmt::filemap::{write_system_newlines, FileMap}; -use rustfmt::config::Config; +use rustfmt::config::{Config, ReportTactic}; use rustfmt::rustfmt_diff::*; const DIFF_CONTEXT_SIZE: usize = 3; @@ -224,16 +224,12 @@ fn read_config(filename: &str) -> Config { for (key, val) in &sig_comments { if key != "target" && key != "config" { - config - .override_value(key, val) - .expect(&format!("Failed to override config {} (\"{}\")", key, val)); + config.override_value(key, val); } } // Don't generate warnings for to-do items. - config - .override_value("report_todo", "Never") - .expect("Could not set report-todo to Never"); + config.set().report_todo(ReportTactic::Never); config }