WIP: include logic to deprecate output option

gocamille · gocamille · commit 0c0824dba654 · 2022-11-24T16:46:40.000+01:00
diff --git a/src/cli.rs b/src/cli.rs
@@ -1,11 +1,12 @@
 use camino::Utf8PathBuf;
-use clap::{Parser, ValueEnum};
+use clap::{error::ErrorKind as ClapErrorKind, Parser, ValueEnum};
 use lazycell::{AtomicLazyCell, LazyCell};
 use reqwest::blocking::Client;
 use serde::Serialize;
 
 use crate::command::output::JsonOutput;
 use crate::command::{self, RoverOutput};
+use crate::options::OutputType;
 use crate::utils::{
     client::{ClientBuilder, ClientTimeout, StudioClientConfig},
     env::{RoverEnv, RoverEnvKey},
@@ -14,6 +15,7 @@ use crate::utils::{
 };
 use crate::RoverResult;
 
+use clap::CommandFactory;
 use config::Config;
 use houston as config;
 use rover_client::shared::GitContext;
@@ -62,12 +64,12 @@ pub struct Rover {
     log_level: Option<Level>,
 
     /// Specify Rover's format type
-    #[arg(long = "format", default_value = "plain", global = true)]
-    format_type: FormatType,
+    #[arg(long = "format", global = true)]
+    format_type: Option<FormatType>,
 
     /// Specify a file to write Rover's output to
     #[arg(long = "output", global = true)]
-    output_type: OutputType,
+    output_type: Option<OutputType>,
 
     /// Accept invalid certificates when performing HTTPS requests.
     ///
@@ -118,9 +120,24 @@ impl Rover {
         Rover::parse().run()
     }
 
+    pub fn validate_options(&self) {
+        match (&self.format_type, &self.output_type) {
+            (Some(_), Some(OutputType::LegacyOutputType(_))) => {
+                let mut cmd = Rover::command();
+                cmd.error(
+                    ClapErrorKind::ArgumentConflict,
+                    "The argument '--output' cannot be used with '--format' when '--output' is not a file",
+                )
+                .exit();
+            },
+            _ => ()
+        }
+    }
+
     pub fn run(&self) -> io::Result<()> {
         timber::init(self.log_level);
         tracing::trace!(command_structure = ?self);
+        self.validate_options();
 
         // attempt to create a new `Session` to capture anonymous usage data
         let rover_output = match Session::new(self) {
@@ -157,15 +174,16 @@ impl Rover {
         match rover_output {
             Ok(output) => {
                 match self.format_type {
-                    FormatType::Plain => output.print()?,
-                    FormatType::Json => JsonOutput::from(output).print()?,
-                }
+                    Some(FormatType::Plain)| None => output.get_stdout()?,
+                    Some(FormatType::Json) => JsonOutput::from(output).print()?,
+                    _ => None
+                };
                 process::exit(0);
             }
             Err(error) => {
                 match self.format_type {
-                    FormatType::Json => JsonOutput::from(error).print()?,
-                    FormatType::Plain => {
+                    Some(FormatType::Json) => JsonOutput::from(error).print()?,
+                    Some(FormatType::Plain) | None => {
                         tracing::debug!(?error);
                         error.print()?;
                     }
@@ -225,7 +243,7 @@ impl Rover {
     }
 
     pub(crate) fn get_json(&self) -> bool {
-        matches!(self.format_type, FormatType::Json)
+        matches!(self.format_type, Some(FormatType::Json)) || matches!(self.output_type, Some(OutputType::LegacyOutputType(FormatType::Json)))
     }
 
     pub(crate) fn get_rover_config(&self) -> RoverResult<Config> {
diff --git a/src/command/output.rs b/src/command/output.rs
@@ -2,6 +2,7 @@ use std::collections::BTreeMap;
 use std::fmt::{self, Debug, Display};
 use std::io;
 
+use crate::cli::FormatType;
 use crate::command::supergraph::compose::CompositionOutput;
 use crate::utils::table::{self, row};
 use crate::RoverError;
@@ -27,7 +28,7 @@ use termimad::MadSkin;
 /// RoverOutput defines all of the different types of data that are printed
 /// to `stdout`. Every one of Rover's commands should return `saucer::Result<RoverOutput>`
 /// If the command needs to output some type of data, it should be structured
-/// in this enum, and its print logic should be handled in `RoverOutput::print`
+/// in this enum, and its print logic should be handled in `RoverOutput::get_stdout`
 ///
 /// Not all commands will output machine readable information, and those should
 /// return `Ok(RoverOutput::EmptySuccess)`. If a new command is added and it needs to
@@ -78,8 +79,8 @@ pub enum RoverOutput {
 }
 
 impl RoverOutput {
-    pub fn print(&self) -> io::Result<()> {
-        match self {
+    pub fn get_stdout(&self) -> io::Result<Option<String>> {
+        Ok(match self {
             RoverOutput::DocsList(shortlinks) => {
                 stderrln!(
                     "You can open any of these documentation pages by running {}.\n",
@@ -92,14 +93,14 @@ impl RoverOutput {
                 for (shortlink_slug, shortlink_description) in shortlinks {
                     table.add_row(row![shortlink_slug, shortlink_description]);
                 }
-                stdoutln!("{}", table)?;
+                Some(format!("{}", table))
             }
             RoverOutput::FetchResponse(fetch_response) => {
                 match fetch_response.sdl.r#type {
                     SdlType::Graph | SdlType::Subgraph { .. } => print_descriptor("SDL")?,
                     SdlType::Supergraph => print_descriptor("Supergraph SDL")?,
                 }
-                print_content(&fetch_response.sdl.contents)?;
+                Some(format!("{}", &fetch_response.sdl.contents))
             }
             RoverOutput::GraphPublishResponse {
                 graph_ref,
@@ -112,7 +113,7 @@ impl RoverOutput {
                     publish_response.change_summary
                 )?;
                 print_one_line_descriptor("Schema Hash")?;
-                print_content(&publish_response.api_schema_hash)?;
+                Some(format!("{}", &&publish_response.api_schema_hash))
             }
             RoverOutput::SubgraphPublishResponse {
                 graph_ref,
@@ -147,6 +148,7 @@ impl RoverOutput {
                     stderrln!("{} The following build errors occurred:", warn_prefix)?;
                     stderrln!("{}", &publish_response.build_errors)?;
                 }
+                None
             }
             RoverOutput::SubgraphDeleteResponse {
                 graph_ref,
@@ -196,10 +198,11 @@ impl RoverOutput {
                         stderrln!("{}", &delete_response.build_errors)?;
                     }
                 }
+                None
             }
             RoverOutput::CoreSchema(csdl) => {
                 print_descriptor("CoreSchema")?;
-                print_content(csdl)?;
+                Some(format!("{}", csdl))
             }
             RoverOutput::CompositionResult(composition_output) => {
                 let warn_prefix = Style::HintPrefix.paint("HINT:");
@@ -213,7 +216,7 @@ impl RoverOutput {
                 stderrln!("{}", hints_string)?;
 
                 print_descriptor("CoreSchema")?;
-                print_content(&composition_output.supergraph_sdl)?;
+                Some(format!("{}", &composition_output.supergraph_sdl))
             }
             RoverOutput::SubgraphList(details) => {
                 let mut table = table::get_table();
@@ -240,13 +243,10 @@ impl RoverOutput {
 
                     table.add_row(row![subgraph.name, url, formatted_updated_at]);
                 }
-
-                stdoutln!("{}", table)?;
-                stdoutln!(
-                    "View full details at {}/graph/{}/service-list",
-                    details.root_url,
-                    details.graph_ref.name
-                )?;
+                Some(format!(
+                    "{}/n View full details at {}/graph/{}/service-list",
+                    table, details.root_url, details.graph_ref.name
+                ))
             }
             RoverOutput::TemplateList(templates) => {
                 let mut table = table::get_table();
@@ -263,78 +263,72 @@ impl RoverOutput {
                     ]);
                 }
 
-                stdoutln!("{}", table)?;
+                Some(format!("{}", table))
             }
             RoverOutput::TemplateUseSuccess { template, path } => {
                 print_descriptor("Project generated")?;
-                stdoutln!(
-                    "Successfully created a new project from the '{template_id}' template in {path}",
-                    template_id = Style::Command.paint(template.id),
-                    path = Style::Path.paint(path.as_str())
-                )?;
-                stdoutln!(
-                    "Read the generated '{readme}' file for next steps.",
-                    readme = Style::Path.paint("README.md")
-                )?;
+                let template_id = Style::Command.paint(template.id);
+                let path = Style::Path.paint(path.as_str());
+                let readme = Style::Path.paint("README.md");
                 let forum_call_to_action = Style::CallToAction.paint(
                     "Have a question or suggestion about templates? Let us know at \
                     https://community.apollographql.com",
                 );
-                stdoutln!("{}", forum_call_to_action)?;
+                Some(format!("Successfully created a new project from the '{}' template in {}/n Read the generated '{}' file for next steps./n{}",
+                template_id,
+                path,
+                readme,
+                forum_call_to_action))
             }
             RoverOutput::CheckResponse(check_response) => {
                 print_descriptor("Check Result")?;
-                print_content(check_response.get_table())?;
+                Some(format!("{}", check_response.get_table()))
             }
             RoverOutput::AsyncCheckResponse(check_response) => {
                 print_descriptor("Check Started")?;
-                stdoutln!(
-                    "Check successfully started with workflow ID: {}",
-                    check_response.workflow_id,
-                )?;
-                stdoutln!("View full details at {}", check_response.target_url)?;
+                Some(format!(
+                    "Check successfully started with workflow ID: {}/nView full details at {}",
+                    check_response.workflow_id, check_response.target_url
+                ))
             }
             RoverOutput::Profiles(profiles) => {
                 if profiles.is_empty() {
                     stderrln!("No profiles found.")?;
                 } else {
                     print_descriptor("Profiles")?;
                 }
-
-                for profile in profiles {
-                    stdoutln!("{}", profile)?;
-                }
+                Some(format!("{}", profiles.join("\n")))
             }
             RoverOutput::Introspection(introspection_response) => {
                 print_descriptor("Introspection Response")?;
-                print_content(introspection_response)?;
+                Some(format!("{}", introspection_response))
             }
             RoverOutput::ErrorExplanation(explanation) => {
                 // underline bolded md
                 let mut skin = MadSkin::default();
                 skin.bold.add_attr(Underlined);
 
-                stdoutln!("{}", skin.inline(explanation))?;
+                Some(format!("{}", skin.inline(explanation)))
             }
             RoverOutput::ReadmeFetchResponse {
                 graph_ref: _,
                 content,
                 last_updated_time: _,
             } => {
                 print_descriptor("Readme")?;
-                print_content(content)?;
+                Some(format!("{}", content))
             }
             RoverOutput::ReadmePublishResponse {
                 graph_ref,
                 new_content: _,
                 last_updated_time: _,
             } => {
                 stderrln!("Readme for {} published successfully", graph_ref,)?;
+                None
             }
-            RoverOutput::EmptySuccess => (),
-        };
-        Ok(())
-    }
+            RoverOutput::EmptySuccess => None,
+        }
+)    }
 
     pub(crate) fn get_internal_data_json(&self) -> Value {
         match self {
diff --git a/src/command/subgraph/delete.rs b/src/command/subgraph/delete.rs
@@ -1,6 +1,7 @@
 use clap::Parser;
 use serde::Serialize;
 
+use crate::cli::FormatType;
 use crate::options::{GraphRefOpt, ProfileOpt, SubgraphOpt};
 use crate::utils::client::StudioClientConfig;
 use crate::{RoverOutput, RoverResult};
@@ -56,7 +57,7 @@ impl Delete {
                 dry_run,
                 delete_response: delete_dry_run_response,
             }
-            .print()?;
+            .get_stdout()?;
 
             // I chose not to error here, since this is a perfectly valid path
             if !prompt::confirm_delete()? {
diff --git a/src/options/introspect.rs b/src/options/introspect.rs
@@ -44,9 +44,9 @@ impl IntrospectOpts {
                     if was_updated {
                         let output = RoverOutput::Introspection(sdl.to_string());
                         if json {
-                            let _ = JsonOutput::from(output).print();
+                            let _ = output.get_stdout();
                         } else {
-                            let _ = output.print();
+                            let _ = output.get_stdout();
                         }
                     }
                     last_result = Some(sdl);
@@ -61,7 +61,7 @@ impl IntrospectOpts {
                     }
                     if was_updated {
                         if json {
-                            let _ = JsonOutput::from(error).print();
+                            let _ = error.print();
                         } else {
                             let _ = error.print();
                         }
diff --git a/src/options/mod.rs b/src/options/mod.rs
@@ -14,6 +14,7 @@ pub(crate) use compose::*;
 pub(crate) use graph::*;
 pub(crate) use introspect::*;
 pub(crate) use license::*;
+pub(crate) use output::*;
 pub(crate) use profile::*;
 pub(crate) use schema::*;
 pub(crate) use subgraph::*;
diff --git a/src/options/output.rs b/src/options/output.rs
@@ -1,7 +1,8 @@
 use std::str::FromStr;
 
 use camino::Utf8PathBuf;
-use clap::Parser;
+use clap::{Parser, ValueEnum};
+use serde::Serialize;
 
 use crate::cli::FormatType;
 
@@ -12,7 +13,7 @@ pub struct Output {
     output: OutputType,
 }
 
-#[derive(Debug, Clone, Eq, PartialEq)]
+#[derive(Debug, Clone, Eq, PartialEq, Serialize)]
 pub enum OutputType {
     LegacyOutputType(FormatType),
     File(Utf8PathBuf),
@@ -22,7 +23,7 @@ impl FromStr for OutputType {
     type Err = anyhow::Error;
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
-        if let Ok(format_type) = FormatType::from_str(s) {
+        if let Ok(format_type) = FormatType::from_str(s, true) {
             Ok(Self::LegacyOutputType(format_type))
         } else {
             Ok(Self::File(Utf8PathBuf::from(s)))

Original file line number	Diff line number	Diff line change
`@@ -44,9 +44,9 @@ impl IntrospectOpts {`
`44`	`44`	`if was_updated {`
`45`	`45`	`let output = RoverOutput::Introspection(sdl.to_string());`
`46`	`46`	`if json {`
`47`		`- let _ = JsonOutput::from(output).print();`
	`47`	`+ let _ = output.get_stdout();`
`48`	`48`	`} else {`
`49`		`- let _ = output.print();`
	`49`	`+ let _ = output.get_stdout();`
`50`	`50`	`}`
`51`	`51`	`}`
`52`	`52`	`last_result = Some(sdl);`
`@@ -61,7 +61,7 @@ impl IntrospectOpts {`
`61`	`61`	`}`
`62`	`62`	`if was_updated {`
`63`	`63`	`if json {`
`64`		`- let _ = JsonOutput::from(error).print();`
	`64`	`+ let _ = error.print();`
`65`	`65`	`} else {`
`66`	`66`	`let _ = error.print();`
`67`	`67`	`}`