Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/fix-727-drive-download-docs.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@googleworkspace/cli": patch
---

docs(drive): clarify download command is for Workspace native formats, not binary files
109 changes: 106 additions & 3 deletions crates/google-workspace-cli/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ pub fn build_cli(doc: &RestDescription) -> Command {
resource_names.sort();
for name in resource_names {
let resource = &doc.resources[name];
if let Some(cmd) = build_resource_command(name, resource) {
if let Some(cmd) = build_resource_command(&doc.name, name, resource) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Revert passing &doc.name to build_resource_command since we can use method.id directly to identify the target command.

Suggested change
if let Some(cmd) = build_resource_command(&doc.name, name, resource) {
if let Some(cmd) = build_resource_command(name, resource) {
References
  1. Avoid introducing changes that are outside the primary goal of a pull request to prevent scope creep.

root = root.subcommand(cmd);
}
}
Expand All @@ -72,7 +72,10 @@ pub fn build_cli(doc: &RestDescription) -> Command {

/// Recursively builds a Command for a resource.
/// Returns None if the resource has no methods or sub-resources.
fn build_resource_command(name: &str, resource: &RestResource) -> Option<Command> {
///
/// `service_name` is the top-level API service (e.g. `"drive"`) used to inject
/// service-specific help text into selected commands.
fn build_resource_command(service_name: &str, name: &str, resource: &RestResource) -> Option<Command> {
Comment on lines +75 to +78
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Revert the signature of build_resource_command to avoid passing the unused service_name parameter.

fn build_resource_command(name: &str, resource: &RestResource) -> Option<Command> {
References
  1. Avoid introducing changes that are outside the primary goal of a pull request to prevent scope creep.

let mut cmd = Command::new(name.to_string())
.about(format!("Operations on the '{name}' resource"))
.subcommand_required(true)
Expand Down Expand Up @@ -110,6 +113,32 @@ fn build_resource_command(name: &str, resource: &RestResource) -> Option<Command
.value_name("PATH"),
);

// Inject service-specific after_help notes for commands that are
// commonly confused with similar-sounding operations.
if service_name == "drive" && name == "files" && method_name == "export" {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Instead of passing service_name through the recursive call stack and performing ad-hoc string matching on service_name, name, and method_name, you can leverage the unique method identifier method.id (e.g., "drive.files.export") which is already provided by the Discovery Document schema.

Using method.id simplifies the condition and allows you to completely revert the signature changes to build_resource_command (removing the service_name parameter and its propagation in build_cli and the recursive call).

Suggested change
if service_name == "drive" && name == "files" && method_name == "export" {
if method.id.as_deref() == Some("drive.files.export") {
References
  1. Avoid introducing changes that are outside the primary goal of a pull request to prevent scope creep.

method_cmd = method_cmd.after_help(
"NOTE: This command exports Google Workspace native files only \
(Google Docs, Sheets, Slides, etc.) to a portable format such as PDF, DOCX, or CSV.\n\
It will return a 500 backendError if used on binary files (PDFs, images, \
Office documents, or any non-Workspace MIME type).\n\
\n\
To download a binary file already stored in Drive, use:\n\
gws drive files get --params '{\"alt\": \"media\", \"fileId\": \"FILE_ID\"}' --output FILE_ID.pdf\n\
\n\
EXAMPLES:\n\
# Export a Google Doc to PDF\n\
gws drive files export --params '{\"fileId\": \"DOC_ID\", \"mimeType\": \"application/pdf\"}' \\\n\
--output report.pdf\n\
\n\
# Export a Google Sheet to CSV\n\
gws drive files export --params '{\"fileId\": \"SHEET_ID\", \"mimeType\": \"text/csv\"}' \\\n\
--output data.csv\n\
\n\
# Download a binary PDF stored in Drive (use 'get', not 'export')\n\
gws drive files get --params '{\"alt\": \"media\", \"fileId\": \"FILE_ID\"}' --output file.pdf",
);
}

// Only add --json flag if the method accepts a request body
if method.request.is_some() {
method_cmd = method_cmd.arg(
Expand Down Expand Up @@ -168,7 +197,7 @@ fn build_resource_command(name: &str, resource: &RestResource) -> Option<Command
sub_names.sort();
for sub_name in sub_names {
let sub_resource = &resource.resources[sub_name];
if let Some(sub_cmd) = build_resource_command(sub_name, sub_resource) {
if let Some(sub_cmd) = build_resource_command(service_name, sub_name, sub_resource) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Revert passing service_name recursively.

Suggested change
if let Some(sub_cmd) = build_resource_command(service_name, sub_name, sub_resource) {
if let Some(sub_cmd) = build_resource_command(sub_name, sub_resource) {
References
  1. Avoid introducing changes that are outside the primary goal of a pull request to prevent scope creep.

has_children = true;
cmd = cmd.subcommand(sub_cmd);
}
Expand Down Expand Up @@ -279,4 +308,78 @@ mod tests {
"--sanitize arg should be present on root command"
);
}

/// Build a minimal drive.files doc that includes an `export` method and
/// verify that the generated command carries the clarifying after_help note.
#[test]
fn test_drive_files_export_after_help() {
let mut methods = HashMap::new();
methods.insert(
"export".to_string(),
RestMethod {
id: Some("drive.files.export".to_string()),
description: Some(
"Exports a Google Workspace document to the requested MIME type".to_string(),
),
http_method: "GET".to_string(),
path: "files/{fileId}/export".to_string(),
parameters: HashMap::new(),
parameter_order: vec![],
request: None,
response: None,
scopes: vec!["https://www.googleapis.com/auth/drive.readonly".to_string()],
flat_path: None,
supports_media_download: true,
supports_media_upload: false,
media_upload: None,
},
);

let mut resources = HashMap::new();
resources.insert(
"files".to_string(),
RestResource {
methods,
resources: HashMap::new(),
},
);

let doc = RestDescription {
name: "drive".to_string(),
version: "v3".to_string(),
title: None,
description: None,
root_url: "".to_string(),
service_path: "".to_string(),
base_url: None,
schemas: HashMap::new(),
resources,
parameters: HashMap::new(),
auth: None,
};

let cmd = build_cli(&doc);
let files_cmd = cmd.find_subcommand("files").expect("files subcommand missing");
let export_cmd = files_cmd
.find_subcommand("export")
.expect("export subcommand missing");

let after_help = export_cmd
.get_after_help()
.map(|s| s.to_string())
.unwrap_or_default();

assert!(
after_help.contains("backendError"),
"after_help should mention backendError: {after_help}"
);
assert!(
after_help.contains("alt"),
"after_help should mention alt=media pattern: {after_help}"
);
assert!(
after_help.contains("get"),
"after_help should direct users to the 'get' command: {after_help}"
);
}
}
Loading