From 894d81a450dc40f32860136da55cb6f543864427 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Fri, 21 Oct 2022 15:23:10 +0200 Subject: [PATCH 01/26] feat(cli): change --output option name to --format --- src/cli.rs | 25 ++++++++++++++----------- src/options/mod.rs | 1 + src/options/output.rs | 30 ++++++++++++++++++++++++++++++ 3 files changed, 45 insertions(+), 11 deletions(-) create mode 100644 src/options/output.rs diff --git a/src/cli.rs b/src/cli.rs index 35b727022..3cb0cb966 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -65,6 +65,9 @@ pub struct Rover { #[arg(long = "output", default_value = "plain", global = true)] output_type: OutputType, + #[clap(long = "format", default_value = "plain", global = true)] + format_type: FormatType, + /// Accept invalid certificates when performing HTTPS requests. /// /// You should think very carefully before using this flag. @@ -152,16 +155,16 @@ impl Rover { match rover_output { Ok(output) => { - match self.output_type { - OutputType::Plain => output.print()?, - OutputType::Json => JsonOutput::from(output).print()?, + match self.format_type { + FormatType::Plain => output.print()?, + FormatType::Json => JsonOutput::from(output).print()?, } process::exit(0); } Err(error) => { - match self.output_type { - OutputType::Json => JsonOutput::from(error).print()?, - OutputType::Plain => { + match self.format_type { + FormatType::Json => JsonOutput::from(error).print()?, + FormatType::Plain => { tracing::debug!(?error); error.print()?; } @@ -221,7 +224,7 @@ impl Rover { } pub(crate) fn get_json(&self) -> bool { - matches!(self.output_type, OutputType::Json) + matches!(self.format_type, FormatType::Json) } pub(crate) fn get_rover_config(&self) -> RoverResult { @@ -411,13 +414,13 @@ pub enum Command { } #[derive(ValueEnum, Debug, Serialize, Clone, Eq, PartialEq)] -pub enum OutputType { +pub enum FormatType { Plain, Json, } -impl Default for OutputType { +impl Default for FormatType { fn default() -> Self { - OutputType::Plain + FormatType::Plain } -} +} \ No newline at end of file diff --git a/src/options/mod.rs b/src/options/mod.rs index 3f81a1385..613ba78b1 100644 --- a/src/options/mod.rs +++ b/src/options/mod.rs @@ -3,6 +3,7 @@ mod compose; mod graph; mod introspect; mod license; +mod output; mod profile; mod schema; mod subgraph; diff --git a/src/options/output.rs b/src/options/output.rs new file mode 100644 index 000000000..6974ebe13 --- /dev/null +++ b/src/options/output.rs @@ -0,0 +1,30 @@ +use std::str::FromStr; + +use saucer::{clap, Parser, Utf8PathBuf}; + +use crate::{cli::FormatType}; + +#[derive(Debug, Parser)] +pub struct Output { + /// The file path to write the command output to. + #[clap(long)] + output: OutputType, +} + +#[derive(Debug, Clone, Eq, PartialEq)] +pub enum OutputType { + LegacyOutputType(FormatType), + File(Utf8PathBuf), +} + +impl FromStr for OutputType { + type Err = saucer::Error; + + fn from_str(s: &str) -> Result { + if let Ok(format_type) = FormatType::from_str(s) { + Ok(Self::LegacyOutputType(format_type)) + } else { + Ok(Self::File(Utf8PathBuf::from(s))) + } + } +} From bda3c89d7283382607da9de8b9c3e8240a3b9dfa Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Thu, 10 Nov 2022 10:20:59 -0600 Subject: [PATCH 02/26] Merge branch 'main' into camille/update-format-option (#1403) Co-authored-by: Trevor Blades Co-authored-by: Sachin D. Shinde Co-authored-by: Jesse Wang Co-authored-by: Patrick Arminio Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- src/cli.rs | 11 +++-- src/command/dev/command.rs | 93 ++++++++++++++++++++++++++++++++++++++ src/options/output.rs | 7 +-- xtask/src/tools/cargo.rs | 2 +- xtask/src/tools/runner.rs | 3 +- 5 files changed, 106 insertions(+), 10 deletions(-) create mode 100644 src/command/dev/command.rs diff --git a/src/cli.rs b/src/cli.rs index 3cb0cb966..614cec94a 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -61,13 +61,14 @@ pub struct Rover { #[serde(serialize_with = "option_from_display")] log_level: Option, - /// Specify Rover's output type - #[arg(long = "output", default_value = "plain", global = true)] - output_type: OutputType, - - #[clap(long = "format", default_value = "plain", global = true)] + /// Specify Rover's format type + #[arg(long = "format", default_value = "plain", global = true)] format_type: FormatType, + /// Specify a file to write Rover's output to + #[arg(long = "output", global = true)] + output_type: OutputType, + /// Accept invalid certificates when performing HTTPS requests. /// /// You should think very carefully before using this flag. diff --git a/src/command/dev/command.rs b/src/command/dev/command.rs new file mode 100644 index 000000000..787444bba --- /dev/null +++ b/src/command/dev/command.rs @@ -0,0 +1,93 @@ +use std::{ + io::{BufRead, BufReader}, + process::{Child, Command, Stdio}, +}; + +use anyhow::{anyhow, Context}; +use crossbeam_channel::Sender; + +use crate::{command::dev::do_dev::log_err_and_continue, RoverError, RoverResult}; + +#[derive(Debug)] +pub struct BackgroundTask { + child: Child, +} + +pub enum BackgroundTaskLog { + Stdout(String), + Stderr(String), +} + +impl BackgroundTask { + pub fn new(command: String, log_sender: Sender) -> RoverResult { + let args: Vec<&str> = command.split(' ').collect(); + let (bin, args) = match args.len() { + 0 => Err(anyhow!("the command you passed is empty")), + 1 => Ok((args[0], Vec::new())), + _ => Ok((args[0], Vec::from_iter(args[1..].iter()))), + }?; + tracing::info!("starting `{}`", &command); + if which::which(bin).is_err() { + return Err(anyhow!("{} is not installed on this machine", &bin).into()); + } + + let mut command = Command::new(bin); + command.args(args).env("APOLLO_ROVER", "true"); + + command.stdout(Stdio::piped()).stderr(Stdio::piped()); + + let mut child = command + .spawn() + .with_context(|| "could not spawn child process")?; + + if let Some(stdout) = child.stdout.take() { + let log_sender = log_sender.clone(); + rayon::spawn(move || { + let stdout = BufReader::new(stdout); + stdout.lines().for_each(|line| { + if let Ok(line) = line { + log_sender + .send(BackgroundTaskLog::Stdout(line)) + .expect("could not update stdout logs for command"); + } + }); + }); + } + + if let Some(stderr) = child.stderr.take() { + rayon::spawn(move || { + let stderr = BufReader::new(stderr); + stderr.lines().for_each(|line| { + if let Ok(line) = line { + log_sender + .send(BackgroundTaskLog::Stderr(line)) + .expect("could not update stderr logs for command"); + } + }); + }); + } + + Ok(Self { child }) + } + + pub fn kill(&mut self) { + let pid = self.id(); + tracing::info!("killing child with pid {}", &pid); + let _ = self.child.kill().map_err(|_| { + log_err_and_continue(RoverError::new(anyhow!( + "could not kill child with pid {}", + &pid + ))); + }); + } + + pub fn id(&self) -> u32 { + self.child.id() + } +} + +impl Drop for BackgroundTask { + fn drop(&mut self) { + self.kill() + } +} diff --git a/src/options/output.rs b/src/options/output.rs index 6974ebe13..54e0d8cdc 100644 --- a/src/options/output.rs +++ b/src/options/output.rs @@ -1,8 +1,9 @@ use std::str::FromStr; -use saucer::{clap, Parser, Utf8PathBuf}; +use camino::Utf8PathBuf; +use clap::Parser; -use crate::{cli::FormatType}; +use crate::cli::FormatType; #[derive(Debug, Parser)] pub struct Output { @@ -18,7 +19,7 @@ pub enum OutputType { } impl FromStr for OutputType { - type Err = saucer::Error; + type Err = anyhow::Error; fn from_str(s: &str) -> Result { if let Ok(format_type) = FormatType::from_str(s) { diff --git a/xtask/src/tools/cargo.rs b/xtask/src/tools/cargo.rs index 219ee18c2..0d9bd84b1 100644 --- a/xtask/src/tools/cargo.rs +++ b/xtask/src/tools/cargo.rs @@ -4,7 +4,7 @@ use camino::Utf8PathBuf; use crate::commands::version::RoverVersion; use crate::target::Target; use crate::tools::{GitRunner, Runner}; -use crate::utils::{CommandOutput, PKG_PROJECT_ROOT}; +use crate::utils::{self, CommandOutput, PKG_PROJECT_ROOT}; use std::fs; diff --git a/xtask/src/tools/runner.rs b/xtask/src/tools/runner.rs index 7a770dd5a..7a7748f3e 100644 --- a/xtask/src/tools/runner.rs +++ b/xtask/src/tools/runner.rs @@ -1,7 +1,8 @@ +use anyhow::{anyhow, Context, Result}; use camino::Utf8PathBuf; use shell_candy::{ShellTask, ShellTaskBehavior, ShellTaskLog, ShellTaskOutput}; - use std::collections::HashMap; +use which::which; use crate::{utils::CommandOutput, Result}; From f587f382460ed9aa8dcc35c50dfeb742ee3c297c Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Thu, 24 Nov 2022 16:46:40 +0100 Subject: [PATCH 03/26] feat(cli): Include logic to deprecate output option feat(cli): check both output_type and format_type for formatting --- src/cli.rs | 68 +++++++++++++++++----- src/command/output.rs | 101 ++++++++++++++++----------------- src/command/subgraph/delete.rs | 3 +- src/options/introspect.rs | 6 +- src/options/mod.rs | 1 + src/options/output.rs | 7 ++- 6 files changed, 111 insertions(+), 75 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 614cec94a..d87f31bac 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,11 +1,13 @@ 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::options::OutputType::LegacyOutputType; use crate::utils::{ client::{ClientBuilder, ClientTimeout, StudioClientConfig}, env::{RoverEnv, RoverEnvKey}, @@ -14,6 +16,7 @@ use crate::utils::{ }; use crate::RoverResult; +use clap::CommandFactory; use config::Config; use houston as config; use rover_client::shared::GitContext; @@ -62,12 +65,12 @@ pub struct Rover { log_level: Option, /// Specify Rover's format type - #[arg(long = "format", default_value = "plain", global = true)] - format_type: FormatType, + #[arg(long = "format", global = true)] + format_type: Option, /// Specify a file to write Rover's output to #[arg(long = "output", global = true)] - output_type: OutputType, + output_type: Option, /// Accept invalid certificates when performing HTTPS requests. /// @@ -118,9 +121,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) { @@ -156,20 +174,36 @@ impl Rover { match rover_output { Ok(output) => { - match self.format_type { - FormatType::Plain => output.print()?, - FormatType::Json => JsonOutput::from(output).print()?, - } + match self { + Rover { + format_type: Some(FormatType::Json), + .. + } => JsonOutput::from(output).print()?, + Rover { + output_type: Some(LegacyOutputType(FormatType::Json)), + .. + } => JsonOutput::from(output).print()?, + _ => RoverOutput::from(output).print()?, + }; + process::exit(0); } Err(error) => { - match self.format_type { - FormatType::Json => JsonOutput::from(error).print()?, - FormatType::Plain => { + match self { + Rover { + format_type: Some(FormatType::Json), + .. + } => JsonOutput::from(error).print()?, + Rover { + output_type: Some(LegacyOutputType(FormatType::Json)), + .. + } => JsonOutput::from(error).print()?, + _ => { tracing::debug!(?error); - error.print()?; + error.print()? } - } + }; + process::exit(1); } } @@ -225,7 +259,11 @@ 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 { @@ -424,4 +462,4 @@ impl Default for FormatType { fn default() -> Self { FormatType::Plain } -} \ No newline at end of file +} diff --git a/src/command/output.rs b/src/command/output.rs index 44fb77c47..eaeb1015b 100644 --- a/src/command/output.rs +++ b/src/command/output.rs @@ -1,12 +1,14 @@ use std::collections::BTreeMap; use std::fmt::{self, Debug, Display}; use std::io; +use std::str::FromStr; +use crate::cli::FormatType; use crate::command::supergraph::compose::CompositionOutput; use crate::utils::table::{self, row}; use crate::RoverError; -use crate::options::GithubTemplate; +use crate::options::{GithubTemplate, OutputType}; use atty::Stream; use calm_io::{stderr, stderrln, stdout, stdoutln}; use camino::Utf8PathBuf; @@ -27,7 +29,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` /// 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 +80,8 @@ pub enum RoverOutput { } impl RoverOutput { - pub fn print(&self) -> io::Result<()> { - match self { + pub fn get_stdout(&self) -> io::Result<()> { + Ok(match self { RoverOutput::DocsList(shortlinks) => { stderrln!( "You can open any of these documentation pages by running {}.\n", @@ -92,14 +94,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 +114,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 +149,7 @@ impl RoverOutput { stderrln!("{} The following build errors occurred:", warn_prefix)?; stderrln!("{}", &publish_response.build_errors)?; } + (); } RoverOutput::SubgraphDeleteResponse { graph_ref, @@ -170,6 +173,7 @@ impl RoverOutput { stderrln!("{} At the time of checking, there would be no build errors resulting from the deletion of this subgraph.", warn_prefix)?; stderrln!("{} This is only a prediction. If the graph changes before confirming, there could be build errors.", warn_prefix)? } + (); } else { if delete_response.supergraph_was_updated { stderrln!( @@ -195,11 +199,12 @@ impl RoverOutput { stderrln!("{}", &delete_response.build_errors)?; } + (); } } 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 +218,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 +245,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,36 +265,33 @@ 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() { @@ -300,21 +299,18 @@ impl RoverOutput { } 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: _, @@ -322,7 +318,7 @@ impl RoverOutput { last_updated_time: _, } => { print_descriptor("Readme")?; - print_content(content)?; + Some(format!("{}", content)); } RoverOutput::ReadmePublishResponse { graph_ref, @@ -330,10 +326,10 @@ impl RoverOutput { last_updated_time: _, } => { stderrln!("Readme for {} published successfully", graph_ref,)?; + (); } RoverOutput::EmptySuccess => (), - }; - Ok(()) + }) } pub(crate) fn get_internal_data_json(&self) -> Value { @@ -453,6 +449,16 @@ impl RoverOutput { pub(crate) fn get_json_version(&self) -> JsonVersion { JsonVersion::default() } + + pub(crate) fn print(&self) -> io::Result<()> { + stdoutln!("{}", self) + } +} + +impl fmt::Display for RoverOutput { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + write!(f, "{}", self) + } } fn print_descriptor(descriptor: impl Display) -> io::Result<()> { @@ -468,17 +474,6 @@ fn print_one_line_descriptor(descriptor: impl Display) -> io::Result<()> { Ok(()) } -/// if the user is outputting to a terminal, we want there to be a terminating -/// newline, but we don't want that newline to leak into output that's piped -/// to a file, like from a `graph fetch` -fn print_content(content: impl Display) -> io::Result<()> { - if atty::is(Stream::Stdout) { - stdoutln!("{}", content) - } else { - stdout!("{}", content) - } -} - #[derive(Debug, Clone, Serialize)] pub(crate) struct JsonOutput { json_version: JsonVersion, diff --git a/src/command/subgraph/delete.rs b/src/command/subgraph/delete.rs index da9644080..c51d2f7ec 100644 --- 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 index 3b586670d..c64d30f2a 100644 --- 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 index 613ba78b1..0aa8afbbe 100644 --- 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 index 54e0d8cdc..00bfd42a7 100644 --- 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 { - 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))) From 7b75c1c2249c1c23df880ab27fedd48ead6d7a7b Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Sat, 26 Nov 2022 00:26:51 +0100 Subject: [PATCH 04/26] feat(cli): refactor rover_output logic --- src/cli.rs | 36 +++++++++++------------------------- 1 file changed, 11 insertions(+), 25 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index d87f31bac..dfe855e30 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -174,35 +174,21 @@ impl Rover { match rover_output { Ok(output) => { - match self { - Rover { - format_type: Some(FormatType::Json), - .. - } => JsonOutput::from(output).print()?, - Rover { - output_type: Some(LegacyOutputType(FormatType::Json)), - .. - } => JsonOutput::from(output).print()?, - _ => RoverOutput::from(output).print()?, - }; + if self.get_json() { + JsonOutput::from(output).print()? + } else { + RoverOutput::from(output).print()? + } process::exit(0); } Err(error) => { - match self { - Rover { - format_type: Some(FormatType::Json), - .. - } => JsonOutput::from(error).print()?, - Rover { - output_type: Some(LegacyOutputType(FormatType::Json)), - .. - } => JsonOutput::from(error).print()?, - _ => { - tracing::debug!(?error); - error.print()? - } - }; + if self.get_json() { + JsonOutput::from(error).print()? + } else { + tracing::debug!(?error); + error.print()? + } process::exit(1); } From e08c20c29da5c6efb67c26e98180455d2efa1964 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Sat, 26 Nov 2022 03:46:09 +0100 Subject: [PATCH 05/26] feat(cli): add a print function to stdout plain text --- src/cli.rs | 3 +- src/command/output.rs | 56 +++++++++++++++------------------- src/command/subgraph/delete.rs | 1 - src/options/introspect.rs | 2 +- 4 files changed, 27 insertions(+), 35 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index dfe855e30..96c451e75 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -7,7 +7,6 @@ use serde::Serialize; use crate::command::output::JsonOutput; use crate::command::{self, RoverOutput}; use crate::options::OutputType; -use crate::options::OutputType::LegacyOutputType; use crate::utils::{ client::{ClientBuilder, ClientTimeout, StudioClientConfig}, env::{RoverEnv, RoverEnvKey}, @@ -177,7 +176,7 @@ impl Rover { if self.get_json() { JsonOutput::from(output).print()? } else { - RoverOutput::from(output).print()? + output.print()? } process::exit(0); diff --git a/src/command/output.rs b/src/command/output.rs index eaeb1015b..34c7d1708 100644 --- a/src/command/output.rs +++ b/src/command/output.rs @@ -1,16 +1,14 @@ use std::collections::BTreeMap; use std::fmt::{self, Debug, Display}; use std::io; -use std::str::FromStr; -use crate::cli::FormatType; use crate::command::supergraph::compose::CompositionOutput; use crate::utils::table::{self, row}; use crate::RoverError; -use crate::options::{GithubTemplate, OutputType}; +use crate::options::GithubTemplate; use atty::Stream; -use calm_io::{stderr, stderrln, stdout, stdoutln}; +use calm_io::{stderr, stderrln, stdoutln}; use camino::Utf8PathBuf; use crossterm::style::Attribute::Underlined; use rover_client::operations::graph::publish::GraphPublishResponse; @@ -80,7 +78,7 @@ pub enum RoverOutput { } impl RoverOutput { - pub fn get_stdout(&self) -> io::Result<()> { + pub fn get_stdout(&self) -> io::Result> { Ok(match self { RoverOutput::DocsList(shortlinks) => { stderrln!( @@ -94,14 +92,14 @@ impl RoverOutput { for (shortlink_slug, shortlink_description) in shortlinks { table.add_row(row![shortlink_slug, shortlink_description]); } - Some(format!("{}", 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")?, } - Some(format!("{}", &fetch_response.sdl.contents)); + Some(format!("{}", &fetch_response.sdl.contents)) } RoverOutput::GraphPublishResponse { graph_ref, @@ -114,7 +112,7 @@ impl RoverOutput { publish_response.change_summary )?; print_one_line_descriptor("Schema Hash")?; - Some(format!("{}", &&publish_response.api_schema_hash)); + Some(format!("{}", &&publish_response.api_schema_hash)) } RoverOutput::SubgraphPublishResponse { graph_ref, @@ -149,7 +147,7 @@ impl RoverOutput { stderrln!("{} The following build errors occurred:", warn_prefix)?; stderrln!("{}", &publish_response.build_errors)?; } - (); + None } RoverOutput::SubgraphDeleteResponse { graph_ref, @@ -173,7 +171,7 @@ impl RoverOutput { stderrln!("{} At the time of checking, there would be no build errors resulting from the deletion of this subgraph.", warn_prefix)?; stderrln!("{} This is only a prediction. If the graph changes before confirming, there could be build errors.", warn_prefix)? } - (); + None } else { if delete_response.supergraph_was_updated { stderrln!( @@ -199,12 +197,12 @@ impl RoverOutput { stderrln!("{}", &delete_response.build_errors)?; } - (); + None } } RoverOutput::CoreSchema(csdl) => { print_descriptor("CoreSchema")?; - Some(format!("{}", csdl)); + Some(format!("{}", csdl)) } RoverOutput::CompositionResult(composition_output) => { let warn_prefix = Style::HintPrefix.paint("HINT:"); @@ -218,7 +216,7 @@ impl RoverOutput { stderrln!("{}", hints_string)?; print_descriptor("CoreSchema")?; - Some(format!("{}", &composition_output.supergraph_sdl)); + Some(format!("{}", &composition_output.supergraph_sdl)) } RoverOutput::SubgraphList(details) => { let mut table = table::get_table(); @@ -248,7 +246,7 @@ impl RoverOutput { 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(); @@ -265,7 +263,7 @@ impl RoverOutput { ]); } - Some(format!("{}", table)); + Some(format!("{}", table)) } RoverOutput::TemplateUseSuccess { template, path } => { print_descriptor("Project generated")?; @@ -280,18 +278,18 @@ impl RoverOutput { template_id, path, readme, - forum_call_to_action)); + forum_call_to_action)) } RoverOutput::CheckResponse(check_response) => { print_descriptor("Check Result")?; - Some(format!("{}", check_response.get_table())); + Some(format!("{}", check_response.get_table())) } RoverOutput::AsyncCheckResponse(check_response) => { print_descriptor("Check Started")?; 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() { @@ -299,18 +297,18 @@ impl RoverOutput { } else { print_descriptor("Profiles")?; } - Some(format!("{}", profiles.join("\n"))); + Some(format!("{}", profiles.join("\n"))) } RoverOutput::Introspection(introspection_response) => { print_descriptor("Introspection Response")?; - Some(format!("{}", introspection_response)); + Some(format!("{}", introspection_response)) } RoverOutput::ErrorExplanation(explanation) => { // underline bolded md let mut skin = MadSkin::default(); skin.bold.add_attr(Underlined); - Some(format!("{}", skin.inline(explanation))); + Some(format!("{}", skin.inline(explanation))) } RoverOutput::ReadmeFetchResponse { graph_ref: _, @@ -318,7 +316,7 @@ impl RoverOutput { last_updated_time: _, } => { print_descriptor("Readme")?; - Some(format!("{}", content)); + Some(format!("{}", content)) } RoverOutput::ReadmePublishResponse { graph_ref, @@ -326,9 +324,9 @@ impl RoverOutput { last_updated_time: _, } => { stderrln!("Readme for {} published successfully", graph_ref,)?; - (); + None } - RoverOutput::EmptySuccess => (), + RoverOutput::EmptySuccess => None, }) } @@ -451,13 +449,9 @@ impl RoverOutput { } pub(crate) fn print(&self) -> io::Result<()> { - stdoutln!("{}", self) - } -} - -impl fmt::Display for RoverOutput { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self) + // TODO: refactor to use a safe method to unwrap the Optional Result + let content = format!("{}", self.get_stdout().unwrap().unwrap()); + stdoutln!("{}", content) } } diff --git a/src/command/subgraph/delete.rs b/src/command/subgraph/delete.rs index c51d2f7ec..20c8da9c3 100644 --- a/src/command/subgraph/delete.rs +++ b/src/command/subgraph/delete.rs @@ -1,7 +1,6 @@ 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}; diff --git a/src/options/introspect.rs b/src/options/introspect.rs index c64d30f2a..c4371dbe7 100644 --- a/src/options/introspect.rs +++ b/src/options/introspect.rs @@ -2,7 +2,7 @@ use clap::Parser; use reqwest::Url; use serde::{Deserialize, Serialize}; -use crate::{command::output::JsonOutput, utils::parsers::parse_header, RoverOutput, RoverResult}; +use crate::{utils::parsers::parse_header, RoverOutput, RoverResult}; #[derive(Debug, Serialize, Deserialize, Parser)] pub struct IntrospectOpts { From e8eb42ca2328ec8081a5887d046922a26113c199 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Sun, 27 Nov 2022 00:14:49 +0100 Subject: [PATCH 06/26] feat(cli): add matching and error handling for plaintext stdout --- src/command/output.rs | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/src/command/output.rs b/src/command/output.rs index 34c7d1708..6c1c0ed3f 100644 --- a/src/command/output.rs +++ b/src/command/output.rs @@ -7,6 +7,7 @@ use crate::utils::table::{self, row}; use crate::RoverError; use crate::options::GithubTemplate; +use anyhow::anyhow; use atty::Stream; use calm_io::{stderr, stderrln, stdoutln}; use camino::Utf8PathBuf; @@ -449,9 +450,28 @@ impl RoverOutput { } pub(crate) fn print(&self) -> io::Result<()> { - // TODO: refactor to use a safe method to unwrap the Optional Result - let content = format!("{}", self.get_stdout().unwrap().unwrap()); - stdoutln!("{}", content) + let result = self.get_stdout(); + + match result { + Ok(data) => match data { + Some(data) => { + stdoutln!("{}", format!("{}", data)) + } + None => Ok(()), + }, + Err(e) => { + if let Some(raw_os_err) = e.raw_os_error() { + tracing::debug!( + "Unknown error when formatting RoverOutput:\nError: {}\n{}", + e, + raw_os_err + ); + Ok(()) + } else { + Err(io::Error::new(io::ErrorKind::Other, anyhow!("{}", e))) + } + } + } } } From 0a7dd7c2654f7a1193de72a7ae20ee55a333b9a2 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Sun, 27 Nov 2022 01:16:53 +0100 Subject: [PATCH 07/26] feat(cli): Add deprecation message for '--output' --- src/cli.rs | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/cli.rs b/src/cli.rs index 96c451e75..ca106a339 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -121,15 +121,25 @@ impl Rover { } pub fn validate_options(&self) { + let mut cmd = Rover::command(); + 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(); } + (None, Some(OutputType::LegacyOutputType(_))) => { + // Provide a deprecation message for the '--output' type. Ideally another ErrorKind will + // soon be available to perform deprecation notifications. + cmd.error( + ClapErrorKind::DisplayHelp, + "The argument '--output' will soon be deprecated. Please use the '--format' argument to specify the output type.", + ) + .exit(); + } _ => (), } } From 0520dabc9a5f5cfd502efd9d73311f53b5afce8b Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Sun, 27 Nov 2022 18:32:39 +0100 Subject: [PATCH 08/26] feat(cli): remove unused imports --- xtask/src/tools/cargo.rs | 2 +- xtask/src/tools/runner.rs | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/xtask/src/tools/cargo.rs b/xtask/src/tools/cargo.rs index 0d9bd84b1..219ee18c2 100644 --- a/xtask/src/tools/cargo.rs +++ b/xtask/src/tools/cargo.rs @@ -4,7 +4,7 @@ use camino::Utf8PathBuf; use crate::commands::version::RoverVersion; use crate::target::Target; use crate::tools::{GitRunner, Runner}; -use crate::utils::{self, CommandOutput, PKG_PROJECT_ROOT}; +use crate::utils::{CommandOutput, PKG_PROJECT_ROOT}; use std::fs; diff --git a/xtask/src/tools/runner.rs b/xtask/src/tools/runner.rs index 7a7748f3e..7a770dd5a 100644 --- a/xtask/src/tools/runner.rs +++ b/xtask/src/tools/runner.rs @@ -1,8 +1,7 @@ -use anyhow::{anyhow, Context, Result}; use camino::Utf8PathBuf; use shell_candy::{ShellTask, ShellTaskBehavior, ShellTaskLog, ShellTaskOutput}; + use std::collections::HashMap; -use which::which; use crate::{utils::CommandOutput, Result}; From 49fd6b9c437674788a0d4378d2f3dd2ba665e20c Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Mon, 28 Nov 2022 13:55:43 +0100 Subject: [PATCH 09/26] feat(cli): update '--output' deprecation msg from error to warning --- src/cli.rs | 13 ++++--------- src/command/output.rs | 2 +- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index ca106a339..4c051b6e0 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -19,6 +19,7 @@ use clap::CommandFactory; use config::Config; use houston as config; use rover_client::shared::GitContext; +use rover_std::Style; use sputnik::Session; use timber::Level; @@ -121,10 +122,9 @@ impl Rover { } pub fn validate_options(&self) { - let mut cmd = Rover::command(); - 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", @@ -132,13 +132,8 @@ impl Rover { .exit(); } (None, Some(OutputType::LegacyOutputType(_))) => { - // Provide a deprecation message for the '--output' type. Ideally another ErrorKind will - // soon be available to perform deprecation notifications. - cmd.error( - ClapErrorKind::DisplayHelp, - "The argument '--output' will soon be deprecated. Please use the '--format' argument to specify the output type.", - ) - .exit(); + let warn_prefix = Style::WarningPrefix.paint("WARN:"); + eprintln!("{} The argument '--output' will soon be deprecated. Please use the '--format' argument to specify the output type.", warn_prefix); } _ => (), } diff --git a/src/command/output.rs b/src/command/output.rs index 6c1c0ed3f..ca7c98ae9 100644 --- a/src/command/output.rs +++ b/src/command/output.rs @@ -113,7 +113,7 @@ impl RoverOutput { publish_response.change_summary )?; print_one_line_descriptor("Schema Hash")?; - Some(format!("{}", &&publish_response.api_schema_hash)) + Some(format!("{}", &publish_response.api_schema_hash)) } RoverOutput::SubgraphPublishResponse { graph_ref, From 7ff032b3b352fc9e4376bdaf75112e397a2440ad Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Mon, 28 Nov 2022 14:08:49 +0100 Subject: [PATCH 10/26] feat(cli): Update changelog --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index daa390e71..31addc34f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,15 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm ## 📚 Documentation --> +# [0.10.1] - 2022-11-28 + +## 🚀 Features + +- **Replace the '--output' option type with '--format' - @gocamille, #1413 fixes #1212** + +This change adds the new option, `--format`, to allow users to define the format type for messages printed to `stdout` (either by passing `plain` or `json` as an argument to `--format`). This replaces the use of `--output` for defining format types. The `--output` option will be available to define the output file type instead, following [Command Line Interface Guidelines for file outputs](https://clig.dev/#:~:text=%2Do%2C%20%2D%2Doutput%3A%20Output%20file.%20For%20example%2C%20sort%2C%20gcc.). + + # [0.10.0] - 2022-11-10 > Important: 1 potentially breaking change below, indicated by **❗ BREAKING ❗** From e72f260c8903ebd36a0ba10557dfc7302a247a04 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Mon, 28 Nov 2022 14:09:25 +0100 Subject: [PATCH 11/26] feat(cli): add indent to changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 31addc34f..d4222fcbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **Replace the '--output' option type with '--format' - @gocamille, #1413 fixes #1212** -This change adds the new option, `--format`, to allow users to define the format type for messages printed to `stdout` (either by passing `plain` or `json` as an argument to `--format`). This replaces the use of `--output` for defining format types. The `--output` option will be available to define the output file type instead, following [Command Line Interface Guidelines for file outputs](https://clig.dev/#:~:text=%2Do%2C%20%2D%2Doutput%3A%20Output%20file.%20For%20example%2C%20sort%2C%20gcc.). + This change adds the new option, `--format`, to allow users to define the format type for messages printed to `stdout` (either by passing `plain` or `json` as an argument to `--format`). This replaces the use of `--output` for defining format types. The `--output` option will be available to define the output file type instead, following [Command Line Interface Guidelines for file outputs](https://clig.dev/#:~:text=%2Do%2C%20%2D%2Doutput%3A%20Output%20file.%20For%20example%2C%20sort%2C%20gcc.). # [0.10.0] - 2022-11-10 From c6c90f487da4c6f4294c3ee77910c93cf8773d69 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Wed, 30 Nov 2022 12:49:45 +0100 Subject: [PATCH 12/26] feat(cli): refactor from clippy suggestions --- src/command/output.rs | 16 ++++++++-------- src/options/introspect.rs | 8 ++++---- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/command/output.rs b/src/command/output.rs index ca7c98ae9..d4bbace7c 100644 --- a/src/command/output.rs +++ b/src/command/output.rs @@ -100,7 +100,7 @@ impl RoverOutput { SdlType::Graph | SdlType::Subgraph { .. } => print_descriptor("SDL")?, SdlType::Supergraph => print_descriptor("Supergraph SDL")?, } - Some(format!("{}", &fetch_response.sdl.contents)) + Some((fetch_response.sdl.contents).to_string()) } RoverOutput::GraphPublishResponse { graph_ref, @@ -113,7 +113,7 @@ impl RoverOutput { publish_response.change_summary )?; print_one_line_descriptor("Schema Hash")?; - Some(format!("{}", &publish_response.api_schema_hash)) + Some((publish_response.api_schema_hash).to_string()) } RoverOutput::SubgraphPublishResponse { graph_ref, @@ -203,7 +203,7 @@ impl RoverOutput { } RoverOutput::CoreSchema(csdl) => { print_descriptor("CoreSchema")?; - Some(format!("{}", csdl)) + Some((csdl).to_string()) } RoverOutput::CompositionResult(composition_output) => { let warn_prefix = Style::HintPrefix.paint("HINT:"); @@ -217,7 +217,7 @@ impl RoverOutput { stderrln!("{}", hints_string)?; print_descriptor("CoreSchema")?; - Some(format!("{}", &composition_output.supergraph_sdl)) + Some((composition_output.supergraph_sdl).to_string()) } RoverOutput::SubgraphList(details) => { let mut table = table::get_table(); @@ -283,7 +283,7 @@ impl RoverOutput { } RoverOutput::CheckResponse(check_response) => { print_descriptor("Check Result")?; - Some(format!("{}", check_response.get_table())) + Some(check_response.get_table()) } RoverOutput::AsyncCheckResponse(check_response) => { print_descriptor("Check Started")?; @@ -298,11 +298,11 @@ impl RoverOutput { } else { print_descriptor("Profiles")?; } - Some(format!("{}", profiles.join("\n"))) + Some(profiles.join("\n")) } RoverOutput::Introspection(introspection_response) => { print_descriptor("Introspection Response")?; - Some(format!("{}", introspection_response)) + Some((introspection_response).to_string()) } RoverOutput::ErrorExplanation(explanation) => { // underline bolded md @@ -317,7 +317,7 @@ impl RoverOutput { last_updated_time: _, } => { print_descriptor("Readme")?; - Some(format!("{}", content)) + Some((content).to_string()) } RoverOutput::ReadmePublishResponse { graph_ref, diff --git a/src/options/introspect.rs b/src/options/introspect.rs index c4371dbe7..535ff2129 100644 --- a/src/options/introspect.rs +++ b/src/options/introspect.rs @@ -2,7 +2,7 @@ use clap::Parser; use reqwest::Url; use serde::{Deserialize, Serialize}; -use crate::{utils::parsers::parse_header, RoverOutput, RoverResult}; +use crate::{utils::parsers::parse_header, RoverOutput, RoverResult, command::output::JsonOutput}; #[derive(Debug, Serialize, Deserialize, Parser)] pub struct IntrospectOpts { @@ -44,9 +44,9 @@ impl IntrospectOpts { if was_updated { let output = RoverOutput::Introspection(sdl.to_string()); if json { - let _ = output.get_stdout(); + let _ = JsonOutput::from(output).print(); } else { - let _ = output.get_stdout(); + let _ = output.print(); } } last_result = Some(sdl); @@ -61,7 +61,7 @@ impl IntrospectOpts { } if was_updated { if json { - let _ = error.print(); + let _ = JsonOutput::from(error).print(); } else { let _ = error.print(); } From fe9abf2941c8cb6a3eb5352c0b0a6aa06340be46 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Wed, 30 Nov 2022 15:08:33 +0100 Subject: [PATCH 13/26] feat(cli): remove unused module --- src/command/dev/command.rs | 93 -------------------------------------- 1 file changed, 93 deletions(-) delete mode 100644 src/command/dev/command.rs diff --git a/src/command/dev/command.rs b/src/command/dev/command.rs deleted file mode 100644 index 787444bba..000000000 --- a/src/command/dev/command.rs +++ /dev/null @@ -1,93 +0,0 @@ -use std::{ - io::{BufRead, BufReader}, - process::{Child, Command, Stdio}, -}; - -use anyhow::{anyhow, Context}; -use crossbeam_channel::Sender; - -use crate::{command::dev::do_dev::log_err_and_continue, RoverError, RoverResult}; - -#[derive(Debug)] -pub struct BackgroundTask { - child: Child, -} - -pub enum BackgroundTaskLog { - Stdout(String), - Stderr(String), -} - -impl BackgroundTask { - pub fn new(command: String, log_sender: Sender) -> RoverResult { - let args: Vec<&str> = command.split(' ').collect(); - let (bin, args) = match args.len() { - 0 => Err(anyhow!("the command you passed is empty")), - 1 => Ok((args[0], Vec::new())), - _ => Ok((args[0], Vec::from_iter(args[1..].iter()))), - }?; - tracing::info!("starting `{}`", &command); - if which::which(bin).is_err() { - return Err(anyhow!("{} is not installed on this machine", &bin).into()); - } - - let mut command = Command::new(bin); - command.args(args).env("APOLLO_ROVER", "true"); - - command.stdout(Stdio::piped()).stderr(Stdio::piped()); - - let mut child = command - .spawn() - .with_context(|| "could not spawn child process")?; - - if let Some(stdout) = child.stdout.take() { - let log_sender = log_sender.clone(); - rayon::spawn(move || { - let stdout = BufReader::new(stdout); - stdout.lines().for_each(|line| { - if let Ok(line) = line { - log_sender - .send(BackgroundTaskLog::Stdout(line)) - .expect("could not update stdout logs for command"); - } - }); - }); - } - - if let Some(stderr) = child.stderr.take() { - rayon::spawn(move || { - let stderr = BufReader::new(stderr); - stderr.lines().for_each(|line| { - if let Ok(line) = line { - log_sender - .send(BackgroundTaskLog::Stderr(line)) - .expect("could not update stderr logs for command"); - } - }); - }); - } - - Ok(Self { child }) - } - - pub fn kill(&mut self) { - let pid = self.id(); - tracing::info!("killing child with pid {}", &pid); - let _ = self.child.kill().map_err(|_| { - log_err_and_continue(RoverError::new(anyhow!( - "could not kill child with pid {}", - &pid - ))); - }); - } - - pub fn id(&self) -> u32 { - self.child.id() - } -} - -impl Drop for BackgroundTask { - fn drop(&mut self) { - self.kill() - } -} From 3c6c21df65c49e7f38428b362755da598cb8187b Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Thu, 1 Dec 2022 14:42:38 +0100 Subject: [PATCH 14/26] feat(cli): Add print to file option for output --- src/cli.rs | 21 +++++++++++++++++---- src/command/output.rs | 20 +++++++++++++++++++- src/options/introspect.rs | 2 +- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index 4c051b6e0..e89481a27 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -178,10 +178,23 @@ impl Rover { match rover_output { Ok(output) => { - if self.get_json() { - JsonOutput::from(output).print()? - } else { - output.print()? + match (&self.format_type, &self.output_type) { + // print json data + (None, Some(OutputType::LegacyOutputType(FormatType::Json))) => { + JsonOutput::from(output).print()? + } + (Some(FormatType::Json), None) => JsonOutput::from(output).print()?, + // print to a specified file + (None, Some(OutputType::File(utf_8_path_buf))) => { + output.print_to_file(utf_8_path_buf); + } + // print plain text + (None, None) => output.print()?, + (None, Some(OutputType::LegacyOutputType(FormatType::Plain))) => { + output.print()? + } + (Some(FormatType::Plain), None) => output.print()?, + _ => output.print()?, } process::exit(0); diff --git a/src/command/output.rs b/src/command/output.rs index d4bbace7c..d42b672d6 100644 --- a/src/command/output.rs +++ b/src/command/output.rs @@ -20,7 +20,7 @@ use rover_client::shared::{ CheckRequestSuccessResult, CheckResponse, FetchResponse, GraphRef, SdlType, }; use rover_client::RoverClientError; -use rover_std::Style; +use rover_std::{Fs, Style}; use serde::Serialize; use serde_json::{json, Value}; use termimad::MadSkin; @@ -473,6 +473,24 @@ impl RoverOutput { } } } + + pub(crate) fn print_to_file(&self, path: &Utf8PathBuf) -> Result<(), RoverError> { + let result = self.get_stdout(); + + match result { + Ok(contents) => match contents { + None => (), + Some(content) => { + Fs::write_file(path, content)?; + } + }, + Err(e) => { + tracing::debug!("Unknown error when printing RoverOutput:\nError: {}", e); + let _ = RoverError::new(anyhow!("the router was unable to start up",)); + } + } + Ok(()) + } } fn print_descriptor(descriptor: impl Display) -> io::Result<()> { diff --git a/src/options/introspect.rs b/src/options/introspect.rs index 535ff2129..3b586670d 100644 --- a/src/options/introspect.rs +++ b/src/options/introspect.rs @@ -2,7 +2,7 @@ use clap::Parser; use reqwest::Url; use serde::{Deserialize, Serialize}; -use crate::{utils::parsers::parse_header, RoverOutput, RoverResult, command::output::JsonOutput}; +use crate::{command::output::JsonOutput, utils::parsers::parse_header, RoverOutput, RoverResult}; #[derive(Debug, Serialize, Deserialize, Parser)] pub struct IntrospectOpts { From 13d2c24d36971151119c6ef92fd1781e610264c5 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Thu, 1 Dec 2022 14:47:23 +0100 Subject: [PATCH 15/26] feat(cli): update changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d4222fcbb..d7f2a1a58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,7 +22,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - **Replace the '--output' option type with '--format' - @gocamille, #1413 fixes #1212** - This change adds the new option, `--format`, to allow users to define the format type for messages printed to `stdout` (either by passing `plain` or `json` as an argument to `--format`). This replaces the use of `--output` for defining format types. The `--output` option will be available to define the output file type instead, following [Command Line Interface Guidelines for file outputs](https://clig.dev/#:~:text=%2Do%2C%20%2D%2Doutput%3A%20Output%20file.%20For%20example%2C%20sort%2C%20gcc.). + This change adds the new option, `--format`, to allow users to define the format type for messages printed to `stdout` (either by passing `plain` or `json` as an argument to `--format`). This replaces the use of `--output` for defining format types. The `--output` option will be available to define the output file type instead, following [Command Line Interface Guidelines for file outputs](https://clig.dev/#:~:text=%2Do%2C%20%2D%2Doutput%3A%20Output%20file.%20For%20example%2C%20sort%2C%20gcc.). This is an additive, non-breaking change and using the `--output` option will continue to be valid. # [0.10.0] - 2022-11-10 From ee07e6ee9d44808694dca90628802311efcab161 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Mon, 9 Jan 2023 01:46:18 +0100 Subject: [PATCH 16/26] feat(cli): Add logic to deprecate --output and print to file --- src/bin/rover.rs | 4 +- src/cli.rs | 84 ++++-------------- src/command/output.rs | 105 +++++------------------ src/options/introspect.rs | 2 +- src/options/output.rs | 175 +++++++++++++++++++++++++++++++++++++- 5 files changed, 215 insertions(+), 155 deletions(-) diff --git a/src/bin/rover.rs b/src/bin/rover.rs index c364f6834..40ead2101 100644 --- a/src/bin/rover.rs +++ b/src/bin/rover.rs @@ -2,7 +2,7 @@ use robot_panic::setup_panic; use rover::cli::Rover; #[calm_io::pipefail] -fn main() -> std::io::Result<()> { +fn main() -> Result<_, std::io::Error> { setup_panic!(Metadata { name: rover::PKG_NAME.into(), version: rover::PKG_VERSION.into(), @@ -10,5 +10,5 @@ fn main() -> std::io::Result<()> { homepage: rover::PKG_HOMEPAGE.into(), repository: rover::PKG_REPOSITORY.into() }); - Rover::run_from_args() + Ok(Rover::run_from_args()) } diff --git a/src/cli.rs b/src/cli.rs index e89481a27..1452a9399 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -1,12 +1,11 @@ use camino::Utf8PathBuf; -use clap::{error::ErrorKind as ClapErrorKind, Parser, ValueEnum}; +use clap::{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::options::OutputStrategy; use crate::utils::{ client::{ClientBuilder, ClientTimeout, StudioClientConfig}, env::{RoverEnv, RoverEnvKey}, @@ -15,11 +14,9 @@ use crate::utils::{ }; use crate::RoverResult; -use clap::CommandFactory; use config::Config; use houston as config; use rover_client::shared::GitContext; -use rover_std::Style; use sputnik::Session; use timber::Level; @@ -64,13 +61,8 @@ pub struct Rover { #[serde(serialize_with = "option_from_display")] log_level: Option, - /// Specify Rover's format type - #[arg(long = "format", global = true)] - format_type: Option, - - /// Specify a file to write Rover's output to - #[arg(long = "output", global = true)] - output_type: Option, + #[clap(flatten)] + output_strategy: OutputStrategy, /// Accept invalid certificates when performing HTTPS requests. /// @@ -117,32 +109,14 @@ pub struct Rover { } impl Rover { - pub fn run_from_args() -> io::Result<()> { + pub fn run_from_args() -> RoverResult<()> { 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(); - } - (None, Some(OutputType::LegacyOutputType(_))) => { - let warn_prefix = Style::WarningPrefix.paint("WARN:"); - eprintln!("{} The argument '--output' will soon be deprecated. Please use the '--format' argument to specify the output type.", warn_prefix); - } - _ => (), - } - } - - pub fn run(&self) -> io::Result<()> { + pub fn run(&self) -> RoverResult<()> { timber::init(self.log_level); tracing::trace!(command_structure = ?self); - self.validate_options(); + self.output_strategy.validate_options(); // attempt to create a new `Session` to capture anonymous usage data let rover_output = match Session::new(self) { @@ -178,34 +152,12 @@ impl Rover { match rover_output { Ok(output) => { - match (&self.format_type, &self.output_type) { - // print json data - (None, Some(OutputType::LegacyOutputType(FormatType::Json))) => { - JsonOutput::from(output).print()? - } - (Some(FormatType::Json), None) => JsonOutput::from(output).print()?, - // print to a specified file - (None, Some(OutputType::File(utf_8_path_buf))) => { - output.print_to_file(utf_8_path_buf); - } - // print plain text - (None, None) => output.print()?, - (None, Some(OutputType::LegacyOutputType(FormatType::Plain))) => { - output.print()? - } - (Some(FormatType::Plain), None) => output.print()?, - _ => output.print()?, - } + self.output_strategy.write_rover_output(output)?; process::exit(0); } Err(error) => { - if self.get_json() { - JsonOutput::from(error).print()? - } else { - tracing::debug!(?error); - error.print()? - } + self.output_strategy.write_rover_output(error)?; process::exit(1); } @@ -240,7 +192,7 @@ impl Rover { self.get_client_config()?, self.get_git_context()?, self.get_checks_timeout_seconds()?, - self.get_json(), + self.output_strategy.get_json(), ), Command::Template(command) => command.run(self.get_client_config()?), Command::Readme(command) => command.run(self.get_client_config()?), @@ -248,7 +200,7 @@ impl Rover { self.get_client_config()?, self.get_git_context()?, self.get_checks_timeout_seconds()?, - self.get_json(), + self.output_strategy.get_json(), ), Command::Update(command) => { command.run(self.get_rover_config()?, self.get_reqwest_client()?) @@ -261,14 +213,6 @@ impl Rover { } } - pub(crate) fn get_json(&self) -> bool { - matches!(self.format_type, Some(FormatType::Json)) - || matches!( - self.output_type, - Some(OutputType::LegacyOutputType(FormatType::Json)) - ) - } - pub(crate) fn get_rover_config(&self) -> RoverResult { let override_home: Option = self .get_env_var(RoverEnvKey::ConfigHome)? @@ -461,6 +405,12 @@ pub enum FormatType { Json, } +#[derive(ValueEnum, Debug, Serialize, Clone, Eq, PartialEq)] +pub enum RoverFormatType { + RoverOutput, + RoverError, +} + impl Default for FormatType { fn default() -> Self { FormatType::Plain diff --git a/src/command/output.rs b/src/command/output.rs index d42b672d6..aab691101 100644 --- a/src/command/output.rs +++ b/src/command/output.rs @@ -7,7 +7,6 @@ use crate::utils::table::{self, row}; use crate::RoverError; use crate::options::GithubTemplate; -use anyhow::anyhow; use atty::Stream; use calm_io::{stderr, stderrln, stdoutln}; use camino::Utf8PathBuf; @@ -20,7 +19,7 @@ use rover_client::shared::{ CheckRequestSuccessResult, CheckResponse, FetchResponse, GraphRef, SdlType, }; use rover_client::RoverClientError; -use rover_std::{Fs, Style}; +use rover_std::Style; use serde::Serialize; use serde_json::{json, Value}; use termimad::MadSkin; @@ -97,9 +96,9 @@ impl RoverOutput { } RoverOutput::FetchResponse(fetch_response) => { match fetch_response.sdl.r#type { - SdlType::Graph | SdlType::Subgraph { .. } => print_descriptor("SDL")?, - SdlType::Supergraph => print_descriptor("Supergraph SDL")?, - } + SdlType::Graph | SdlType::Subgraph { .. } => self.print_descriptor("SDL"), + SdlType::Supergraph => self.print_descriptor("Supergraph SDL"), + }?; Some((fetch_response.sdl.contents).to_string()) } RoverOutput::GraphPublishResponse { @@ -112,7 +111,7 @@ impl RoverOutput { publish_response.api_schema_hash, publish_response.change_summary )?; - print_one_line_descriptor("Schema Hash")?; + self.print_one_line_descriptor("Schema Hash")?; Some((publish_response.api_schema_hash).to_string()) } RoverOutput::SubgraphPublishResponse { @@ -170,7 +169,7 @@ impl RoverOutput { stderrln!("{} This is only a prediction. If the graph changes before confirming, these errors could change.", warn_prefix)?; } else { stderrln!("{} At the time of checking, there would be no build errors resulting from the deletion of this subgraph.", warn_prefix)?; - stderrln!("{} This is only a prediction. If the graph changes before confirming, there could be build errors.", warn_prefix)? + stderrln!("{} This is only a prediction. If the graph changes before confirming, there could be build errors.", warn_prefix)?; } None } else { @@ -179,13 +178,13 @@ impl RoverOutput { "The '{}' subgraph was removed from '{}'. The remaining subgraphs were composed.", Style::Link.paint(subgraph), Style::Link.paint(graph_ref.to_string()), - )? + )?; } else { stderrln!( "{} The supergraph schema for '{}' was not updated. See errors below.", warn_prefix, Style::Link.paint(graph_ref.to_string()) - )? + )?; } if !delete_response.build_errors.is_empty() { @@ -201,10 +200,7 @@ impl RoverOutput { None } } - RoverOutput::CoreSchema(csdl) => { - print_descriptor("CoreSchema")?; - Some((csdl).to_string()) - } + RoverOutput::CoreSchema(csdl) => Some((csdl).to_string()), RoverOutput::CompositionResult(composition_output) => { let warn_prefix = Style::HintPrefix.paint("HINT:"); @@ -216,7 +212,6 @@ impl RoverOutput { stderrln!("{}", hints_string)?; - print_descriptor("CoreSchema")?; Some((composition_output.supergraph_sdl).to_string()) } RoverOutput::SubgraphList(details) => { @@ -267,7 +262,6 @@ impl RoverOutput { Some(format!("{}", table)) } RoverOutput::TemplateUseSuccess { template, path } => { - print_descriptor("Project generated")?; let template_id = Style::Command.paint(template.id); let path = Style::Path.paint(path.as_str()); let readme = Style::Path.paint("README.md"); @@ -281,27 +275,18 @@ impl RoverOutput { readme, forum_call_to_action)) } - RoverOutput::CheckResponse(check_response) => { - print_descriptor("Check Result")?; - Some(check_response.get_table()) - } - RoverOutput::AsyncCheckResponse(check_response) => { - print_descriptor("Check Started")?; - Some(format!( - "Check successfully started with workflow ID: {}/nView full details at {}", - check_response.workflow_id, check_response.target_url - )) - } + RoverOutput::CheckResponse(check_response) => Some(check_response.get_table()), + RoverOutput::AsyncCheckResponse(check_response) => 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")?; } Some(profiles.join("\n")) } RoverOutput::Introspection(introspection_response) => { - print_descriptor("Introspection Response")?; Some((introspection_response).to_string()) } RoverOutput::ErrorExplanation(explanation) => { @@ -315,10 +300,7 @@ impl RoverOutput { graph_ref: _, content, last_updated_time: _, - } => { - print_descriptor("Readme")?; - Some((content).to_string()) - } + } => Some((content).to_string()), RoverOutput::ReadmePublishResponse { graph_ref, new_content: _, @@ -449,63 +431,20 @@ impl RoverOutput { JsonVersion::default() } - pub(crate) fn print(&self) -> io::Result<()> { - let result = self.get_stdout(); - - match result { - Ok(data) => match data { - Some(data) => { - stdoutln!("{}", format!("{}", data)) - } - None => Ok(()), - }, - Err(e) => { - if let Some(raw_os_err) = e.raw_os_error() { - tracing::debug!( - "Unknown error when formatting RoverOutput:\nError: {}\n{}", - e, - raw_os_err - ); - Ok(()) - } else { - Err(io::Error::new(io::ErrorKind::Other, anyhow!("{}", e))) - } - } + pub(crate) fn print_descriptor(&self, descriptor: impl Display) -> io::Result<()> { + if atty::is(Stream::Stdout) { + stderrln!("{}: \n", Style::Heading.paint(descriptor.to_string()))?; } + Ok(()) } - - pub(crate) fn print_to_file(&self, path: &Utf8PathBuf) -> Result<(), RoverError> { - let result = self.get_stdout(); - - match result { - Ok(contents) => match contents { - None => (), - Some(content) => { - Fs::write_file(path, content)?; - } - }, - Err(e) => { - tracing::debug!("Unknown error when printing RoverOutput:\nError: {}", e); - let _ = RoverError::new(anyhow!("the router was unable to start up",)); - } + pub(crate) fn print_one_line_descriptor(&self, descriptor: impl Display) -> io::Result<()> { + if atty::is(Stream::Stdout) { + stderr!("{}: ", Style::Heading.paint(descriptor.to_string()))?; } Ok(()) } } -fn print_descriptor(descriptor: impl Display) -> io::Result<()> { - if atty::is(Stream::Stdout) { - stderrln!("{}: \n", Style::Heading.paint(descriptor.to_string()))?; - } - Ok(()) -} -fn print_one_line_descriptor(descriptor: impl Display) -> io::Result<()> { - if atty::is(Stream::Stdout) { - stderr!("{}: ", Style::Heading.paint(descriptor.to_string()))?; - } - Ok(()) -} - #[derive(Debug, Clone, Serialize)] pub(crate) struct JsonOutput { json_version: JsonVersion, diff --git a/src/options/introspect.rs b/src/options/introspect.rs index 3b586670d..b42a215cd 100644 --- a/src/options/introspect.rs +++ b/src/options/introspect.rs @@ -46,7 +46,7 @@ impl IntrospectOpts { if json { let _ = JsonOutput::from(output).print(); } else { - let _ = output.print(); + let _ = output.get_stdout(); } } last_result = Some(sdl); diff --git a/src/options/output.rs b/src/options/output.rs index 00bfd42a7..8b370aed7 100644 --- a/src/options/output.rs +++ b/src/options/output.rs @@ -1,10 +1,18 @@ use std::str::FromStr; +use anyhow::Result; +use calm_io::{stderrln, stdoutln}; use camino::Utf8PathBuf; -use clap::{Parser, ValueEnum}; +use clap::{error::ErrorKind as ClapErrorKind, CommandFactory, Parser, ValueEnum}; +use rover_client::shared::SdlType; +use rover_std::{Fs, Style}; use serde::Serialize; -use crate::cli::FormatType; +use crate::{ + cli::{FormatType, Rover}, + command::output::JsonOutput, + RoverError, RoverOutput, RoverResult, +}; #[derive(Debug, Parser)] pub struct Output { @@ -13,6 +21,12 @@ pub struct Output { output: OutputType, } +#[derive(Debug, Clone, Eq, PartialEq, Serialize)] +pub enum FinalOutputType { + File(Utf8PathBuf), + Stdout, +} + #[derive(Debug, Clone, Eq, PartialEq, Serialize)] pub enum OutputType { LegacyOutputType(FormatType), @@ -22,6 +36,7 @@ pub enum OutputType { impl FromStr for OutputType { type Err = anyhow::Error; + // TODO: HANDLE THE TRANSFORMATION OF THE JSON LEGACY FORMAT TYPE fn from_str(s: &str) -> Result { if let Ok(format_type) = FormatType::from_str(s, true) { Ok(Self::LegacyOutputType(format_type)) @@ -30,3 +45,159 @@ impl FromStr for OutputType { } } } + +pub trait RoverOutputTrait { + fn write_or_print( + self, + format_type: FormatType, + output_type: FinalOutputType, + ) -> RoverResult<()>; +} + +impl RoverOutputTrait for RoverOutput { + fn write_or_print( + self, + format_type: FormatType, + output_type: FinalOutputType, + ) -> RoverResult<()> { + // Format the RoverOutput as either plain text or JSON. + let output = match format_type { + FormatType::Plain => self.get_stdout(), + FormatType::Json => Ok(Some(JsonOutput::from(self.clone()).to_string())), + }; + + // Print the RoverOutput to file or stdout. + if let Ok(Some(result)) = output { + match output_type { + FinalOutputType::File(path) => { + let success_heading = Style::Heading.paint("The output was printed to "); + let path_text = Style::Path.paint(&path); + Fs::write_file(&path, result)?; + stderrln!("{} {}", success_heading, path_text)?; + } + FinalOutputType::Stdout => { + // Call the appropriate method based on the variant of RoverOutput. + let descriptor = match &self { + RoverOutput::FetchResponse(fetch_response) => { + match fetch_response.sdl.r#type { + SdlType::Graph | SdlType::Subgraph { .. } => "SDL", + SdlType::Supergraph => "Supergraph SDL", + } + } + RoverOutput::CoreSchema(_) => "CoreSchema", + RoverOutput::CompositionResult(_) => "CoreSchema", + RoverOutput::TemplateUseSuccess { .. } => "Project generated", + RoverOutput::CheckResponse(_) => "Check Result", + RoverOutput::AsyncCheckResponse(_) => "Check Started", + RoverOutput::Profiles(_) => "Profiles", + RoverOutput::Introspection(_) => "Introspection Response", + RoverOutput::ReadmeFetchResponse { .. } => "Readme", + RoverOutput::GraphPublishResponse { .. } => "Schema Hash", + _ => return Ok(()), + }; + if let RoverOutput::GraphPublishResponse { .. } = self { + self.print_one_line_descriptor(descriptor)?; + } else { + self.print_descriptor(descriptor)?; + } + + stdoutln!("{}", &result)?; + } + } + } + + Ok(()) + } +} + +impl RoverOutputTrait for RoverError { + fn write_or_print(self, format_type: FormatType, _: FinalOutputType) -> RoverResult<()> { + match format_type { + FormatType::Plain => self.print(), + FormatType::Json => JsonOutput::from(self).print(), + }?; + + Ok(()) + } +} + +#[derive(Debug, Parser, Serialize)] +pub struct OutputStrategy { + /// Specify Rover's format type + #[arg(long = "format", global = true)] + format_type: Option, + + /// Specify a file to write Rover's output to + #[arg(long = "output", global = true)] + output_type: Option, +} + +impl OutputStrategy { + 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(); + } + (None, Some(OutputType::LegacyOutputType(_))) => { + let warn_prefix = Style::WarningPrefix.paint("WARN:"); + eprintln!("{} The argument '--output' will soon be deprecated. Please use the '--format' argument to specify the output type.", warn_prefix); + } + _ => (), + } + } + + pub fn get_strategy(&self) -> (FormatType, FinalOutputType) { + let output_type = self.output_type.clone(); + + match (&self.format_type, output_type) { + (None, None) | (None, Some(OutputType::LegacyOutputType(FormatType::Plain))) => { + (FormatType::Plain, FinalOutputType::Stdout) + } + (None, Some(OutputType::LegacyOutputType(FormatType::Json))) => { + (FormatType::Json, FinalOutputType::Stdout) + } + (None, Some(OutputType::File(path))) => { + (FormatType::Plain, FinalOutputType::File(path)) + } + (Some(FormatType::Plain), None) + | (Some(FormatType::Plain), Some(OutputType::LegacyOutputType(FormatType::Plain))) => { + (FormatType::Plain, FinalOutputType::Stdout) + } + (Some(FormatType::Plain), Some(OutputType::LegacyOutputType(FormatType::Json))) => { + (FormatType::Json, FinalOutputType::Stdout) + } + (Some(FormatType::Plain), Some(OutputType::File(path))) => { + (FormatType::Plain, FinalOutputType::File(path)) + } + (Some(FormatType::Json), None) + | (Some(FormatType::Json), Some(OutputType::LegacyOutputType(_))) => { + (FormatType::Json, FinalOutputType::Stdout) + } + (Some(FormatType::Json), Some(OutputType::File(path))) => { + (FormatType::Json, FinalOutputType::File(path)) + } + } + } + + pub fn write_rover_output(&self, output_trait: T) -> Result<(), RoverError> + where + T: RoverOutputTrait, + { + let (format_type, output_type) = self.get_strategy(); + output_trait.write_or_print(format_type, output_type)?; + Ok(()) + } + + pub fn get_json(&self) -> bool { + matches!(self.format_type, Some(FormatType::Json)) + || matches!( + self.output_type, + Some(OutputType::LegacyOutputType(FormatType::Json)) + ) + } +} From a0f4dd69b4b56eb56d226ee382dcd31ce240c7b3 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Tue, 10 Jan 2023 12:39:32 +0100 Subject: [PATCH 17/26] feat(cli): Add emoji to file output message --- crates/rover-std/src/emoji.rs | 2 ++ src/options/output.rs | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/crates/rover-std/src/emoji.rs b/crates/rover-std/src/emoji.rs index 8b75169db..cf000c12b 100644 --- a/crates/rover-std/src/emoji.rs +++ b/crates/rover-std/src/emoji.rs @@ -20,6 +20,7 @@ pub enum Emoji { Skull, Compose, Warn, + Memo, } impl Emoji { @@ -42,6 +43,7 @@ impl Emoji { Skull => "💀 ", Compose => "🎶 ", Warn => "⚠️ ", + Memo => "📝 ", } } } diff --git a/src/options/output.rs b/src/options/output.rs index 8b370aed7..a21f619b0 100644 --- a/src/options/output.rs +++ b/src/options/output.rs @@ -5,7 +5,7 @@ use calm_io::{stderrln, stdoutln}; use camino::Utf8PathBuf; use clap::{error::ErrorKind as ClapErrorKind, CommandFactory, Parser, ValueEnum}; use rover_client::shared::SdlType; -use rover_std::{Fs, Style}; +use rover_std::{Fs, Style, Emoji}; use serde::Serialize; use crate::{ @@ -70,7 +70,7 @@ impl RoverOutputTrait for RoverOutput { if let Ok(Some(result)) = output { match output_type { FinalOutputType::File(path) => { - let success_heading = Style::Heading.paint("The output was printed to "); + let success_heading = Style::Heading.paint(format!("{}The output was printed to", Emoji::Memo)); let path_text = Style::Path.paint(&path); Fs::write_file(&path, result)?; stderrln!("{} {}", success_heading, path_text)?; From d39d9370e75439bdc8ef3724da2967f0c37dec52 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Tue, 10 Jan 2023 12:40:52 +0100 Subject: [PATCH 18/26] feat(cli): remove unneeded comments and imports --- src/command/output.rs | 7 +------ src/options/output.rs | 6 +++--- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/command/output.rs b/src/command/output.rs index aab691101..18f34bd1c 100644 --- a/src/command/output.rs +++ b/src/command/output.rs @@ -16,7 +16,7 @@ use rover_client::operations::subgraph::delete::SubgraphDeleteResponse; use rover_client::operations::subgraph::list::SubgraphListResponse; use rover_client::operations::subgraph::publish::SubgraphPublishResponse; use rover_client::shared::{ - CheckRequestSuccessResult, CheckResponse, FetchResponse, GraphRef, SdlType, + CheckRequestSuccessResult, CheckResponse, FetchResponse, GraphRef, }; use rover_client::RoverClientError; use rover_std::Style; @@ -95,10 +95,6 @@ impl RoverOutput { Some(format!("{}", table)) } RoverOutput::FetchResponse(fetch_response) => { - match fetch_response.sdl.r#type { - SdlType::Graph | SdlType::Subgraph { .. } => self.print_descriptor("SDL"), - SdlType::Supergraph => self.print_descriptor("Supergraph SDL"), - }?; Some((fetch_response.sdl.contents).to_string()) } RoverOutput::GraphPublishResponse { @@ -111,7 +107,6 @@ impl RoverOutput { publish_response.api_schema_hash, publish_response.change_summary )?; - self.print_one_line_descriptor("Schema Hash")?; Some((publish_response.api_schema_hash).to_string()) } RoverOutput::SubgraphPublishResponse { diff --git a/src/options/output.rs b/src/options/output.rs index a21f619b0..bffa9451f 100644 --- a/src/options/output.rs +++ b/src/options/output.rs @@ -36,7 +36,6 @@ pub enum OutputType { impl FromStr for OutputType { type Err = anyhow::Error; - // TODO: HANDLE THE TRANSFORMATION OF THE JSON LEGACY FORMAT TYPE fn from_str(s: &str) -> Result { if let Ok(format_type) = FormatType::from_str(s, true) { Ok(Self::LegacyOutputType(format_type)) @@ -128,7 +127,7 @@ pub struct OutputStrategy { format_type: Option, /// Specify a file to write Rover's output to - #[arg(long = "output", global = true)] + #[arg(long = "output", short= 'o', global = true)] output_type: Option, } @@ -145,8 +144,9 @@ impl OutputStrategy { } (None, Some(OutputType::LegacyOutputType(_))) => { let warn_prefix = Style::WarningPrefix.paint("WARN:"); - eprintln!("{} The argument '--output' will soon be deprecated. Please use the '--format' argument to specify the output type.", warn_prefix); + eprintln!("{} The argument for specifying the format of Rover's output has been renamed from '--output' to '--format'. Please use the '--format' argument to specify the output format instead of '--output'. Future versions of Rover may be incompatible with this usage.", warn_prefix); } + // there are default options, so if nothing is passed, print no errors or warnings _ => (), } } From d05f80882ee367b0b9f0b200b6d183d6f25cbff6 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Tue, 10 Jan 2023 12:41:19 +0100 Subject: [PATCH 19/26] feat(cli): rename RoverOutputTrait --- src/options/output.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/options/output.rs b/src/options/output.rs index bffa9451f..089fdb268 100644 --- a/src/options/output.rs +++ b/src/options/output.rs @@ -45,7 +45,7 @@ impl FromStr for OutputType { } } -pub trait RoverOutputTrait { +pub trait RoverPrinter { fn write_or_print( self, format_type: FormatType, @@ -53,7 +53,7 @@ pub trait RoverOutputTrait { ) -> RoverResult<()>; } -impl RoverOutputTrait for RoverOutput { +impl RoverPrinter for RoverOutput { fn write_or_print( self, format_type: FormatType, @@ -109,7 +109,7 @@ impl RoverOutputTrait for RoverOutput { } } -impl RoverOutputTrait for RoverError { +impl RoverPrinter for RoverError { fn write_or_print(self, format_type: FormatType, _: FinalOutputType) -> RoverResult<()> { match format_type { FormatType::Plain => self.print(), @@ -186,7 +186,7 @@ impl OutputStrategy { pub fn write_rover_output(&self, output_trait: T) -> Result<(), RoverError> where - T: RoverOutputTrait, + T: RoverPrinter, { let (format_type, output_type) = self.get_strategy(); output_trait.write_or_print(format_type, output_type)?; From f099b32736803357d6f65eda0ff7d44a71021541 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Tue, 10 Jan 2023 23:35:31 +0100 Subject: [PATCH 20/26] feat(cli): update output documentation to include --output deprecation --- docs/source/configuring.md | 32 +++++++++++++++++++++++++++----- 1 file changed, 27 insertions(+), 5 deletions(-) diff --git a/docs/source/configuring.md b/docs/source/configuring.md index ce242d4df..2df0cdbe6 100644 --- a/docs/source/configuring.md +++ b/docs/source/configuring.md @@ -76,15 +76,17 @@ If Rover log messages are unhelpful or unclear, please leave us feedback in an ## Output format -### `--output plain` (default) +### `--format plain` (default) By default, Rover prints the main output of its commands to `stdout` in plaintext. It also prints a _descriptor_ for that output to `stderr` if it thinks it's being operated by a human (it checks whether the terminal is TTY). -> For more on `stdout`, see [Conventions](./conventions/#using-stdout). +Note: The `--output` option has been deprecated in favor of `--format`. The `--output` option can still be used, but it will be removed in a future release. The `--format` option offers the same functionality as `--output`, but it aligns with industry conventions and is easier to understand. -### `--output json` +For more on `stdout`, see [Conventions](https://chat.openai.com/conventions/#using-stdout). -For more programmatic control over Rover's output, you can pass `--output json` to any command. Rover JSON output has the following minimal structure: +### `--format json` + +For more programmatic control over Rover's output, you can pass `--format json` to any command. Rover JSON output has the following minimal structure: ```json title="success_example" { @@ -232,7 +234,27 @@ This particular `error` object includes `details` about what went wrong. Notice #### Example `jq` script -You can combine the `--output json` flag with the [`jq`](https://stedolan.github.io/jq/) command line tool to create powerful custom workflows. For example, [this gist](https://gist.github.com/EverlastingBugstopper/d6aa0d9a49bcf39f2df53e1cfb9bb88a) demonstrates converting output from `rover {sub}graph check my-graph --output json` to Markdown. +You can combine the `--format json` flag with the [`jq`](https://stedolan.github.io/jq/) command line tool to create powerful custom workflows. For example, [this gist](https://gist.github.com/EverlastingBugstopper/d6aa0d9a49bcf39f2df53e1cfb9bb88a) demonstrates converting output from `rover {sub}graph check my-graph --format json` to Markdown. + +## `--output` option + +The `--output` option allows you to specify the format and destination of the command's output. It has three options: + +### `--output plain` (default) + +This option is the default behavior of the command. It prints the main output of its commands to `stdout` in plaintext. It also prints a descriptor for that output to `stderr` if it thinks it's being operated by a human (it checks whether the terminal is TTY). This option is equivalent to `--format plain`. + +### `--output json` + +This option will output the command result to `stdout` in JSON format. This output format is similar to `--format json`, with a minimal structure, including `json_version`, `data`, and `error`. + +### `--output ` + +This option allows you to specify a file name as an argument. It will save the command output to the specified file in plaintext format. + +Please note that if the file already exists, it will be overwritten. + +Note: The `--output` option has been deprecated in favor of `--format`, but it can still be used, however, it will be removed in a future release. The `--format` option offers the same functionality as `--output`, but it aligns with industry conventions and is easier to understand. ## Setting config storage location From ca454495ba801f966bf0d4c3202ed0f34e310373 Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Tue, 10 Jan 2023 23:36:25 +0100 Subject: [PATCH 21/26] feat(cli): re-include SdlType crate --- src/command/output.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/command/output.rs b/src/command/output.rs index 18f34bd1c..cebb19258 100644 --- a/src/command/output.rs +++ b/src/command/output.rs @@ -540,7 +540,7 @@ mod tests { list::{SubgraphInfo, SubgraphUpdatedAt}, }, }, - shared::{ChangeSeverity, SchemaChange, Sdl}, + shared::{ChangeSeverity, SchemaChange, Sdl, SdlType}, }; use apollo_federation_types::build::{BuildError, BuildErrors}; From 1c8611e8f3d3be609776318d94924140396cfaaf Mon Sep 17 00:00:00 2001 From: Camille Lawrence Date: Tue, 10 Jan 2023 23:45:11 +0100 Subject: [PATCH 22/26] feat(cli): change doc references from to --- README.md | 8 +++++++- docs/source/commands/readmes.mdx | 4 ++-- docs/source/migration.md | 4 ++-- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 1c7003de7..fa43e246c 100644 --- a/README.md +++ b/README.md @@ -69,11 +69,17 @@ Options: -l, --log Specify Rover's log level + --format + Specify Rover's format type + + [default: plain] + [possible values: plain, json] + --output Specify Rover's output type [default: plain] - [possible values: plain, json] + [possible values: plain, json, filename] --insecure-accept-invalid-certs Accept invalid certificates when performing HTTPS requests. diff --git a/docs/source/commands/readmes.mdx b/docs/source/commands/readmes.mdx index 5250d70b8..3e45c1e3e 100644 --- a/docs/source/commands/readmes.mdx +++ b/docs/source/commands/readmes.mdx @@ -32,10 +32,10 @@ By default, the README is output to `stdout`. You can also save the output to a rover readme fetch my-graph@my-variant > README.md ``` -You can also request the output as JSON with the `--output json` option: +You can also request the output as JSON with the `--format json` option: ```bash -rover readme fetch my-graph@my-variant --output json +rover readme fetch my-graph@my-variant --format json ``` > For more on passing values via `stdout`, see [Conventions](../conventions#using-stdout). diff --git a/docs/source/migration.md b/docs/source/migration.md index 43a63ea60..d4df83a8e 100644 --- a/docs/source/migration.md +++ b/docs/source/migration.md @@ -144,9 +144,9 @@ As a workaround, you might be able to use `cat` to combine multiple files and pa ### Machine-readable output -In the Apollo CLI, many commands support alternate output formatting options, such as `--json` and `--markdown`. Currently, Rover only supports `--output json`, and leaves markdown formatting up to the consumer. For more information on JSON output in Rover, see [these docs](./configuring/#--output-json). +In the Apollo CLI, many commands support alternate output formatting options, such as `--json` and `--markdown`. Currently, Rover only supports `--format json`, and leaves markdown formatting up to the consumer. For more information on JSON output in Rover, see [these docs](./configuring/#--output-json). -An example script for converting output from `rover {sub}graph check my-graph --output json` can be found in [this gist](https://gist.github.com/EverlastingBugstopper/d6aa0d9a49bcf39f2df53e1cfb9bb88a). +An example script for converting output from `rover {sub}graph check my-graph --format json` can be found in [this gist](https://gist.github.com/EverlastingBugstopper/d6aa0d9a49bcf39f2df53e1cfb9bb88a). ## Examples From d66b79ae53c1f64e5e86087b7eb7d3f37c1984b2 Mon Sep 17 00:00:00 2001 From: Stephen Barlow Date: Tue, 10 Jan 2023 15:49:38 -0800 Subject: [PATCH 23/26] Edits to new docs around --format --- docs/source/configuring.md | 41 +++++++++++++++++++------------------- docs/source/migration.md | 2 +- 2 files changed, 21 insertions(+), 22 deletions(-) diff --git a/docs/source/configuring.md b/docs/source/configuring.md index 2df0cdbe6..892660c34 100644 --- a/docs/source/configuring.md +++ b/docs/source/configuring.md @@ -74,17 +74,22 @@ rover graph check my-graph@prod --schema ./schema.graphql --log debug If Rover log messages are unhelpful or unclear, please leave us feedback in an [issue on GitHub](https://github.com/apollographql/rover/issues/new/choose)! -## Output format - -### `--format plain` (default) +## Configuring output By default, Rover prints the main output of its commands to `stdout` in plaintext. It also prints a _descriptor_ for that output to `stderr` if it thinks it's being operated by a human (it checks whether the terminal is TTY). -Note: The `--output` option has been deprecated in favor of `--format`. The `--output` option can still be used, but it will be removed in a future release. The `--format` option offers the same functionality as `--output`, but it aligns with industry conventions and is easier to understand. +> For more on `stdout`, see [Conventions](https://chat.openai.com/conventions/#using-stdout). + +Every Rover command supports two options for configuring its output behavior: + +- `--format`, for [setting the output format](#setting-output-format) (`plain` or `json`) +- `--output`, for [writing a command's output to a file](#setting-output-location) instead of `stdout` -For more on `stdout`, see [Conventions](https://chat.openai.com/conventions/#using-stdout). +### JSON output -### `--format json` +> **Note:** The `--format` option was added in Rover v0.10.1. Earlier versions of Rover use the `--output` option to set output format. +> +> Current versions of Rover still support using `--output` this way, but that support is deprecated and will be removed in a future release. For more programmatic control over Rover's output, you can pass `--format json` to any command. Rover JSON output has the following minimal structure: @@ -236,25 +241,19 @@ This particular `error` object includes `details` about what went wrong. Notice You can combine the `--format json` flag with the [`jq`](https://stedolan.github.io/jq/) command line tool to create powerful custom workflows. For example, [this gist](https://gist.github.com/EverlastingBugstopper/d6aa0d9a49bcf39f2df53e1cfb9bb88a) demonstrates converting output from `rover {sub}graph check my-graph --format json` to Markdown. -## `--output` option - -The `--output` option allows you to specify the format and destination of the command's output. It has three options: - -### `--output plain` (default) +### Writing to a file -This option is the default behavior of the command. It prints the main output of its commands to `stdout` in plaintext. It also prints a descriptor for that output to `stderr` if it thinks it's being operated by a human (it checks whether the terminal is TTY). This option is equivalent to `--format plain`. +The `--output` option enables you to specify a file destination for writing a Rover command's output: -### `--output json` - -This option will output the command result to `stdout` in JSON format. This output format is similar to `--format json`, with a minimal structure, including `json_version`, `data`, and `error`. - -### `--output ` - -This option allows you to specify a file name as an argument. It will save the command output to the specified file in plaintext format. +```bash +rover supergraph compose --output ./supergraph-schema.graphql --config ./supergraph.yaml +``` -Please note that if the file already exists, it will be overwritten. +If the specified file already exists, Rover overwrites it. -Note: The `--output` option has been deprecated in favor of `--format`, but it can still be used, however, it will be removed in a future release. The `--format` option offers the same functionality as `--output`, but it aligns with industry conventions and is easier to understand. +> **Note:** This functionality is available in Rover v0.10.1 and later. In _earlier_ versions of Rover, the `--output` option instead provides the functionality that's now provided by the [`--format` option](#json-output). +> +> Current versions of Rover still support using `--output` like `--format`, but that support is deprecated and will be removed in a future release. ## Setting config storage location diff --git a/docs/source/migration.md b/docs/source/migration.md index d4df83a8e..d2698c2f8 100644 --- a/docs/source/migration.md +++ b/docs/source/migration.md @@ -144,7 +144,7 @@ As a workaround, you might be able to use `cat` to combine multiple files and pa ### Machine-readable output -In the Apollo CLI, many commands support alternate output formatting options, such as `--json` and `--markdown`. Currently, Rover only supports `--format json`, and leaves markdown formatting up to the consumer. For more information on JSON output in Rover, see [these docs](./configuring/#--output-json). +In the Apollo CLI, many commands support alternate output formatting options, such as `--json` and `--markdown`. Currently, Rover only supports `--format json`, and leaves markdown formatting up to the consumer. For more information on JSON output in Rover, see [these docs](./configuring/#json-output). An example script for converting output from `rover {sub}graph check my-graph --format json` can be found in [this gist](https://gist.github.com/EverlastingBugstopper/d6aa0d9a49bcf39f2df53e1cfb9bb88a). From b378556457f98f479e298822061f2086e406df94 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Wed, 18 Jan 2023 10:12:51 -0600 Subject: [PATCH 24/26] chore: lints and version update in docs --- docs/source/configuring.md | 4 ++-- src/command/output.rs | 4 +--- src/options/output.rs | 7 ++++--- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/docs/source/configuring.md b/docs/source/configuring.md index 892660c34..22957cd0c 100644 --- a/docs/source/configuring.md +++ b/docs/source/configuring.md @@ -87,7 +87,7 @@ Every Rover command supports two options for configuring its output behavior: ### JSON output -> **Note:** The `--format` option was added in Rover v0.10.1. Earlier versions of Rover use the `--output` option to set output format. +> **Note:** The `--format` option was added in Rover v0.11.0. Earlier versions of Rover use the `--output` option to set output format. > > Current versions of Rover still support using `--output` this way, but that support is deprecated and will be removed in a future release. @@ -251,7 +251,7 @@ rover supergraph compose --output ./supergraph-schema.graphql --config ./supergr If the specified file already exists, Rover overwrites it. -> **Note:** This functionality is available in Rover v0.10.1 and later. In _earlier_ versions of Rover, the `--output` option instead provides the functionality that's now provided by the [`--format` option](#json-output). +> **Note:** This functionality is available in Rover v0.11.0 and later. In _earlier_ versions of Rover, the `--output` option instead provides the functionality that's now provided by the [`--format` option](#json-output). > > Current versions of Rover still support using `--output` like `--format`, but that support is deprecated and will be removed in a future release. diff --git a/src/command/output.rs b/src/command/output.rs index cebb19258..080a1409e 100644 --- a/src/command/output.rs +++ b/src/command/output.rs @@ -15,9 +15,7 @@ use rover_client::operations::graph::publish::GraphPublishResponse; use rover_client::operations::subgraph::delete::SubgraphDeleteResponse; use rover_client::operations::subgraph::list::SubgraphListResponse; use rover_client::operations::subgraph::publish::SubgraphPublishResponse; -use rover_client::shared::{ - CheckRequestSuccessResult, CheckResponse, FetchResponse, GraphRef, -}; +use rover_client::shared::{CheckRequestSuccessResult, CheckResponse, FetchResponse, GraphRef}; use rover_client::RoverClientError; use rover_std::Style; use serde::Serialize; diff --git a/src/options/output.rs b/src/options/output.rs index 089fdb268..4147f23cf 100644 --- a/src/options/output.rs +++ b/src/options/output.rs @@ -5,7 +5,7 @@ use calm_io::{stderrln, stdoutln}; use camino::Utf8PathBuf; use clap::{error::ErrorKind as ClapErrorKind, CommandFactory, Parser, ValueEnum}; use rover_client::shared::SdlType; -use rover_std::{Fs, Style, Emoji}; +use rover_std::{Emoji, Fs, Style}; use serde::Serialize; use crate::{ @@ -69,7 +69,8 @@ impl RoverPrinter for RoverOutput { if let Ok(Some(result)) = output { match output_type { FinalOutputType::File(path) => { - let success_heading = Style::Heading.paint(format!("{}The output was printed to", Emoji::Memo)); + let success_heading = + Style::Heading.paint(format!("{}The output was printed to", Emoji::Memo)); let path_text = Style::Path.paint(&path); Fs::write_file(&path, result)?; stderrln!("{} {}", success_heading, path_text)?; @@ -127,7 +128,7 @@ pub struct OutputStrategy { format_type: Option, /// Specify a file to write Rover's output to - #[arg(long = "output", short= 'o', global = true)] + #[arg(long = "output", short = 'o', global = true)] output_type: Option, } From 4482f83896fe02386836214755545db937b303a5 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Thu, 19 Jan 2023 12:01:01 -0600 Subject: [PATCH 25/26] fix: make --watch work with new --output (#1478) --- docs/source/commands/graphs.mdx | 2 +- docs/source/commands/readmes.mdx | 2 +- docs/source/commands/subgraphs.mdx | 4 +- docs/source/commands/supergraphs.mdx | 2 +- docs/source/conventions.md | 12 +- docs/source/migration.md | 10 +- src/cli.rs | 22 +-- src/command/graph/introspect.rs | 15 +- src/command/graph/mod.rs | 7 +- src/command/output.rs | 133 ++++--------- src/command/subgraph/introspect.rs | 12 +- src/command/subgraph/mod.rs | 7 +- src/error/metadata/mod.rs | 2 +- src/error/mod.rs | 4 +- src/options/introspect.rs | 20 +- src/options/output.rs | 283 +++++++++++++++++---------- 16 files changed, 276 insertions(+), 261 deletions(-) diff --git a/docs/source/commands/graphs.mdx b/docs/source/commands/graphs.mdx index 937ae7dcb..3f12deadf 100644 --- a/docs/source/commands/graphs.mdx +++ b/docs/source/commands/graphs.mdx @@ -63,7 +63,7 @@ You can also save the output to a local `.graphql` file like so: ```bash # Creates prod-schema.graphql or overwrites if it already exists -rover graph fetch my-graph@my-variant > prod-schema.graphql +rover graph fetch my-graph@my-variant --output prod-schema.graphql ``` > For more on passing values via `stdout`, see [Conventions](../conventions#using-stdout). diff --git a/docs/source/commands/readmes.mdx b/docs/source/commands/readmes.mdx index 3e45c1e3e..fb8af6583 100644 --- a/docs/source/commands/readmes.mdx +++ b/docs/source/commands/readmes.mdx @@ -29,7 +29,7 @@ By default, the README is output to `stdout`. You can also save the output to a ```bash # Creates README.md or overwrites if it already exists -rover readme fetch my-graph@my-variant > README.md +rover readme fetch my-graph@my-variant --output README.md ``` You can also request the output as JSON with the `--format json` option: diff --git a/docs/source/commands/subgraphs.mdx b/docs/source/commands/subgraphs.mdx index 52ca89326..6e9fd2adf 100644 --- a/docs/source/commands/subgraphs.mdx +++ b/docs/source/commands/subgraphs.mdx @@ -54,7 +54,7 @@ The subgraph must be reachable by Rover. The subgraph does _not_ need to have in #### Watching for schema changes -If you pass `--watch` to `rover subgraph introspect`, Rover introspects your subgraph every second. Whenever the returned schema differs from the _previously_ returned schema, Rover outputs the updated schema. +If you pass `--watch` to `rover subgraph introspect`, Rover introspects your subgraph every second. Whenever the returned schema differs from the _previously_ returned schema, Rover outputs the updated schema. This is most useful when combined with the `--output ` argument which will write the introspection response out to a file whenever its contents change. #### Including headers @@ -83,7 +83,7 @@ You can also save the output to a local `.graphql` file like so: ```bash # Creates accounts-schema.graphql or overwrites if it already exists -rover subgraph introspect http://localhost:4000 > accounts-schema.graphql +rover subgraph introspect http://localhost:4000 --output accounts-schema.graphql ``` > For more on passing values via `stdout`, see [Using `stdout`](../conventions#using-stdout). diff --git a/docs/source/commands/supergraphs.mdx b/docs/source/commands/supergraphs.mdx index b183a01fd..0eff6e2a9 100644 --- a/docs/source/commands/supergraphs.mdx +++ b/docs/source/commands/supergraphs.mdx @@ -105,7 +105,7 @@ You can save the schema output to a local `.graphql` file like so: ```bash # Creates prod-schema.graphql or overwrites if it already exists -rover supergraph compose --config ./supergraph.yaml > prod-schema.graphql +rover supergraph compose --config ./supergraph.yaml --output prod-schema.graphql ``` > For more on passing values via `stdout`, see [Using `stdout`](../conventions#using-stdout). diff --git a/docs/source/conventions.md b/docs/source/conventions.md index 8fcfe050a..375a6cf8a 100644 --- a/docs/source/conventions.md +++ b/docs/source/conventions.md @@ -39,9 +39,9 @@ All Rover commands that interact with the Apollo graph registry require a graph ### Using `stdout` -Rover commands print to `stdout` in a predictable, portable format. This enables output to be used elsewhere (such as in another CLI, or as input to another Rover command). To help maintain this predictability, Rover prints logs to `stderr` instead of `stdout`. +Rover commands print to `stdout` in a predictable, portable format. This enables output to be used elsewhere (such as in another CLI, or as input to another Rover command). To help maintain this predictability, Rover prints progress logs to `stderr` instead of `stdout`. -To redirect Rover's output to a location other than your terminal, you can use the pipe `|` or output redirect `>` operators. +To redirect Rover's output to a location other than your terminal, you can use the `--output ` argument, the pipe `|` operator, or the redirect `>` operator. #### Pipe `|` @@ -53,12 +53,12 @@ rover graph introspect http://localhost:4000 | pbcopy In this example, the output of the `introspect` command is piped to `pbcopy`, a MacOS command that copies a value to the clipboard. Certain Rover commands also accept values from `stdin`, as explained in [Using `stdin`](#using-stdin). -#### Output redirect `>` +#### Output to a file -Use the output redirect operator to write the `stdout` of a command to a file, like so: +Use the `--output ` argument to write command output to a file. ``` -rover graph fetch my-graph@prod > schema.graphql +rover graph fetch my-graph@prod --output schema.graphql ``` In this example, the schema returned by `graph fetch` is written to the file `schema.graphql`. If this file already exists, it's overwritten. Otherwise, it's created. @@ -72,5 +72,3 @@ rover graph introspect http://localhost:4000 | rover graph check my-graph --sche ``` In this example, the schema returned by `graph introspect` is then passed as the `--schema` option to `graph check`. - -> Currently, `--schema` is the only Rover option that accepts a file path. diff --git a/docs/source/migration.md b/docs/source/migration.md index d2698c2f8..4943e81d6 100644 --- a/docs/source/migration.md +++ b/docs/source/migration.md @@ -160,8 +160,8 @@ An example script for converting output from `rover {sub}graph check my-graph -- apollo client:download-schema --graph my-graph --variant prod ## Rover ## -# automatically outputs to stdout. Can redirect to schema.graphql -rover graph fetch my-graph@prod > schema.graphql +# automatically outputs to stdout. Can redirect to schema.graphql with the `--output ` argument +rover graph fetch my-graph@prod --output schema.graphql ``` ### Fetching a graph's schema from introspection @@ -173,9 +173,9 @@ rover graph fetch my-graph@prod > schema.graphql apollo service:download --endpoint http://localhost:4000 ## Rover ## -# automatically outputs to stdout. Can redirect to schema.graphql +# automatically outputs to stdout. Can redirect to schema.graphql with the `--output ` argument # can ONLY output SDL -rover graph introspect http://localhost:4000 +rover graph introspect http://localhost:4000 --output schema.graphql ``` ### Publishing a monolithic schema to the Apollo graph registry @@ -261,8 +261,6 @@ rover graph introspect http://localhost:4001 --header "authorization: Bearer wxy | rover graph check my-graph@prod --schema - ``` - - ### Pushing Subgraph Changes (with a config file) ```js diff --git a/src/cli.rs b/src/cli.rs index 1452a9399..42c02944d 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -5,7 +5,7 @@ use reqwest::blocking::Client; use serde::Serialize; use crate::command::{self, RoverOutput}; -use crate::options::OutputStrategy; +use crate::options::OutputOpts; use crate::utils::{ client::{ClientBuilder, ClientTimeout, StudioClientConfig}, env::{RoverEnv, RoverEnvKey}, @@ -62,7 +62,7 @@ pub struct Rover { log_level: Option, #[clap(flatten)] - output_strategy: OutputStrategy, + output_opts: OutputOpts, /// Accept invalid certificates when performing HTTPS requests. /// @@ -116,7 +116,7 @@ impl Rover { pub fn run(&self) -> RoverResult<()> { timber::init(self.log_level); tracing::trace!(command_structure = ?self); - self.output_strategy.validate_options(); + self.output_opts.validate_options(); // attempt to create a new `Session` to capture anonymous usage data let rover_output = match Session::new(self) { @@ -152,12 +152,12 @@ impl Rover { match rover_output { Ok(output) => { - self.output_strategy.write_rover_output(output)?; + self.output_opts.handle_output(output)?; process::exit(0); } Err(error) => { - self.output_strategy.write_rover_output(error)?; + self.output_opts.handle_output(error)?; process::exit(1); } @@ -192,7 +192,7 @@ impl Rover { self.get_client_config()?, self.get_git_context()?, self.get_checks_timeout_seconds()?, - self.output_strategy.get_json(), + &self.output_opts, ), Command::Template(command) => command.run(self.get_client_config()?), Command::Readme(command) => command.run(self.get_client_config()?), @@ -200,7 +200,7 @@ impl Rover { self.get_client_config()?, self.get_git_context()?, self.get_checks_timeout_seconds()?, - self.output_strategy.get_json(), + &self.output_opts, ), Command::Update(command) => { command.run(self.get_rover_config()?, self.get_reqwest_client()?) @@ -400,19 +400,19 @@ pub enum Command { } #[derive(ValueEnum, Debug, Serialize, Clone, Eq, PartialEq)] -pub enum FormatType { +pub enum RoverOutputFormatKind { Plain, Json, } #[derive(ValueEnum, Debug, Serialize, Clone, Eq, PartialEq)] -pub enum RoverFormatType { +pub enum RoverOutputKind { RoverOutput, RoverError, } -impl Default for FormatType { +impl Default for RoverOutputFormatKind { fn default() -> Self { - FormatType::Plain + RoverOutputFormatKind::Plain } } diff --git a/src/command/graph/introspect.rs b/src/command/graph/introspect.rs index 4abeff467..fd569d369 100644 --- a/src/command/graph/introspect.rs +++ b/src/command/graph/introspect.rs @@ -8,7 +8,10 @@ use rover_client::{ operations::graph::introspect::{self, GraphIntrospectInput}, }; -use crate::{options::IntrospectOpts, RoverOutput, RoverResult}; +use crate::{ + options::{IntrospectOpts, OutputOpts}, + RoverOutput, RoverResult, +}; #[derive(Debug, Serialize, Parser)] pub struct Introspect { @@ -17,10 +20,9 @@ pub struct Introspect { } impl Introspect { - pub fn run(&self, client: Client, json: bool) -> RoverResult { + pub fn run(&self, client: Client, output_opts: &OutputOpts) -> RoverResult { if self.opts.watch { - self.exec_and_watch(&client, json)?; - Ok(RoverOutput::EmptySuccess) + self.exec_and_watch(&client, output_opts) } else { let sdl = self.exec(&client, true)?; Ok(RoverOutput::Introspection(sdl)) @@ -41,9 +43,8 @@ impl Introspect { Ok(introspect::run(GraphIntrospectInput { headers }, &client, should_retry)?.schema_sdl) } - pub fn exec_and_watch(&self, client: &Client, json: bool) -> RoverResult { + pub fn exec_and_watch(&self, client: &Client, output_opts: &OutputOpts) -> ! { self.opts - .exec_and_watch(|| self.exec(client, false), json)?; - Ok(RoverOutput::EmptySuccess) + .exec_and_watch(|| self.exec(client, false), output_opts) } } diff --git a/src/command/graph/mod.rs b/src/command/graph/mod.rs index bb91cc54a..1ac9bf3a3 100644 --- a/src/command/graph/mod.rs +++ b/src/command/graph/mod.rs @@ -13,6 +13,7 @@ pub use publish::Publish; use clap::Parser; use serde::Serialize; +use crate::options::OutputOpts; use crate::utils::client::StudioClientConfig; use crate::{RoverOutput, RoverResult}; @@ -49,7 +50,7 @@ impl Graph { client_config: StudioClientConfig, git_context: GitContext, checks_timeout_seconds: u64, - json: bool, + output_opts: &OutputOpts, ) -> RoverResult { match &self.command { Command::Check(command) => { @@ -58,7 +59,9 @@ impl Graph { Command::Delete(command) => command.run(client_config), Command::Fetch(command) => command.run(client_config), Command::Publish(command) => command.run(client_config, git_context), - Command::Introspect(command) => command.run(client_config.get_reqwest_client()?, json), + Command::Introspect(command) => { + command.run(client_config.get_reqwest_client()?, output_opts) + } } } } diff --git a/src/command/output.rs b/src/command/output.rs index 080a1409e..b7d091338 100644 --- a/src/command/output.rs +++ b/src/command/output.rs @@ -1,24 +1,26 @@ use std::collections::BTreeMap; -use std::fmt::{self, Debug, Display}; +use std::fmt::Debug; use std::io; use crate::command::supergraph::compose::CompositionOutput; +use crate::options::JsonVersion; use crate::utils::table::{self, row}; use crate::RoverError; use crate::options::GithubTemplate; use atty::Stream; -use calm_io::{stderr, stderrln, stdoutln}; +use calm_io::{stderr, stderrln}; use camino::Utf8PathBuf; use crossterm::style::Attribute::Underlined; use rover_client::operations::graph::publish::GraphPublishResponse; use rover_client::operations::subgraph::delete::SubgraphDeleteResponse; use rover_client::operations::subgraph::list::SubgraphListResponse; use rover_client::operations::subgraph::publish::SubgraphPublishResponse; -use rover_client::shared::{CheckRequestSuccessResult, CheckResponse, FetchResponse, GraphRef}; +use rover_client::shared::{ + CheckRequestSuccessResult, CheckResponse, FetchResponse, GraphRef, SdlType, +}; use rover_client::RoverClientError; use rover_std::Style; -use serde::Serialize; use serde_json::{json, Value}; use termimad::MadSkin; @@ -34,7 +36,7 @@ use termimad::MadSkin; pub enum RoverOutput { DocsList(BTreeMap<&'static str, &'static str>), FetchResponse(FetchResponse), - CoreSchema(String), + SupergraphSchema(String), CompositionResult(CompositionOutput), SubgraphList(SubgraphListResponse), CheckResponse(CheckResponse), @@ -193,7 +195,7 @@ impl RoverOutput { None } } - RoverOutput::CoreSchema(csdl) => Some((csdl).to_string()), + RoverOutput::SupergraphSchema(csdl) => Some((csdl).to_string()), RoverOutput::CompositionResult(composition_output) => { let warn_prefix = Style::HintPrefix.paint("HINT:"); @@ -318,7 +320,7 @@ impl RoverOutput { json!({ "shortlinks": shortlink_vec }) } RoverOutput::FetchResponse(fetch_response) => json!(fetch_response), - RoverOutput::CoreSchema(csdl) => json!({ "core_schema": csdl }), + RoverOutput::SupergraphSchema(csdl) => json!({ "core_schema": csdl }), RoverOutput::CompositionResult(composition_output) => { if let Some(federation_version) = &composition_output.federation_version { json!({ @@ -424,106 +426,43 @@ impl RoverOutput { JsonVersion::default() } - pub(crate) fn print_descriptor(&self, descriptor: impl Display) -> io::Result<()> { + pub(crate) fn print_descriptor(&self) -> io::Result<()> { if atty::is(Stream::Stdout) { - stderrln!("{}: \n", Style::Heading.paint(descriptor.to_string()))?; + if let Some(descriptor) = self.descriptor() { + stderrln!("{}: \n", Style::Heading.paint(descriptor))?; + } } Ok(()) } - pub(crate) fn print_one_line_descriptor(&self, descriptor: impl Display) -> io::Result<()> { + pub(crate) fn print_one_line_descriptor(&self) -> io::Result<()> { if atty::is(Stream::Stdout) { - stderr!("{}: ", Style::Heading.paint(descriptor.to_string()))?; + if let Some(descriptor) = self.descriptor() { + stderr!("{}: ", Style::Heading.paint(descriptor))?; + } } Ok(()) } -} - -#[derive(Debug, Clone, Serialize)] -pub(crate) struct JsonOutput { - json_version: JsonVersion, - data: JsonData, - error: Value, -} - -impl JsonOutput { - pub(crate) fn success(data: Value, error: Value, json_version: JsonVersion) -> JsonOutput { - JsonOutput { - json_version, - data: JsonData::success(data), - error, - } - } - - pub(crate) fn failure(data: Value, error: Value, json_version: JsonVersion) -> JsonOutput { - JsonOutput { - json_version, - data: JsonData::failure(data), - error, - } - } - - pub(crate) fn print(&self) -> io::Result<()> { - stdoutln!("{}", self) - } -} - -impl fmt::Display for JsonOutput { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { - write!(f, "{}", json!(self)) - } -} - -impl From for JsonOutput { - fn from(error: RoverError) -> Self { - let data_json = error.get_internal_data_json(); - let error_json = error.get_internal_error_json(); - JsonOutput::failure(data_json, error_json, error.get_json_version()) - } -} - -impl From for JsonOutput { - fn from(output: RoverOutput) -> Self { - let data = output.get_internal_data_json(); - let error = output.get_internal_error_json(); - JsonOutput::success(data, error, output.get_json_version()) - } -} - -#[derive(Debug, Clone, Serialize)] -pub(crate) struct JsonData { - #[serde(flatten)] - inner: Value, - success: bool, -} - -impl JsonData { - pub(crate) fn success(inner: Value) -> JsonData { - JsonData { - inner, - success: true, - } - } - - pub(crate) fn failure(inner: Value) -> JsonData { - JsonData { - inner, - success: false, + pub(crate) fn descriptor(&self) -> Option<&str> { + match &self { + RoverOutput::FetchResponse(fetch_response) => match fetch_response.sdl.r#type { + SdlType::Graph | SdlType::Subgraph { .. } => Some("Schema"), + SdlType::Supergraph => Some("Supergraph Schema"), + }, + RoverOutput::CompositionResult(_) | RoverOutput::SupergraphSchema(_) => { + Some("Supergraph Schema") + } + RoverOutput::TemplateUseSuccess { .. } => Some("Project generated"), + RoverOutput::CheckResponse(_) => Some("Check Result"), + RoverOutput::AsyncCheckResponse(_) => Some("Check Started"), + RoverOutput::Profiles(_) => Some("Profiles"), + RoverOutput::Introspection(_) => Some("Introspection Response"), + RoverOutput::ReadmeFetchResponse { .. } => Some("Readme"), + RoverOutput::GraphPublishResponse { .. } => Some("Schema Hash"), + _ => None, } } } -#[derive(Debug, Clone, Serialize)] -pub(crate) enum JsonVersion { - #[serde(rename = "1")] - One, -} - -impl Default for JsonVersion { - fn default() -> Self { - JsonVersion::One - } -} - #[cfg(test)] mod tests { use std::collections::BTreeMap; @@ -545,6 +484,8 @@ mod tests { use anyhow::anyhow; + use crate::options::JsonOutput; + use super::*; #[test] @@ -602,7 +543,7 @@ mod tests { #[test] fn core_schema_json() { let mock_core_schema = "core schema contents".to_string(); - let actual_json: JsonOutput = RoverOutput::CoreSchema(mock_core_schema).into(); + let actual_json: JsonOutput = RoverOutput::SupergraphSchema(mock_core_schema).into(); let expected_json = json!( { "json_version": "1", diff --git a/src/command/subgraph/introspect.rs b/src/command/subgraph/introspect.rs index b7b2e3c6d..b429bd828 100644 --- a/src/command/subgraph/introspect.rs +++ b/src/command/subgraph/introspect.rs @@ -8,7 +8,7 @@ use rover_client::{ operations::subgraph::introspect::{self, SubgraphIntrospectInput}, }; -use crate::options::IntrospectOpts; +use crate::options::{IntrospectOpts, OutputOpts}; use crate::{RoverOutput, RoverResult}; #[derive(Debug, Serialize, Parser)] @@ -18,10 +18,9 @@ pub struct Introspect { } impl Introspect { - pub fn run(&self, client: Client, json: bool) -> RoverResult { + pub fn run(&self, client: Client, output_opts: &OutputOpts) -> RoverResult { if self.opts.watch { - self.exec_and_watch(&client, json)?; - Ok(RoverOutput::EmptySuccess) + self.exec_and_watch(&client, output_opts) } else { let sdl = self.exec(&client, true)?; Ok(RoverOutput::Introspection(sdl)) @@ -42,9 +41,8 @@ impl Introspect { Ok(introspect::run(SubgraphIntrospectInput { headers }, &client, should_retry)?.result) } - pub fn exec_and_watch(&self, client: &Client, json: bool) -> RoverResult { + pub fn exec_and_watch(&self, client: &Client, output_opts: &OutputOpts) -> ! { self.opts - .exec_and_watch(|| self.exec(client, false), json)?; - Ok(RoverOutput::EmptySuccess) + .exec_and_watch(|| self.exec(client, false), output_opts) } } diff --git a/src/command/subgraph/mod.rs b/src/command/subgraph/mod.rs index b73d04c64..b615d3ca7 100644 --- a/src/command/subgraph/mod.rs +++ b/src/command/subgraph/mod.rs @@ -15,6 +15,7 @@ pub use publish::Publish; use clap::Parser; use serde::Serialize; +use crate::options::OutputOpts; use crate::utils::client::StudioClientConfig; use crate::{RoverOutput, RoverResult}; @@ -54,14 +55,16 @@ impl Subgraph { client_config: StudioClientConfig, git_context: GitContext, checks_timeout_seconds: u64, - json: bool, + output_opts: &OutputOpts, ) -> RoverResult { match &self.command { Command::Check(command) => { command.run(client_config, git_context, checks_timeout_seconds) } Command::Delete(command) => command.run(client_config), - Command::Introspect(command) => command.run(client_config.get_reqwest_client()?, json), + Command::Introspect(command) => { + command.run(client_config.get_reqwest_client()?, output_opts) + } Command::Fetch(command) => command.run(client_config), Command::List(command) => command.run(client_config), Command::Publish(command) => command.run(client_config, git_context), diff --git a/src/error/metadata/mod.rs b/src/error/metadata/mod.rs index 10b7c8287..5798d0e82 100644 --- a/src/error/metadata/mod.rs +++ b/src/error/metadata/mod.rs @@ -7,7 +7,7 @@ pub use suggestion::RoverErrorSuggestion; use houston::HoustonProblem; use rover_client::RoverClientError; -use crate::{command::output::JsonVersion, utils::env::RoverEnvKey}; +use crate::{options::JsonVersion, utils::env::RoverEnvKey}; use std::env; diff --git a/src/error/mod.rs b/src/error/mod.rs index 5bd55c653..323676dc6 100644 --- a/src/error/mod.rs +++ b/src/error/mod.rs @@ -16,10 +16,10 @@ use std::error::Error; use std::fmt::{self, Debug, Display}; use std::io; -use crate::command::output::JsonVersion; - use apollo_federation_types::build::BuildErrors; +use crate::options::JsonVersion; + /// A specialized `Error` type for Rover that wraps `anyhow` /// and provides some extra `Metadata` for end users depending /// on the specific error they encountered. diff --git a/src/options/introspect.rs b/src/options/introspect.rs index b42a215cd..a2fa2446c 100644 --- a/src/options/introspect.rs +++ b/src/options/introspect.rs @@ -2,7 +2,11 @@ use clap::Parser; use reqwest::Url; use serde::{Deserialize, Serialize}; -use crate::{command::output::JsonOutput, utils::parsers::parse_header, RoverOutput, RoverResult}; +use crate::{ + options::{OutputOpts, RoverPrinter}, + utils::parsers::parse_header, + RoverOutput, RoverResult, +}; #[derive(Debug, Serialize, Deserialize, Parser)] pub struct IntrospectOpts { @@ -26,7 +30,7 @@ pub struct IntrospectOpts { } impl IntrospectOpts { - pub fn exec_and_watch(&self, exec_fn: F, json: bool) -> RoverResult + pub fn exec_and_watch(&self, exec_fn: F, output_opts: &OutputOpts) -> ! where F: Fn() -> RoverResult, { @@ -43,11 +47,7 @@ impl IntrospectOpts { if was_updated { let output = RoverOutput::Introspection(sdl.to_string()); - if json { - let _ = JsonOutput::from(output).print(); - } else { - let _ = output.get_stdout(); - } + let _ = output.write_or_print(output_opts).map_err(|e| e.print()); } last_result = Some(sdl); } @@ -60,11 +60,7 @@ impl IntrospectOpts { } } if was_updated { - if json { - let _ = JsonOutput::from(error).print(); - } else { - let _ = error.print(); - } + let _ = error.write_or_print(output_opts).map_err(|e| e.print()); } last_result = Some(e); } diff --git a/src/options/output.rs b/src/options/output.rs index 4147f23cf..f44d54d8a 100644 --- a/src/options/output.rs +++ b/src/options/output.rs @@ -1,16 +1,15 @@ -use std::str::FromStr; +use std::{fmt, io, str::FromStr}; use anyhow::Result; use calm_io::{stderrln, stdoutln}; use camino::Utf8PathBuf; use clap::{error::ErrorKind as ClapErrorKind, CommandFactory, Parser, ValueEnum}; -use rover_client::shared::SdlType; use rover_std::{Emoji, Fs, Style}; use serde::Serialize; +use serde_json::{json, Value}; use crate::{ - cli::{FormatType, Rover}, - command::output::JsonOutput, + cli::{Rover, RoverOutputFormatKind}, RoverError, RoverOutput, RoverResult, }; @@ -18,27 +17,27 @@ use crate::{ pub struct Output { /// The file path to write the command output to. #[clap(long)] - output: OutputType, + output: OutputOpt, } #[derive(Debug, Clone, Eq, PartialEq, Serialize)] -pub enum FinalOutputType { +pub enum RoverOutputDestination { File(Utf8PathBuf), Stdout, } #[derive(Debug, Clone, Eq, PartialEq, Serialize)] -pub enum OutputType { - LegacyOutputType(FormatType), +pub enum OutputOpt { + LegacyOutputType(RoverOutputFormatKind), File(Utf8PathBuf), } -impl FromStr for OutputType { +impl FromStr for OutputOpt { type Err = anyhow::Error; fn from_str(s: &str) -> Result { - if let Ok(format_type) = FormatType::from_str(s, true) { - Ok(Self::LegacyOutputType(format_type)) + if let Ok(format_kind) = RoverOutputFormatKind::from_str(s, true) { + Ok(Self::LegacyOutputType(format_kind)) } else { Ok(Self::File(Utf8PathBuf::from(s))) } @@ -46,59 +45,38 @@ impl FromStr for OutputType { } pub trait RoverPrinter { - fn write_or_print( - self, - format_type: FormatType, - output_type: FinalOutputType, - ) -> RoverResult<()>; + fn write_or_print(self, output_opts: &OutputOpts) -> RoverResult<()>; } impl RoverPrinter for RoverOutput { - fn write_or_print( - self, - format_type: FormatType, - output_type: FinalOutputType, - ) -> RoverResult<()> { + fn write_or_print(self, output_opts: &OutputOpts) -> RoverResult<()> { + let (format_kind, output_destination) = output_opts.get_format_and_strategy(); + // Format the RoverOutput as either plain text or JSON. - let output = match format_type { - FormatType::Plain => self.get_stdout(), - FormatType::Json => Ok(Some(JsonOutput::from(self.clone()).to_string())), + let output = match format_kind { + RoverOutputFormatKind::Plain => self.get_stdout(), + RoverOutputFormatKind::Json => Ok(Some(JsonOutput::from(self.clone()).to_string())), }; // Print the RoverOutput to file or stdout. if let Ok(Some(result)) = output { - match output_type { - FinalOutputType::File(path) => { - let success_heading = - Style::Heading.paint(format!("{}The output was printed to", Emoji::Memo)); + match output_destination { + RoverOutputDestination::File(path) => { + let success_heading = Style::Heading.paint(format!( + "{}{} was printed to", + Emoji::Memo, + self.descriptor().unwrap_or("The output") + )); let path_text = Style::Path.paint(&path); Fs::write_file(&path, result)?; stderrln!("{} {}", success_heading, path_text)?; } - FinalOutputType::Stdout => { + RoverOutputDestination::Stdout => { // Call the appropriate method based on the variant of RoverOutput. - let descriptor = match &self { - RoverOutput::FetchResponse(fetch_response) => { - match fetch_response.sdl.r#type { - SdlType::Graph | SdlType::Subgraph { .. } => "SDL", - SdlType::Supergraph => "Supergraph SDL", - } - } - RoverOutput::CoreSchema(_) => "CoreSchema", - RoverOutput::CompositionResult(_) => "CoreSchema", - RoverOutput::TemplateUseSuccess { .. } => "Project generated", - RoverOutput::CheckResponse(_) => "Check Result", - RoverOutput::AsyncCheckResponse(_) => "Check Started", - RoverOutput::Profiles(_) => "Profiles", - RoverOutput::Introspection(_) => "Introspection Response", - RoverOutput::ReadmeFetchResponse { .. } => "Readme", - RoverOutput::GraphPublishResponse { .. } => "Schema Hash", - _ => return Ok(()), - }; if let RoverOutput::GraphPublishResponse { .. } = self { - self.print_one_line_descriptor(descriptor)?; + self.print_one_line_descriptor()?; } else { - self.print_descriptor(descriptor)?; + self.print_descriptor()?; } stdoutln!("{}", &result)?; @@ -111,10 +89,23 @@ impl RoverPrinter for RoverOutput { } impl RoverPrinter for RoverError { - fn write_or_print(self, format_type: FormatType, _: FinalOutputType) -> RoverResult<()> { - match format_type { - FormatType::Plain => self.print(), - FormatType::Json => JsonOutput::from(self).print(), + fn write_or_print(self, output_opts: &OutputOpts) -> RoverResult<()> { + let (format_kind, output_destination) = output_opts.get_format_and_strategy(); + match format_kind { + RoverOutputFormatKind::Plain => self.print(), + RoverOutputFormatKind::Json => { + let json = JsonOutput::from(self); + match output_destination { + RoverOutputDestination::File(file) => { + let success_heading = Style::Heading + .paint(format!("{}Error JSON was printed to", Emoji::Memo,)); + Fs::write_file(&file, json.to_string())?; + stderrln!("{} {}", success_heading, file)?; + } + RoverOutputDestination::Stdout => json.print()?, + } + Ok(()) + } }?; Ok(()) @@ -122,28 +113,30 @@ impl RoverPrinter for RoverError { } #[derive(Debug, Parser, Serialize)] -pub struct OutputStrategy { +pub struct OutputOpts { /// Specify Rover's format type #[arg(long = "format", global = true)] - format_type: Option, + format_kind: Option, /// Specify a file to write Rover's output to #[arg(long = "output", short = 'o', global = true)] - output_type: Option, + output_file: Option, } -impl OutputStrategy { +impl OutputOpts { + /// Validates the argument group, exiting early if there are conflicts. + /// This should be called at the start of the application. pub fn validate_options(&self) { - match (&self.format_type, &self.output_type) { - (Some(_), Some(OutputType::LegacyOutputType(_))) => { + match (&self.format_kind, &self.output_file) { + (Some(_), Some(OutputOpt::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(); + ClapErrorKind::ArgumentConflict, + "The argument '--output' cannot be used with '--format' when '--output' is not a file", + ) + .exit(); } - (None, Some(OutputType::LegacyOutputType(_))) => { + (None, Some(OutputOpt::LegacyOutputType(_))) => { let warn_prefix = Style::WarningPrefix.paint("WARN:"); eprintln!("{} The argument for specifying the format of Rover's output has been renamed from '--output' to '--format'. Please use the '--format' argument to specify the output format instead of '--output'. Future versions of Rover may be incompatible with this usage.", warn_prefix); } @@ -152,53 +145,137 @@ impl OutputStrategy { } } - pub fn get_strategy(&self) -> (FormatType, FinalOutputType) { - let output_type = self.output_type.clone(); + /// Handle output and errors from a Rover command. + pub fn handle_output(&self, rover_command_output: T) -> RoverResult<()> + where + T: RoverPrinter, + { + rover_command_output.write_or_print(self) + } - match (&self.format_type, output_type) { - (None, None) | (None, Some(OutputType::LegacyOutputType(FormatType::Plain))) => { - (FormatType::Plain, FinalOutputType::Stdout) - } - (None, Some(OutputType::LegacyOutputType(FormatType::Json))) => { - (FormatType::Json, FinalOutputType::Stdout) - } - (None, Some(OutputType::File(path))) => { - (FormatType::Plain, FinalOutputType::File(path)) - } - (Some(FormatType::Plain), None) - | (Some(FormatType::Plain), Some(OutputType::LegacyOutputType(FormatType::Plain))) => { - (FormatType::Plain, FinalOutputType::Stdout) - } - (Some(FormatType::Plain), Some(OutputType::LegacyOutputType(FormatType::Json))) => { - (FormatType::Json, FinalOutputType::Stdout) - } - (Some(FormatType::Plain), Some(OutputType::File(path))) => { - (FormatType::Plain, FinalOutputType::File(path)) + /// Get the format (plain/json) and strategy (stdout/file) + pub fn get_format_and_strategy(&self) -> (RoverOutputFormatKind, RoverOutputDestination) { + let output_type = self.output_file.clone(); + + match (&self.format_kind, output_type) { + (None, None) + | (None, Some(OutputOpt::LegacyOutputType(RoverOutputFormatKind::Plain))) => { + (RoverOutputFormatKind::Plain, RoverOutputDestination::Stdout) } - (Some(FormatType::Json), None) - | (Some(FormatType::Json), Some(OutputType::LegacyOutputType(_))) => { - (FormatType::Json, FinalOutputType::Stdout) + (None, Some(OutputOpt::LegacyOutputType(RoverOutputFormatKind::Json))) => { + (RoverOutputFormatKind::Json, RoverOutputDestination::Stdout) } - (Some(FormatType::Json), Some(OutputType::File(path))) => { - (FormatType::Json, FinalOutputType::File(path)) + (None, Some(OutputOpt::File(path))) => ( + RoverOutputFormatKind::Plain, + RoverOutputDestination::File(path), + ), + (Some(RoverOutputFormatKind::Plain), None) + | ( + Some(RoverOutputFormatKind::Plain), + Some(OutputOpt::LegacyOutputType(RoverOutputFormatKind::Plain)), + ) => (RoverOutputFormatKind::Plain, RoverOutputDestination::Stdout), + ( + Some(RoverOutputFormatKind::Plain), + Some(OutputOpt::LegacyOutputType(RoverOutputFormatKind::Json)), + ) => (RoverOutputFormatKind::Json, RoverOutputDestination::Stdout), + (Some(RoverOutputFormatKind::Plain), Some(OutputOpt::File(path))) => ( + RoverOutputFormatKind::Plain, + RoverOutputDestination::File(path), + ), + (Some(RoverOutputFormatKind::Json), None) + | (Some(RoverOutputFormatKind::Json), Some(OutputOpt::LegacyOutputType(_))) => { + (RoverOutputFormatKind::Json, RoverOutputDestination::Stdout) } + (Some(RoverOutputFormatKind::Json), Some(OutputOpt::File(path))) => ( + RoverOutputFormatKind::Json, + RoverOutputDestination::File(path), + ), } } +} - pub fn write_rover_output(&self, output_trait: T) -> Result<(), RoverError> - where - T: RoverPrinter, - { - let (format_type, output_type) = self.get_strategy(); - output_trait.write_or_print(format_type, output_type)?; - Ok(()) +#[derive(Debug, Clone, Serialize)] +pub struct JsonOutput { + json_version: JsonVersion, + data: JsonData, + error: Value, +} + +impl JsonOutput { + fn success(data: Value, error: Value, json_version: JsonVersion) -> JsonOutput { + JsonOutput { + json_version, + data: JsonData::success(data), + error, + } + } + + fn failure(data: Value, error: Value, json_version: JsonVersion) -> JsonOutput { + JsonOutput { + json_version, + data: JsonData::failure(data), + error, + } + } + + fn print(&self) -> io::Result<()> { + stdoutln!("{}", self) + } +} + +impl fmt::Display for JsonOutput { + fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "{}", json!(self)) + } +} + +impl From for JsonOutput { + fn from(error: RoverError) -> Self { + let data_json = error.get_internal_data_json(); + let error_json = error.get_internal_error_json(); + JsonOutput::failure(data_json, error_json, error.get_json_version()) + } +} + +impl From for JsonOutput { + fn from(output: RoverOutput) -> Self { + let data = output.get_internal_data_json(); + let error = output.get_internal_error_json(); + JsonOutput::success(data, error, output.get_json_version()) } +} + +#[derive(Debug, Clone, Serialize)] +struct JsonData { + #[serde(flatten)] + inner: Value, + success: bool, +} + +impl JsonData { + fn success(inner: Value) -> JsonData { + JsonData { + inner, + success: true, + } + } + + fn failure(inner: Value) -> JsonData { + JsonData { + inner, + success: false, + } + } +} + +#[derive(Debug, Clone, Serialize)] +pub enum JsonVersion { + #[serde(rename = "1")] + One, +} - pub fn get_json(&self) -> bool { - matches!(self.format_type, Some(FormatType::Json)) - || matches!( - self.output_type, - Some(OutputType::LegacyOutputType(FormatType::Json)) - ) +impl Default for JsonVersion { + fn default() -> Self { + JsonVersion::One } } From ce3f80cdb248c550b57a109b6b6c6b950fd22984 Mon Sep 17 00:00:00 2001 From: Avery Harnish Date: Thu, 19 Jan 2023 14:01:38 -0600 Subject: [PATCH 26/26] chore: small changes on warning text --- src/options/output.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/options/output.rs b/src/options/output.rs index f44d54d8a..ccc3bce3f 100644 --- a/src/options/output.rs +++ b/src/options/output.rs @@ -138,7 +138,9 @@ impl OutputOpts { } (None, Some(OutputOpt::LegacyOutputType(_))) => { let warn_prefix = Style::WarningPrefix.paint("WARN:"); - eprintln!("{} The argument for specifying the format of Rover's output has been renamed from '--output' to '--format'. Please use the '--format' argument to specify the output format instead of '--output'. Future versions of Rover may be incompatible with this usage.", warn_prefix); + let output_argument = Style::Command.paint("'--output'"); + let format_argument = Style::Command.paint("'--format'"); + eprintln!("{} Future versions of Rover may be incompatible with this usage of {output_argument}.\n\nThe argument for specifying the format of Rover's output has been renamed from {output_argument} to {format_argument}.\nPlease use {format_argument} to configure Rover's output format instead of {output_argument}.\n", warn_prefix); } // there are default options, so if nothing is passed, print no errors or warnings _ => (),