✨ Use cached template when it is up-to-date #11
Conversation
src/errors.rs
Outdated
| #[cfg(feature = "fetch-template")] | ||
| #[error(transparent)] | ||
| #[diagnostic(code(cargo_v5::bad_response))] | ||
| BadResponse(#[from] reqwest::Error), |
There was a problem hiding this comment.
This should probably be renamed to ReqwestError
src/main.rs
Outdated
| /// Whether or not to fetch the latest template online. | ||
| #[cfg_attr(feature = "fetch-template", arg(long, default_value = "true"))] | ||
| #[cfg_attr(not(feature = "fetch-template"), arg(skip = true))] | ||
| use_internet: bool, | ||
| }, | ||
| /// Creates a new vexide project in the current directory | ||
| Init, | ||
| Init { | ||
| /// Whether or not to fetch the latest template online. | ||
| #[cfg_attr(feature = "fetch-template", arg(long, default_value = "true"))] | ||
| #[cfg_attr(not(feature = "fetch-template"), arg(skip = true))] | ||
| use_internet: bool, |
There was a problem hiding this comment.
It might be worth considering separating this argument into its own struct and parsing it flattened because it's a significant amount of code duplication. Also I'm not entirely sure about its name.
src/commands/new.rs
Outdated
|
|
||
| #[cfg(feature = "fetch-template")] | ||
| let template = template.unwrap_or_else(|| { | ||
| warn!("No emplate found in cache, using builtin template."); |
There was a problem hiding this comment.
Typo in template and I'm not sure this should be a warning.
src/commands/new.rs
Outdated
| @@ -55,7 +97,7 @@ fn unpack_template(template: Vec<u8>, dir: &Utf8PathBuf) -> io::Result<()> { | |||
| Ok(()) | |||
| } | |||
|
|
|||
| pub async fn new(path: Utf8PathBuf, name: Option<String>) -> Result<(), CliError> { | |||
| pub async fn new(path: Utf8PathBuf, name: Option<String>, use_internet: bool) -> Result<(), CliError> { | |||
There was a problem hiding this comment.
I think that fetch_template or similar is a better name than use_internet because it clearly shows what the internet is being used for.
| let response_text = response.text().await.ok().unwrap_or("{}".to_string()); | ||
| match &serde_json::from_str::<Value>(&response_text).unwrap_or_default()["sha"] { | ||
| Value::String(str) => Ok(str.clone()), | ||
| _ => unreachable!("Internal error: GitHub API broken"), |
There was a problem hiding this comment.
I don't think this should be unreachable or a panic - as unlikely as it is I don't entirely trust serde/Github to be totally reliable here as its something we don't have control over.
There was a problem hiding this comment.
Agreed. Unreachable is completely incorrect here anyway.
| .ok() | ||
| .map(|path| path.with_file_name("vexide-template.tar.gz")) | ||
| }) | ||
| fn get_cached_template() -> Option<Template> { |
There was a problem hiding this comment.
These functions should probably use the tokio version of fs::write and be made async
| /// Do not download the latest template online. | ||
| #[cfg_attr(feature = "fetch-template", arg(long, default_value = "false"))] | ||
| #[cfg_attr(not(feature = "fetch-template"), arg(skip = false))] | ||
| no_download_template: bool, |
There was a problem hiding this comment.
This could be named something a little more concise, maybe --offline?
No description provided.