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.

Sign up for GitHub

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

CLI Tool: Allow using --file - to read metadata from stdin #1336

Merged
merged 4 commits into from
Jan 3, 2024
Merged

CLI Tool: Allow using --file - to read metadata from stdin #1336

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b15911d
allow using --file - to read from stdin
tadeohepperle Jan 3, 2024
6566ffd
add test for path and url parsing
tadeohepperle Jan 3, 2024
7b57d96
Merge branch 'master' into tadeohepperle/cli-accept-input-via-std-in
tadeohepperle Jan 3, 2024
fa9b1a3
Merge branch 'master' into tadeohepperle/cli-accept-input-via-std-in
jsdw Jan 3, 2024
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
43 changes: 39 additions & 4 deletions cli/src/utils.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ pub struct FileOrUrl {
pub url: Option<Url>,
/// The path to the encoded metadata file.
#[clap(long, value_parser)]
pub file: Option<PathBuf>,
pub file: Option<PathOrStdIn>,
/// Specify the metadata version.
///
/// - "latest": Use the latest stable version available.
Expand All @@ -33,6 +33,26 @@ pub struct FileOrUrl {
pub version: Option<MetadataVersion>,
}

/// if `--path -` is provided, read bytes for metadata from stdin
const STDIN_PATH_NAME: &str = "-";
#[derive(Debug, Clone)]
pub enum PathOrStdIn {
Path(PathBuf),
StdIn,
}

impl FromStr for PathOrStdIn {
type Err = <PathBuf as FromStr>::Err;

fn from_str(s: &str) -> Result<Self, Self::Err> {
Copy link
Member

@niklasad1 niklasad1 Jan 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether we should trim the leading and trailing whitespaces before checking that the --file -

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and a simple test for the from_str impl would be nice to have.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think regarding trimming, clap already takes care of that before the args are given to us to parse, but I am not entirely sure.

Copy link
Collaborator

@jsdw jsdw Jan 3, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

worth a quick check but I'd assume the shell would handle it :)

(and if the user provides something like ' - ' as an argument then I wouldn't expect that to be treated as stdin anyways. Edit: Ah I can see it's trimmed now; that's fine too; either way works!)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tadeo added trimming and tests for it :)

if s == STDIN_PATH_NAME {
Ok(PathOrStdIn::StdIn)
} else {
PathBuf::from_str(s).map(PathOrStdIn::Path)
}
}
}

impl FileOrUrl {
/// Fetch the metadata bytes.
pub async fn fetch(&self) -> color_eyre::Result<Vec<u8>> {
Expand All @@ -42,12 +62,20 @@ impl FileOrUrl {
eyre::bail!("specify one of `--url` or `--file` but not both")
}
// Load from --file path
(Some(path), None, None) => {
(Some(PathOrStdIn::Path(path)), None, None) => {
let mut file = fs::File::open(path)?;
let mut bytes = Vec::new();
file.read_to_end(&mut bytes)?;
Ok(bytes)
}
(Some(PathOrStdIn::StdIn), None, None) => {
let res = std::io::stdin().bytes().collect::<Result<Vec<u8>, _>>();

match res {
Ok(bytes) => Ok(bytes),
Err(err) => eyre::bail!("reading bytes from stdin (`--file -`) failed: {err}"),
}
}
// Cannot load the metadata from the file and specify a version to fetch.
(Some(_), None, Some(_)) => {
// Note: we could provide the ability to convert between metadata versions
Expand Down Expand Up @@ -92,16 +120,23 @@ impl FromStr for FileOrUrl {
type Err = &'static str;

fn from_str(s: &str) -> Result<Self, Self::Err> {
if s == STDIN_PATH_NAME {
return Ok(FileOrUrl {
url: None,
file: Some(PathOrStdIn::StdIn),
version: None,
});
}
let path = std::path::Path::new(s);
if path.exists() {
Ok(FileOrUrl {
url: None,
file: Some(PathBuf::from(s)),
file: Some(PathOrStdIn::Path(PathBuf::from(s))),
version: None,
})
} else {
Url::parse(s)
.map_err(|_| "no path or uri could be crated")
.map_err(|_| "no path or uri could be created.")
.map(|uri| FileOrUrl {
url: Some(uri),
file: None,
Expand Down
Loading