Skip to content

Commit

Permalink
Overhaul progress bars and messaging for entire CLI, enforce consistency
Browse files Browse the repository at this point in the history
  • Loading branch information
filiptibell committed Mar 25, 2024
1 parent 7adda67 commit 5a2770e
Show file tree
Hide file tree
Showing 9 changed files with 228 additions and 84 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ async-signal = "0.2"
async-once-cell = "0.5"
clap = { version = "4.5", features = ["derive"] }
const_format = "0.2"
console = "0.15"
dashmap = { version = "5.5", features = ["serde"] }
dialoguer = "0.11"
dirs = "5.0"
Expand Down
37 changes: 25 additions & 12 deletions src/cli/add.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use anyhow::{bail, Context, Result};
use clap::Parser;
use console::style;

use aftman::{
description::Description,
Expand All @@ -9,8 +10,8 @@ use aftman::{
};

use crate::util::{
discover_aftman_manifest_dir, github_tool_source, new_progress_bar, prompt_for_install_trust,
ToolIdOrSpec,
discover_aftman_manifest_dir, finish_progress_bar, github_tool_source, new_progress_bar,
prompt_for_trust, ToolIdOrSpec,
};

/// Adds a new tool to Aftman and installs it.
Expand Down Expand Up @@ -42,15 +43,15 @@ impl AddSubcommand {
let tool_cache = home.tool_cache();
let tool_storage = home.tool_storage();

// Check for trust, or prompt the user to trust the tool
// 1. Check for trust, or prompt the user to trust the tool
if !tool_cache.is_trusted(&id) {
if !self.force && !prompt_for_install_trust(&id).await? {
bail!("Tool is not trusted - installation was aborted");
if !self.force && !prompt_for_trust(id.clone()).await? {
bail!("Tool is not trusted - operation was aborted");
}
tool_cache.add_trust(id.clone());
}

// Load tool source, manifest, and do a preflight check
// 2. Load tool source, manifest, and do a preflight check
// to make sure we don't overwrite any existing tool(s)
let source = github_tool_source(home).await?;
let manifest_path = if self.global {
Expand All @@ -73,9 +74,9 @@ impl AddSubcommand {
);
}

// If we only got an id without a specified version, we
// 3. If we only got an id without a specified version, we
// will fetch the latest non-prerelease release and use that
let pb = new_progress_bar("Fetching", 5);
let pb = new_progress_bar("Fetching", 5, 1);
let spec = match self.tool.clone() {
ToolIdOrSpec::Spec(spec) => {
pb.inc(1);
Expand All @@ -91,11 +92,11 @@ impl AddSubcommand {
}
};

// Add the tool spec to the desired manifest file and save it
// 4. Add the tool spec to the desired manifest file and save it
manifest.add_tool(&alias, &spec);
manifest.save(manifest_path).await?;

// Install the tool and create the link for its alias
// 5. Download and install the tool
let description = Description::current();
if !tool_cache.is_installed(&spec) || self.force {
pb.set_message("Downloading");
Expand Down Expand Up @@ -129,11 +130,23 @@ impl AddSubcommand {
pb.inc(4);
}

// 6. Create the tool alias link
pb.set_message("Linking");
tool_storage.create_tool_link(&alias).await?;
pb.finish_and_clear();

tracing::info!("Added tool successfully: {spec}");
// 7. Finally, display a nice message to the user
let msg = format!(
"Added version {} of tool {}{} {}",
style(spec.version()).bold().yellow(),
style(spec.name()).bold().magenta(),
if alias.name() != id.name() {
format!(" with alias {}", style(alias.to_string()).bold().cyan())
} else {
"".into()
},
style(format!("(took {:.2?})", pb.elapsed())).dim(),
);
finish_progress_bar(pb, msg);

Ok(())
}
Expand Down
70 changes: 39 additions & 31 deletions src/cli/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,12 @@ use anyhow::{Context, Result};
use clap::Parser;

use aftman::{description::Description, manifests::AftmanManifest, storage::Home};
use console::style;
use futures::{stream::FuturesUnordered, TryStreamExt};
use tokio::time::Instant;

use crate::util::{
discover_aftman_manifest_dirs, github_tool_source, new_progress_bar, prompt_for_install_trust,
discover_aftman_manifest_dirs, finish_progress_bar, github_tool_source, new_progress_bar,
prompt_for_trust_specs,
};

/// Adds a new tool to Aftman and installs it.
Expand All @@ -26,7 +27,6 @@ pub struct InstallSubcommand {
impl InstallSubcommand {
pub async fn run(&self, home: &Home) -> Result<()> {
let force = self.force;
let start = Instant::now();
let (manifest_paths, source) = tokio::try_join!(
discover_aftman_manifest_dirs(home),
github_tool_source(home)
Expand All @@ -53,24 +53,6 @@ impl InstallSubcommand {

// 2. Check for trust

let tools = if self.no_trust_check {
tools
} else {
let mut trusted_specs = Vec::new();
for (tool_alias, tool_spec) in tools {
let tool_id = tool_spec.clone().into_id();
if !tool_cache.is_trusted(&tool_id) {
if prompt_for_install_trust(&tool_id).await? {
tool_cache.add_trust(tool_id);
trusted_specs.push((tool_alias, tool_spec));
}
} else {
trusted_specs.push((tool_alias, tool_spec));
}
}
trusted_specs
};

// NOTE: Deduplicate tool aliases and specs since they may appear in several manifests
let tool_aliases = tools
.iter()
Expand All @@ -81,14 +63,31 @@ impl InstallSubcommand {
.map(|(_, spec)| spec)
.collect::<BTreeSet<_>>();

let tool_specs = if self.no_trust_check {
tool_specs
} else {
let (trusted_specs, untrusted_specs) = tool_specs
.into_iter()
.partition(|spec| tool_cache.is_trusted(&spec.clone().into_id()));
let newly_trusted_specs = prompt_for_trust_specs(untrusted_specs).await?;
for spec in &newly_trusted_specs {
tool_cache.add_trust(spec.clone().into_id());
}
trusted_specs
.iter()
.chain(newly_trusted_specs.iter())
.cloned()
.collect::<BTreeSet<_>>()
};

// 3. Find artifacts, download and install them

let pb = new_progress_bar("Installing", tool_specs.len());
let pb = new_progress_bar("Installing", tool_specs.len(), 5);
let artifacts = tool_specs
.into_iter()
.map(|tool_spec| async {
if tool_cache.is_installed(&tool_spec) && !force {
pb.inc(1);
pb.inc(5);
// HACK: Force the async closure to take ownership
// of tool_spec by returning it from the closure
return anyhow::Ok(tool_spec);
Expand All @@ -98,51 +97,60 @@ impl InstallSubcommand {
.find_release(&tool_spec)
.await?
.with_context(|| format!("Failed to find release for {tool_spec}"))?;
pb.inc(1);

let artifact = source
.find_compatible_artifacts(&tool_spec, &release, &description)
.first()
.cloned()
.with_context(|| format!("No compatible artifact found for {tool_spec}"))?;
pb.inc(1);

let contents = source
.download_artifact_contents(&artifact)
.await
.with_context(|| format!("Failed to download contents for {tool_spec}"))?;
pb.inc(1);

let extracted = artifact
.extract_contents(contents)
.await
.with_context(|| format!("Failed to extract contents for {tool_spec}"))?;
pb.inc(1);

tool_storage
.replace_tool_contents(&tool_spec, extracted)
.await?;

tool_cache.add_installed(tool_spec.clone());
pb.inc(1);

tool_cache.add_installed(tool_spec.clone());
Ok(tool_spec)
})
.collect::<FuturesUnordered<_>>()
.try_collect::<Vec<_>>()
.await?;
pb.finish_and_clear();

// 4. Link all of the (possibly new) aliases, we do this even if the
// tool is already installed in case the link(s) have been corrupted
// and the user tries to re-install tools to fix it.

pb.set_message("Linking");
tool_aliases
.iter()
.map(|alias| tool_storage.create_tool_link(alias))
.collect::<FuturesUnordered<_>>()
.try_collect::<Vec<_>>()
.await?;

tracing::info!(
"Completed in {:.2?} ({} tools, {} links)",
start.elapsed(),
artifacts.len(),
tool_aliases.len(),
// 5. Finally, display a nice message to the user
let msg = format!(
"Installed and created link{} for {} tool{} {}",
if artifacts.len() == 1 { "" } else { "s" },
style(artifacts.len()).bold().magenta(),
if artifacts.len() == 1 { "" } else { "s" },
style(format!("(took {:.2?})", pb.elapsed())).dim(),
);
finish_progress_bar(pb, msg);

Ok(())
}
Expand Down
13 changes: 11 additions & 2 deletions src/cli/self_install.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
use anyhow::{Context, Result};
use clap::Parser;
use console::style;

use aftman::storage::Home;

use crate::util::{finish_progress_bar, new_progress_bar};

/// Installs / re-installs Aftman, and updates all tool links.
#[derive(Debug, Parser)]
pub struct SelfInstallSubcommand {}
Expand All @@ -11,6 +14,8 @@ impl SelfInstallSubcommand {
pub async fn run(&self, home: &Home) -> Result<()> {
let storage = home.tool_storage();

let pb = new_progress_bar("Linking", 1, 1);

let (had_aftman_installed, was_aftman_updated) =
storage.recreate_all_links().await.context(
"Failed to recreate tool links!\
Expand All @@ -20,7 +25,7 @@ impl SelfInstallSubcommand {
// TODO: Automatically populate the PATH variable
let path_was_populated = false;
let path_message_lines = if !path_was_populated {
"\nBinaries for Aftman and tools have been added to your PATH.\
"\n\nBinaries for Aftman and tools have been added to your PATH.\
\nPlease restart your terminal for the changes to take effect."
} else {
""
Expand All @@ -34,7 +39,11 @@ impl SelfInstallSubcommand {
"Aftman is already up-to-date."
};

tracing::info!("{main_message}{path_message_lines}");
let msg = format!(
"{main_message} {}{path_message_lines}",
style(format!("(took {:.2?})", pb.elapsed())).dim(),
);
finish_progress_bar(pb, msg);

Ok(())
}
Expand Down
41 changes: 29 additions & 12 deletions src/cli/trust.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use anyhow::{bail, Result};
use clap::Parser;
use tracing::info;
use console::style;

use aftman::{storage::Home, tool::ToolId};

use crate::util::{finish_progress_bar, new_progress_bar};

/// Mark the given tool(s) as being trusted.
#[derive(Debug, Parser)]
pub struct TrustSubcommand {
Expand All @@ -17,36 +19,51 @@ impl TrustSubcommand {
bail!("Please provide at least one tool to trust.");
}

// NOTE: We use a progress bar only to show the final message to the
// user below, to maintain consistent formatting with other commands.
let pb = new_progress_bar("Trusting", 1, 1);

let cache = home.tool_cache();
let (added_tools, existing_tools) = self
.tools
.into_iter()
.partition::<Vec<_>, _>(|tool| cache.add_trust(tool.clone()));
let took = style(format!("(took {:.2?})", pb.elapsed())).dim();

if added_tools.len() == 1 && existing_tools.is_empty() {
info!("Tool has been marked as trusted: {}", added_tools[0]);
// Special case 1 with shorter output - a singular tool was added
let msg = format!("Tool {} is now trusted {took}", added_tools[0]);
finish_progress_bar(pb, msg);
} else if existing_tools.len() == 1 && added_tools.is_empty() {
info!("Tool was already trusted: {}", existing_tools[0]);
// Special case 2 with shorter output - a singular tool was already trusted
let msg = format!("Tool {} was already trusted {took}", existing_tools[0]);
finish_progress_bar(pb, msg);
} else {
// General case with multiple tools added and/or already trusted
let mut lines = Vec::new();
let list_item = style("•").dim();

if !added_tools.is_empty() {
lines.push(String::from(
"The following tools have been marked as trusted:",
));
for tool in added_tools {
lines.push(format!(" - {tool}"));
lines.push(String::from("These tools are now trusted:"));
for tool in &added_tools {
lines.push(format!(" {list_item} {tool}"));
}
}

if !existing_tools.is_empty() {
lines.push(String::from("The following tools were already trusted:"));
for tool in existing_tools {
lines.push(format!(" - {tool}"));
lines.push(String::from("These tools were already trusted:"));
for tool in &existing_tools {
lines.push(format!(" {list_item} {tool}"));
}
}

info!("{}", lines.join("\n"));
let msg = format!(
"Changed trust for {} tool{} {took}\n\n{}",
added_tools.len(),
if added_tools.len() == 1 { "" } else { "s" },
lines.join("\n")
);
finish_progress_bar(pb, msg);
}

Ok(())
Expand Down
4 changes: 2 additions & 2 deletions src/util/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ pub use self::discovery::{
discover_aftman_manifest_dir, discover_aftman_manifest_dirs, discover_closest_tool_spec,
};
pub use self::id_or_spec::ToolIdOrSpec;
pub use self::progress::new_progress_bar;
pub use self::prompts::prompt_for_install_trust;
pub use self::progress::{finish_progress_bar, new_progress_bar};
pub use self::prompts::{prompt_for_trust, prompt_for_trust_specs};
pub use self::sources::github_tool_source;
pub use self::tracing::init as init_tracing;
Loading

0 comments on commit 5a2770e

Please sign in to comment.