Remove one style of printing from CLI (#1393)

echo! was a macro that had its own print style which made it difficult to understand. A few uses of echo! incorrectly assumed it took format!-style args, which was actually incorrect.

Most of this PR was done in a mechanical way via this grit script, and then touched up by hand, and then a pass where msg! was replaced by println! temporarily was used to give clippy a chance to inline format args.

src/cli/install.rs

@@ -23,7 +23,7 @@ use crate::platform::{binary_path, config_dir, home_dir};
 use crate::portable::platform;
 use crate::portable::project::project_dir;
 use crate::portable::project::{self, Init};
-use crate::print::{self, echo};
+use crate::print::{self, msg};
 use crate::print_markdown;
 use crate::process;
 use crate::question::{self, read_choice};
@@ -452,13 +452,13 @@ fn _main(options: &CliInstall) -> anyhow::Result<()> {
     if !options.no_confirm {
         match home_dir_from_passwd().zip(env::var_os("HOME")) {
             Some((passwd, env)) if passwd != env => {
-                echo!(
+                msg!(
                     "$HOME differs from euid-obtained home directory: \
                        you may be using sudo"
                 );
-                echo!("$HOME directory:", Path::new(&env).display());
-                echo!("euid-obtained home directory:", passwd.display());
-                echo!(
+                msg!("$HOME directory: {}", Path::new(&env).display());
+                msg!("euid-obtained home directory: {}", passwd.display());
+                msg!(
                     "if this is what you want, \
                        restart the installation with `-y'"
                 );
@@ -499,10 +499,7 @@ fn _main(options: &CliInstall) -> anyhow::Result<()> {
     }
 
     if cfg!(all(target_os = "macos", target_arch = "x86_64")) && platform::is_arm64_hardware() {
-        echo!(
-            BRANDING,
-            "now supports native M1 build. Downloading binary..."
-        );
+        msg!("{BRANDING} now supports native M1 build. Downloading binary...");
         return upgrade::upgrade_to_arm64();
     }
 

src/cli/upgrade.rs

@@ -12,7 +12,7 @@ use crate::platform::{binary_path, current_exe, home_dir, tmp_file_path};
 use crate::portable::platform;
 use crate::portable::repository::{self, download, Channel};
 use crate::portable::ver;
-use crate::print::{self, echo, Highlight};
+use crate::print::{self, msg, Highlight};
 use crate::process;
 
 const INDEX_TIMEOUT: Duration = Duration::new(60, 0);
@@ -226,7 +226,7 @@ fn _main(options: &CliUpgrade, path: PathBuf) -> anyhow::Result<()> {
         .run()?;
     fs::remove_file(&tmp_path).ok();
     if !options.quiet {
-        echo!("Upgraded to version", pkg.version.emphasize());
+        msg!("Upgraded to version {}", pkg.version.emphasize());
     }
     Ok(())
 }

src/cloud/secret_keys.rs

@@ -12,8 +12,7 @@ use crate::portable::exit_codes;
 
 use crate::table::{self, Cell, Row, Table};
 
-use crate::echo;
-use crate::print::{self, Highlight};
+use crate::print::{self, msg, Highlight};
 use crate::question;
 
 #[derive(Debug, serde::Deserialize, serde::Serialize)]
@@ -142,15 +141,15 @@ pub async fn _do_create(c: &options::CreateSecretKey, client: &CloudClient) -> a
         if c.non_interactive {
             print!("{sk}");
         } else {
-            echo!(
-                "\nYour new ",
+            msg!(
+                "\nYour new {} {}",
                 BRANDING_CLOUD,
                 " secret key is printed below. \
                  Be sure to copy and store it securely, as you will \
                  not be able to see it again.\n"
                     .green()
             );
-            echo!(sk.emphasize());
+            msg!("{}", sk.emphasize());
         }
     }
 

src/commands/ui.rs

@@ -9,7 +9,7 @@ use crate::commands::ExitCode;
 use crate::options::{Options, UI};
 use crate::portable::local;
 use crate::portable::repository::USER_AGENT;
-use crate::print;
+use crate::print::{self, msg};
 
 pub fn show_ui(cmd: &UI, opts: &Options) -> anyhow::Result<()> {
     let connector = opts.block_on_create_connector()?;
@@ -102,10 +102,10 @@ fn _get_local_ui_url(cmd: &UI, cfg: &edgedb_tokio::Config) -> anyhow::Result<Str
                     use_https = true;
                 }
                 Ok(status) => {
-                    print::echo!("{} returned status code {}, retry HTTP.", https_url, status);
+                    msg!("{https_url} returned status code {status}, retry HTTP.");
                 }
                 Err(e) => {
-                    print::echo!("Failed to probe {}: {:#}, retry HTTP.", https_url, e);
+                    msg!("Failed to probe {https_url}: {e:#}, retry HTTP.");
                 }
             }
         }
@@ -118,7 +118,7 @@ fn _get_local_ui_url(cmd: &UI, cfg: &edgedb_tokio::Config) -> anyhow::Result<Str
                         BRANDING,
                         " server."
                     ));
-                    print::echo!(
+                    msg!(
                         "  Try running the \
                         server with `--admin-ui=enabled`."
                     );

src/error_display.rs

@@ -12,7 +12,7 @@ use termcolor::{ColorChoice, StandardStream};
 use edgedb_errors::{Error, InternalServerError};
 
 use crate::branding::BRANDING_CLI_CMD;
-use crate::print;
+use crate::print::{self, msg};
 
 pub fn print_query_error(
     err: &Error,
@@ -119,5 +119,5 @@ fn print_query_warning_plain(warning: &Warning) {
         CString::new(marker)
     };
 
-    print::echo!(marker, warning);
+    msg!("{marker} {warning}");
 }

src/interactive.rs

@@ -25,13 +25,12 @@ use crate::classify;
 use crate::commands::{backslash, ExitCode};
 use crate::config::Config;
 use crate::credentials;
-use crate::echo;
 use crate::error_display::print_query_error;
 use crate::interrupt::{Interrupt, InterruptError};
 use crate::options::Options;
 use crate::outputs::tab_separated;
 use crate::print::Highlight;
-use crate::print::{self, PrintError};
+use crate::print::{self, msg, PrintError};
 use crate::prompt;
 use crate::repl::{self, VectorLimit};
 use crate::variables::input_variables;
@@ -152,9 +151,12 @@ pub fn main(options: Options, cfg: Config) -> Result<(), anyhow::Error> {
 pub async fn _main(options: Options, mut state: repl::State, cfg: Config) -> anyhow::Result<()> {
     state.connect().await?;
     if let Some(config_path) = &cfg.file_name {
-        echo!(format_args!("Applied {} configuration file", config_path.display(),).fade());
+        msg!(
+            "{}",
+            format_args!("Applied {} configuration file", config_path.display(),).fade()
+        );
     }
-    echo!(r#"Type \help for help, \quit to quit."#.light_gray());
+    msg!("{}", r#"Type \help for help, \quit to quit."#.light_gray());
     state.set_history_limit(state.history_limit).await?;
     match _interactive_main(&options, &mut state).await {
         Ok(()) => Ok(()),
@@ -586,7 +588,7 @@ async fn _interactive_main(
                             continue 'retry;
                         }
                         print::error("State could not be updated automatically");
-                        echo!(
+                        msg!(
                             "  Hint: This means that migrations or DDL \
                                statements were run in a concurrent \
                                connection during the interactive \

src/migrations/edit.rs

@@ -12,7 +12,7 @@ use crate::migrations::grammar::parse_migration;
 use crate::migrations::migration::{file_num, read_names};
 use crate::migrations::options::MigrationEdit;
 use crate::platform::{spawn_editor, tmp_file_path};
-use crate::print::{echo, err_marker, Highlight};
+use crate::print::{err_marker, msg, Highlight};
 use crate::question::Choice;
 
 #[derive(Copy, Clone)]
@@ -102,9 +102,9 @@ pub async fn edit_no_check(
         }
         fs::write(&tmp_file, migration.replace_id(&text, &new_id)).await?;
         fs::rename(&tmp_file, &path).await?;
-        echo!("Updated migration id to", new_id.emphasize());
+        msg!("Updated migration id to {}", new_id.emphasize());
     } else {
-        echo!("Id", migration.id.emphasize(), "is already correct.");
+        msg!("Id {} is already correct.", migration.id.emphasize());
     }
     Ok(())
 }
@@ -175,9 +175,9 @@ async fn _edit(
                 anyhow::Ok(())
             })
             .await?;
-            echo!("Updated migration id to", new_id.emphasize());
+            msg!("Updated migration id to {}", new_id.emphasize());
         } else {
-            echo!("Id", migration.id.emphasize(), "is already correct.");
+            msg!("Id {} is already correct.", migration.id.emphasize());
         }
     } else {
         let temp_path = path.parent().unwrap().join(format!(".editing.{n}.edgeql"));
@@ -226,7 +226,7 @@ async fn _edit(
             let migration = match parse_migration(&new_data) {
                 Ok(migr) => migr,
                 Err(e) => {
-                    echo!(err_marker(), "error parsing file:", e);
+                    msg!("{} error parsing file: {}", err_marker(), e);
                     loop {
                         let mut q = Choice::new("Edit again?");
                         q.option(
@@ -276,14 +276,14 @@ async fn _edit(
             if migration.id != new_id {
                 new_data = migration.replace_id(&new_data, &new_id);
                 fs::write(&temp_path, &new_data).await?;
-                echo!("Updated migration id to", new_id.emphasize());
+                msg!("Updated migration id to {}", new_id.emphasize());
             } else {
-                echo!("Id", migration.id.emphasize(), "is already correct.");
+                msg!("Id {} is already correct.", migration.id.emphasize());
             }
             match check_migration(cli, &new_data, &path).await {
                 Ok(()) => {}
                 Err(e) => {
-                    echo!(err_marker(), "error checking migration:", e);
+                    msg!("{} error checking migration: {}", err_marker(), e);
                     loop {
                         let mut q = Choice::new("Edit again?");
                         q.option(FailAction::Edit, &["y", "yes"][..], "edit the file again");

src/migrations/squash.rs

@@ -20,7 +20,7 @@ use crate::migrations::migration;
 use crate::migrations::options::CreateMigration;
 use crate::migrations::status::migrations_applied;
 use crate::migrations::timeout;
-use crate::print::{echo, Highlight};
+use crate::print::{msg, Highlight};
 use crate::question::Confirm;
 
 struct TwoStageRemove<'a> {
@@ -41,11 +41,11 @@ pub async fn main(
     let needs_fixup = needs_fixup(cli, &ctx).await?;
 
     if db_rev == "initial" {
-        echo!("No migrations exist. No actions will be taken.");
+        msg!("No migrations exist. No actions will be taken.");
         return Ok(());
     }
     if migrations.len() == 1 && !needs_fixup {
-        echo!("Only a single revision exists. No actions will be taken.");
+        msg!("Only a single revision exists. No actions will be taken.");
         return Ok(());
     }
     if !create.non_interactive {
@@ -122,75 +122,79 @@ async fn create_revision(
 }
 
 async fn confirm_squashing(db_rev: &str) -> anyhow::Result<()> {
-    echo!("Current database revision:", db_rev.emphasize());
-    echo!(
+    msg!("Current database revision: {}", db_rev.emphasize());
+    msg!(
         "While squashing migrations is non-destructive, it may lead to manual work \
            if done incorrectly."
     );
-    echo!("");
-    echo!("Items to check before using --squash:");
-    echo!("  1. Ensure that the `./dbschema` dir is committed to version control");
-    echo!(
+    msg!();
+    msg!("Items to check before using --squash:");
+    msg!("  1. Ensure that the `./dbschema` dir is committed to version control");
+    msg!(
         "  2. Ensure that other users of the database either have all .edgeql files\n     \
                 up to the revision above or can create the database from scratch.\n \
                 Hint: To see the current revision for a specific instance, run:"
     );
-    echo!(
-        "       ",
+    msg!(
+        "       {} {}",
         BRANDING_CLI_CMD,
         " -I <name> migration log --from-db --newest-first --limit 1".command_hint()
     );
-    echo!(
+    msg!(
         "  3. Merge version control branches that contain schema changes \
                 if possible."
     );
-    echo!("");
+    msg!();
     if !Confirm::new("Proceed?").async_ask().await? {
         return Err(ExitCode::new(0))?;
     }
     Ok(())
 }
 
 async fn want_fixup() -> anyhow::Result<bool> {
-    echo!(
+    msg!(
         "Your schema differs from the last revision. \
            A fixup file can be created to automate \
            upgrading other instances to a squashed revision. \
            This starts the usual migration creation process."
     );
-    echo!("");
-    echo!(
+    msg!();
+    msg!(
         "Feel free to skip this step if you don't have \
            other instances to migrate"
     );
-    echo!("");
+    msg!();
     Confirm::new("Create a fixup file?").async_ask().await
 }
 
 fn print_final_message(fixup_created: bool) -> anyhow::Result<()> {
     if fixup_created {
-        echo!("Squash is complete.");
-        echo!("");
-        echo!(
+        msg!("Squash is complete.");
+        msg!();
+        msg!(
             "Remember to commit the `dbschema` directory including deleted \
                files and `fixups` subdirectory. Recommended command:"
         );
-        echo!("    git add dbschema".command_hint());
-        echo!("");
-        echo!("The normal migration process will update your migration history:");
-        echo!("    ", BRANDING_CLI_CMD, "migrate".command_hint());
+        msg!("{}", "    git add dbschema".command_hint());
+        msg!();
+        msg!("The normal migration process will update your migration history:");
+        msg!("    {} {}", BRANDING_CLI_CMD, "migrate".command_hint());
     } else {
-        echo!("Squash is complete.");
-        echo!("");
-        echo!(
+        msg!("Squash is complete.");
+        msg!();
+        msg!(
             "Remember to commit the `dbschema` directory including deleted \
                files. Recommended command:"
         );
-        echo!("    git add dbschema".command_hint());
-        echo!("");
-        echo!("You can now wipe your instances and apply the new schema:");
-        echo!("    ", BRANDING_CLI_CMD, "database wipe".command_hint());
-        echo!("    ", BRANDING_CLI_CMD, "migrate".command_hint());
+        msg!("{}", "    git add dbschema".command_hint());
+        msg!();
+        msg!("You can now wipe your instances and apply the new schema:");
+        msg!(
+            "    {} {}",
+            BRANDING_CLI_CMD,
+            "database wipe".command_hint()
+        );
+        msg!("    {} {}", BRANDING_CLI_CMD, "migrate".command_hint());
     }
     Ok(())
 }