diff --git a/src/indexer.rs b/src/indexer.rs index 876a5ad..f75bc8c 100644 --- a/src/indexer.rs +++ b/src/indexer.rs @@ -202,8 +202,8 @@ impl From for TablesIndex { } /// Returns all roms names lower case for the roms in the given folder -pub fn find_roms>(rom_path: P) -> io::Result> { - if !rom_path.as_ref().exists() { +pub fn find_roms(rom_path: &Path) -> io::Result> { + if !rom_path.exists() { return Ok(HashMap::new()); } // TODO @@ -232,13 +232,10 @@ pub fn find_roms>(rom_path: P) -> io::Result>( - recursive: bool, - tables_path: P, -) -> io::Result> { +pub fn find_vpx_files(recursive: bool, tables_path: &Path) -> io::Result> { if recursive { let mut vpx_files = Vec::new(); - let mut entries = walk_dir_filtered(&tables_path); + let mut entries = walk_dir_filtered(tables_path); entries.try_for_each(|entry| { let dir_entry = entry?; let path = dir_entry.path(); @@ -277,9 +274,7 @@ pub fn find_vpx_files>( } /// Walks the directory and filters out .git and __MACOSX folders -fn walk_dir_filtered>( - tables_path: &P, -) -> FilterEntry bool> { +fn walk_dir_filtered(tables_path: &Path) -> FilterEntry bool> { WalkDir::new(tables_path).into_iter().filter_entry(|entry| { let path = entry.path(); let git = std::path::Component::Normal(".git".as_ref()); @@ -319,31 +314,29 @@ impl From for IndexError { /// Returns the index. /// If the index file already exists, it will be read and updated. /// If the index file does not exist, it will be created. -pub fn index_folder>( +pub fn index_folder( recursive: bool, - tables_folder: P, - tables_index_path: P, - global_roms_path: Option

, + tables_folder: &Path, + tables_index_path: &Path, + global_roms_path: Option<&Path>, progress: &impl Progress, force_reindex: Vec, ) -> Result { let global_roms = global_roms_path .map(find_roms) .unwrap_or_else(|| Ok(HashMap::new()))?; - println!("Indexing {}", tables_folder.as_ref().display()); + println!("Indexing {}", tables_folder.display()); - if !tables_folder.as_ref().exists() { - return Err(IndexError::FolderDoesNotExist( - tables_folder.as_ref().to_path_buf(), - )); + if !tables_folder.exists() { + return Err(IndexError::FolderDoesNotExist(tables_folder.to_path_buf())); } - let existing_index = read_index_json(&tables_index_path)?; + let existing_index = read_index_json(tables_index_path)?; if let Some(index) = &existing_index { println!( " Found existing index with {} tables at {}", index.tables.len(), - tables_index_path.as_ref().display() + tables_index_path.display() ); } let mut index = existing_index.unwrap_or(TablesIndex::empty()); @@ -549,23 +542,20 @@ fn consider_sidecar_vbs(path: &Path, game_data: GameData) -> io::Result Ok(code) } -fn last_modified>(path: P) -> io::Result { - let metadata: Metadata = path.as_ref().metadata()?; +fn last_modified(path: &Path) -> io::Result { + let metadata: Metadata = path.metadata()?; metadata.modified() } -pub fn write_index_json>( - indexed_tables: &TablesIndex, - json_path: P, -) -> io::Result<()> { +pub fn write_index_json(indexed_tables: &TablesIndex, json_path: &Path) -> io::Result<()> { let json_file = File::create(json_path)?; let indexed_tables_json: TablesIndexJson = indexed_tables.into(); serde_json::to_writer_pretty(json_file, &indexed_tables_json) .map_err(|e| io::Error::new(io::ErrorKind::Other, e)) } -pub fn read_index_json>(json_path: P) -> io::Result> { - if !json_path.as_ref().exists() { +pub fn read_index_json(json_path: &Path) -> io::Result> { + if !json_path.exists() { return Ok(None); } let json_file = File::open(json_path)?; @@ -819,7 +809,7 @@ mod tests { #[test] fn test_read_index_missing() -> io::Result<()> { let index_path = PathBuf::from("missing_index_file.json"); - let read = read_index_json(index_path)?; + let read = read_index_json(&index_path)?; assert_eq!(read, None); Ok(()) } diff --git a/src/lib.rs b/src/lib.rs index b947fae..aef4655 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -173,7 +173,7 @@ fn handle_command(matches: ArgMatches) -> io::Result { let path = sub_matches.get_one::("VPXPATH").map(|s| s.as_str()); let path = path.unwrap_or(""); let expanded_path = expand_path_exists(path)?; - match script_diff(expanded_path) { + match script_diff(&expanded_path) { Ok(output) => { println!("{}", output)?; Ok(ExitCode::SUCCESS) @@ -223,7 +223,7 @@ fn handle_command(matches: ArgMatches) -> io::Result { Some((CMD_SIMPLE_FRONTEND, _sub_matches)) => { let (config_path, config) = config::load_or_setup_config()?; println!("Using config file {}", config_path.display())?; - let roms = indexer::find_roms(config.global_pinmame_rom_folder())?; + let roms = indexer::find_roms(&config.global_pinmame_rom_folder())?; if roms.is_empty() { let warning = format!( "No roms found in {}", @@ -379,7 +379,7 @@ fn handle_command(matches: ArgMatches) -> io::Result { .unwrap_or_default(); let expanded_path = expand_path_exists(path)?; - let diff = script_diff(expanded_path)?; + let diff = script_diff(&expanded_path)?; println!("{}", diff)?; Ok(ExitCode::SUCCESS) } @@ -662,7 +662,7 @@ fn handle_command(matches: ArgMatches) -> io::Result { Some(config_path) => { let loaded_config = config::load_config()?; let config = loaded_config.as_ref().map(|c| &c.1); - open_editor(config_path, config)?; + open_editor(&config_path, config)?; Ok(ExitCode::SUCCESS) } None => fail("No config file found"), @@ -1095,7 +1095,7 @@ fn handle_extractvbs(sub_matches: &ArgMatches) -> io::Result { let directory = sub_matches .get_one::("OUTPUT_DIRECTORY") .map(expand_path); - let expanded_vpx_path = path_exists(vpx_path.expect("should be checked by clap"))?; + let expanded_vpx_path = path_exists(&vpx_path.expect("should be checked by clap"))?; if vbs_path.is_some() && directory.is_some() { return fail("Conflicting VBSPATH and DIRECTORY options, only one can be used"); } @@ -1127,7 +1127,7 @@ fn handle_extractvbs(sub_matches: &ArgMatches) -> io::Result { } fn extract_directb2s(expanded_path: &PathBuf) -> io::Result<()> { - let file = File::open(expanded_path).unwrap(); + let file = File::open(expanded_path)?; let reader = BufReader::new(file); match read(reader) { Ok(b2s) => { @@ -1280,25 +1280,25 @@ fn expand_path>(path: S) -> PathBuf { fn expand_path_exists>(path: S) -> io::Result { // TODO expand all instead of only tilde? let expanded_path = shellexpand::tilde(path.as_ref()); - path_exists(PathBuf::from(expanded_path.to_string())) + path_exists(&PathBuf::from(expanded_path.to_string())) } -fn path_exists>(expanded_path: P) -> io::Result { - match metadata(expanded_path.as_ref()) { +fn path_exists(expanded_path: &Path) -> io::Result { + match metadata(expanded_path) { Ok(md) => { if !md.is_file() && !md.is_dir() && md.is_symlink() { Err(io::Error::new( io::ErrorKind::InvalidInput, - format!("{} is not a file", expanded_path.as_ref().display()), + format!("{} is not a file", expanded_path.display()), )) } else { - Ok(expanded_path.as_ref().to_path_buf()) + Ok(expanded_path.to_path_buf()) } } Err(msg) => { let warning = format!( "Failed to read metadata for {}: {}", - expanded_path.as_ref().display(), + expanded_path.display(), msg ); Err(io::Error::new(io::ErrorKind::InvalidInput, warning)) @@ -1387,7 +1387,7 @@ fn info_gather(vpx_file_path: &PathBuf) -> io::Result { Ok(buffer) } -fn info_extract(vpx_file_path: &PathBuf) -> io::Result { +fn info_extract(vpx_file_path: &Path) -> io::Result { let info_file_path = vpx_file_path.with_extension("info.json"); if info_file_path.exists() { let confirmed = confirm( @@ -1404,7 +1404,7 @@ fn info_extract(vpx_file_path: &PathBuf) -> io::Result { Ok(ExitCode::SUCCESS) } -fn write_info_json(vpx_file_path: &PathBuf, info_file_path: &PathBuf) -> io::Result<()> { +fn write_info_json(vpx_file_path: &Path, info_file_path: &Path) -> io::Result<()> { let mut vpx_file = vpx::open(vpx_file_path)?; let table_info = vpx_file.read_tableinfo()?; let custom_info_tags = vpx_file.read_custominfotags()?; @@ -1414,7 +1414,7 @@ fn write_info_json(vpx_file_path: &PathBuf, info_file_path: &PathBuf) -> io::Res Ok(()) } -fn info_edit(vpx_file_path: &PathBuf, config: Option<&ResolvedConfig>) -> io::Result { +fn info_edit(vpx_file_path: &Path, config: Option<&ResolvedConfig>) -> io::Result { let info_file_path = vpx_file_path.with_extension("info.json"); if !info_file_path.exists() { write_info_json(vpx_file_path, &info_file_path)?; @@ -1423,16 +1423,16 @@ fn info_edit(vpx_file_path: &PathBuf, config: Option<&ResolvedConfig>) -> io::Re Ok(info_file_path) } -fn open_editor>(file_to_edit: P, config: Option<&ResolvedConfig>) -> io::Result<()> { +fn open_editor(file_to_edit: &Path, config: Option<&ResolvedConfig>) -> io::Result<()> { match config.iter().flat_map(|c| c.editor.clone()).next() { - Some(editor) => open_configured_editor(&file_to_edit, &editor), + Some(editor) => open_configured_editor(file_to_edit, &editor), None => edit::edit_file(file_to_edit), } } -fn open_configured_editor>(file_to_edit: &P, editor: &String) -> io::Result<()> { +fn open_configured_editor(file_to_edit: &Path, editor: &String) -> io::Result<()> { let mut command = std::process::Command::new(editor); - command.arg(file_to_edit.as_ref()); + command.arg(file_to_edit); command.stdout(std::process::Stdio::inherit()); command.stderr(std::process::Stdio::inherit()); match command.status() { @@ -1529,21 +1529,19 @@ pub fn extract(vpx_file_path: &Path, yes: bool) -> io::Result { } } -pub fn info_diff>(vpx_file_path: P) -> io::Result { - let expanded_vpx_path = expand_path_exists(vpx_file_path.as_ref().to_str().unwrap())?; +pub fn info_diff(vpx_file_path: &Path) -> io::Result { + let expanded_vpx_path = expand_path_exists(vpx_file_path.to_str().unwrap())?; let info_file_path = expanded_vpx_path.with_extension("info.json"); - let original_info_path = vpx_file_path.as_ref().with_extension("info.original.tmp"); if info_file_path.exists() { - write_info_json(&expanded_vpx_path, &original_info_path)?; + let original_info_path = + RemoveOnDrop::new(vpx_file_path.with_extension("info.original.tmp")); + write_info_json(&expanded_vpx_path, original_info_path.path())?; let diff_color = if colored::control::SHOULD_COLORIZE.should_colorize() { DiffColor::Always } else { DiffColor::Never }; - let output = run_diff(&original_info_path, &info_file_path, diff_color)?; - if original_info_path.exists() { - std::fs::remove_file(original_info_path)?; - } + let output = run_diff(original_info_path.path(), &info_file_path, diff_color)?; Ok(String::from_utf8_lossy(&output).to_string()) } else { let msg = format!("No sidecar info file found: {}", info_file_path.display()); @@ -1551,35 +1549,27 @@ pub fn info_diff>(vpx_file_path: P) -> io::Result { } } -pub fn script_diff>(vpx_file_path: P) -> io::Result { +pub fn script_diff(vpx_file_path: &Path) -> io::Result { // set extension for PathBuf - let vbs_path = vpx_file_path.as_ref().with_extension("vbs"); - let original_vbs_path = vpx_file_path.as_ref().with_extension("vbs.original.tmp"); - + let vbs_path = vpx_file_path.with_extension("vbs"); if vbs_path.exists() { - match vpx::open(&vpx_file_path) { + match vpx::open(vpx_file_path) { Ok(mut vpx_file) => { let gamedata = vpx_file.read_gamedata()?; let script = gamedata.code; - std::fs::write(&original_vbs_path, script.string)?; + let original_vbs_path = + RemoveOnDrop::new(vpx_file_path.with_extension("vbs.original.tmp")); + std::fs::write(original_vbs_path.path(), script.string)?; let diff_color = if colored::control::SHOULD_COLORIZE.should_colorize() { DiffColor::Always } else { DiffColor::Never }; - let output = run_diff(&original_vbs_path, &vbs_path, diff_color)?; - - if original_vbs_path.exists() { - std::fs::remove_file(original_vbs_path)?; - } + let output = run_diff(original_vbs_path.path(), &vbs_path, diff_color)?; Ok(String::from_utf8_lossy(&output).to_string()) } Err(e) => { - let msg = format!( - "Not a valid vpx file {}: {}", - vpx_file_path.as_ref().display(), - e - ); + let msg = format!("Not a valid vpx file {}: {}", vpx_file_path.display(), e); Err(io::Error::new(io::ErrorKind::InvalidData, msg)) } } @@ -1590,6 +1580,29 @@ pub fn script_diff>(vpx_file_path: P) -> io::Result { } } +/// Path to file that will be removed when it goes out of scope +struct RemoveOnDrop { + path: PathBuf, +} +impl RemoveOnDrop { + fn new(path: PathBuf) -> Self { + RemoveOnDrop { path } + } + + fn path(&self) -> &Path { + &self.path + } +} + +impl Drop for RemoveOnDrop { + fn drop(&mut self) { + if self.path.exists() { + // silently ignore any errors + let _ = std::fs::remove_file(&self.path); + } + } +} + pub enum DiffColor { Always, Never, @@ -1618,7 +1631,7 @@ pub fn run_diff( .file_name() .unwrap_or(original_vbs_path.as_os_str()); let vbs_filename = vbs_path.file_name().unwrap_or(vbs_path.as_os_str()); - std::process::Command::new("diff") + let result = std::process::Command::new("diff") .current_dir(parent) .arg("-u") .arg("-w") @@ -1626,7 +1639,14 @@ pub fn run_diff( .arg(original_vbs_filename) .arg(vbs_filename) .output() - .map(|o| o.stdout) + .map(|o| o.stdout); + result.map_err(|e| { + let msg = format!( + "Failed to run 'diff'. Is it installed on your system? {}", + e + ); + io::Error::new(io::ErrorKind::Other, msg) + }) } fn show_dip_switches(nvram: &PathBuf) -> io::Result {