Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed mv subcommand for directories with no filenames #15

Merged
merged 2 commits into from
Feb 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ version = "0.8.7"
edition = "2021"
exclude = ["logo.png", "tests/test_data/**"]
license = "MIT"
authors = ["Vince Buffalo <[email protected]>"]
keywords = ["science", "reproducibility", "bioinformatics", "data"]
categories = ["command-line-utilities", "science"]
repository = "https://github.com/vsbuffalo/scidataflow"
Expand Down
45 changes: 29 additions & 16 deletions src/lib/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ use crate::lib::utils::{load_file, pluralize, print_status};
#[allow(unused_imports)]
use crate::{print_info, print_warn};

use super::utils::is_directory;

const MANIFEST: &str = "data_manifest.yml";

pub fn find_manifest(start_dir: Option<&PathBuf>, filename: &str) -> Option<PathBuf> {
Expand Down Expand Up @@ -466,10 +468,21 @@ impl Project {
// has been successfully moved. So the updating is all done on the DataFile
// directly, since lower interfaces cannot access the relative path.
pub async fn mv(&mut self, source: &str, destination: &str) -> Result<()> {
let source_path = self.relative_path_string(Path::new(source))?;
if let Some(file) = self.data.files.remove(&source_path) {
let source_path = Path::new(source);
let source_path_str = self.relative_path_string(source_path)?;
if let Some(file) = self.data.files.remove(&source_path_str) {
let mut destination_path = PathBuf::from(destination);

if is_directory(&destination_path) {
// if destination is a directory, append the file name from
// the source path to mimic unix mv
if let Some(file_name) = source_path.file_name() {
destination_path = destination_path.join(file_name);
}
}

// move the actual file
rename(source, destination).context("Error encountered when moving file.")?;
rename(source, destination_path).context("Error encountered when moving file.")?;

// update the relative path
let relative_destination = self.relative_path_string(Path::new(destination))?;
Expand All @@ -484,9 +497,9 @@ impl Project {
self.save()
} else {
Err(anyhow!(
"Cannot move file '{}' with 'sdf mv' since it is not in the manifest.",
source
))
"Cannot move file '{}' with 'sdf mv' since it is not in the manifest.",
source
))
}
}

Expand Down Expand Up @@ -516,17 +529,17 @@ impl Project {
} else {
println!(
"File '{}' already existed in \
the manifest, so it was not added.",
the manifest, so it was not added.",
&filepath
);
);
}
Ok(())
} else {
Err(anyhow!(
"The file at '{}' was not downloaded because it would overwrite a file.\n\
Use 'sdf get <URL> --ovewrite' to overwrite it.",
url
))
"The file at '{}' was not downloaded because it would overwrite a file.\n\
Use 'sdf get <URL> --ovewrite' to overwrite it.",
url
))
}
}

Expand All @@ -536,7 +549,7 @@ impl Project {
column: Option<u64>,
header: bool,
overwrite: bool,
) -> Result<()> {
) -> Result<()> {
let extension = std::path::Path::new(filename)
.extension()
.and_then(std::ffi::OsStr::to_str);
Expand Down Expand Up @@ -596,15 +609,15 @@ impl Project {
let num_skipped = skipped.len();
println!(
"{} URLs found in '{}.'\n\
{} files were downloaded, {} added to manifest ({} were already registered).\n\
{} files were skipped because they existed (and --overwrite was no specified).",
{} files were downloaded, {} added to manifest ({} were already registered).\n\
{} files were skipped because they existed (and --overwrite was no specified).",
num_lines,
filename,
urls.len(),
num_added,
num_already_registered,
num_skipped
);
);
self.save()?;
Ok(())
}
Expand Down
7 changes: 7 additions & 0 deletions src/lib/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use md5::Context;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::fs;
use std::fs::File;
use std::io::Read;
use std::path::{Path, PathBuf};
Expand Down Expand Up @@ -36,6 +37,12 @@
}
}

pub fn is_directory(path: &Path) -> bool {
fs::metadata(path)
.map(|metadata| metadata.is_dir())
.unwrap_or(false)
}

pub fn ensure_exists(path: &Path) -> Result<()> {
if path.exists() {
Ok(())
Expand Down Expand Up @@ -355,7 +362,7 @@
format!("{} ({})", timestamp, formatter.convert(std_duration))
}

pub fn shorten(hash: &String, abbrev: Option<i32>) -> String {

Check failure on line 365 in src/lib/utils.rs

View workflow job for this annotation

GitHub Actions / Clippy

writing `&String` instead of `&str` involves a new object where a slice will do

Check failure on line 365 in src/lib/utils.rs

View workflow job for this annotation

GitHub Actions / Clippy

writing `&String` instead of `&str` involves a new object where a slice will do
let n = abbrev.unwrap_or(hash.len() as i32) as usize;
hash.chars().take(n).collect()
}
Expand Down
Loading