updates from PR review

crates/update-notifier/Cargo.toml

@@ -2,14 +2,12 @@
 name = "update-notifier"
 version = "0.1.0"
 edition = "2021"
-description = "Output update notification to the console when new version on npm package is available"
+description = "Display update notification when new version of npm package is available"
 license = "MPL-2.0"
 publish = false
 
-
 [dependencies]
 colored            = "2.0"
-env_logger         = "0.10"
 log                = "0.4"
 reqwest            = { version = "0.11", features = ["json"] }
 semver             = "1.0"

crates/update-notifier/src/fetch.rs

@@ -12,16 +12,16 @@ struct NpmVersionData {
 
 const REGISTRY_URL: &str = "https://registry.npmjs.org";
 const DEFAULT_TAG: &str = "latest";
-const TIMEOUT: Duration = Duration::from_millis(800);
+const DEFAULT_TIMEOUT: Duration = Duration::from_millis(800);
 
 pub async fn get_latest_version(
     package: &str,
     tag: Option<&str>,
     timeout: Option<Duration>,
 ) -> Result<String, UpdateNotifierError> {
     log::debug!("fetching latest version");
-    let tag = tag.unwrap_or_else(|| DEFAULT_TAG);
-    let timeout = timeout.unwrap_or_else(|| TIMEOUT);
+    let tag = tag.unwrap_or(DEFAULT_TAG);
+    let timeout = timeout.unwrap_or(DEFAULT_TIMEOUT);
     let client: Client = reqwest::Client::new();
     let url = format!("{}/{}/{}", REGISTRY_URL, package, tag);
     log::debug!("fetching {:?}", url);

crates/update-notifier/src/lib.rs

@@ -13,7 +13,7 @@ mod ui;
 mod utils;
 
 // default interval to check for new updates (one per day)
-const INTERVAL: Duration = Duration::from_secs(60 * 60 * 24);
+const DEFAULT_INTERVAL: Duration = Duration::from_secs(60 * 60 * 24);
 
 #[derive(Debug)]
 pub struct UpdateNotifier {
@@ -81,13 +81,14 @@ impl UpdateNotifier {
         let msg = format!(
             "
             Update available {} ≫ {}
-            Changelog: https://github.com/vercel/turbo/releases/tag/v{}Run \
-             \"{}\" to update
+            Changelog: {}/releases/tag/v{}
+            Run \"{}\" to update
 
             Follow {} for updates: {}
             ",
             &self.config.current_version.dimmed(),
             &latest_version.green().bold(),
+            "https://github.com/vercel/turbo",
             &latest_version,
             // TODO: make this package manager aware
             "npm i -g turbo".cyan().bold(),
@@ -106,7 +107,7 @@ impl UpdateNotifier {
                 latest_version: None,
                 last_checked: None,
             },
-            interval: interval.unwrap_or(INTERVAL),
+            interval: interval.unwrap_or(DEFAULT_INTERVAL),
             package,
             tag,
         }
@@ -121,7 +122,7 @@ impl UpdateNotifier {
             let file = match file {
                 Ok(f) => f,
                 Err(_) => {
-                    log::debug!("failed to open local config, writing first version");
+                    log::debug!("failed to read local config, writing first version");
                     return Self::first_run(package, tag, interval);
                 }
             };
@@ -138,7 +139,7 @@ impl UpdateNotifier {
                         },
                         package,
                         tag,
-                        interval: interval.unwrap_or(INTERVAL),
+                        interval: interval.unwrap_or(DEFAULT_INTERVAL),
                     }
                 }
                 Err(_) => {

crates/update-notifier/src/ui.rs

@@ -3,7 +3,8 @@ use terminal_size::{terminal_size, Width};
 
 mod utils;
 
-const PADDING: usize = 8;
+const DEFAULT_PADDING: usize = 8;
+
 const TOP_LEFT: &str = "╭";
 const TOP_RIGHT: &str = "╮";
 const BOTTOM_LEFT: &str = "╰";
@@ -42,20 +43,23 @@ fn x_border(width: usize, position: BorderAlignment) {
     }
 }
 
-pub fn rectangle(text: &str) -> Result<(), GetDisplayLengthError> {
+pub fn rectangle(text: &str) {
     let size = terminal_size();
     let lines: Vec<&str> = text.split("\n").map(|line| line.trim()).collect();
     // get the display width of each line so we can center it within the box later
     let lines_display_width: Vec<usize> = lines
         .iter()
         .map(|line| utils::get_display_length(line).unwrap())
         .collect();
-    let longest_line = lines_display_width.iter().max().copied().unwrap_or_default();
-    let full_message_width = longest_line + PADDING;
+    let longest_line = lines_display_width
+        .iter()
+        .max()
+        .copied()
+        .unwrap_or_default();
+    let full_message_width = longest_line + DEFAULT_PADDING;
 
     // handle smaller viewports
     if let Some((Width(term_width), _)) = size {
-        // we can't fit the box, so don't show it
         let term_width = usize::from(term_width) - 2;
         let cant_fit_box = full_message_width >= term_width;
 
@@ -64,7 +68,7 @@ pub fn rectangle(text: &str) -> Result<(), GetDisplayLengthError> {
             // top border
             x_border(term_width, BorderAlignment::Divider);
             for (line, line_display_width) in lines.iter().zip(lines_display_width.iter()) {
-                if line_display_width == 0 {
+                if *line_display_width == 0 {
                     println!("{}", SPACE.repeat(term_width));
                 } else {
                     println!("{}", line);
@@ -75,19 +79,18 @@ pub fn rectangle(text: &str) -> Result<(), GetDisplayLengthError> {
             return;
         }
 
-        // can't fit the box, but we can still center text
+        // can't fit the box, but we can still center the text
         if cant_fit_box {
             // top border
             x_border(term_width, BorderAlignment::Divider);
-            for (idx, line) in lines.iter().enumerate() {
-                let line_display_width = lines_display_width[idx];
-                if line_display_width == 0 {
+            for (line, line_display_width) in lines.iter().zip(lines_display_width.iter()) {
+                if *line_display_width == 0 {
                     println!("{}", SPACE.repeat(term_width));
                 } else {
                     let line_padding = (term_width - line_display_width) / 2;
                     // for lines of odd length, tack the reminder to the end
                     let line_padding_remainder =
-                        term_width - (line_padding * 2) - lines_display_width[idx];
+                        term_width - (line_padding * 2) - line_display_width;
                     println!(
                         "{}{}{}",
                         SPACE.repeat(line_padding),
@@ -104,19 +107,19 @@ pub fn rectangle(text: &str) -> Result<(), GetDisplayLengthError> {
 
     // full output
     x_border(full_message_width, BorderAlignment::Top);
-    for (idx, line) in lines.iter().enumerate() {
-        if line.len() == 0 {
+    for (line, line_display_width) in lines.iter().zip(lines_display_width.iter()) {
+        if *line_display_width == 0 {
             println!(
                 "{}{}{}",
                 VERTICAL.yellow(),
                 SPACE.repeat(full_message_width),
                 VERTICAL.yellow()
             );
         } else {
-            let line_padding = (full_message_width - lines_display_width[idx]) / 2;
+            let line_padding = (full_message_width - line_display_width) / 2;
             // for lines of odd length, tack the reminder to the end
             let line_padding_remainder =
-                full_message_width - (line_padding * 2) - lines_display_width[idx];
+                full_message_width - (line_padding * 2) - line_display_width;
             println!(
                 "{}{}{}{}{}",
                 VERTICAL.yellow(),

crates/update-notifier/src/utils.rs

@@ -11,8 +11,6 @@ pub fn ms_since_epoch() -> u128 {
 }
 
 pub fn get_version() -> &'static str {
-    // TODO: this should probably be passed in?
-
     log::debug!("fetching current version");
     include_str!("../../../version.txt")
         .split_once('\n')