feat: support file-name for drive export#685
Conversation
📝 WalkthroughWalkthroughThe PR adds a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
domain/ccm
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #685 +/- ##
==========================================
- Coverage 63.04% 63.04% -0.01%
==========================================
Files 437 437
Lines 38847 38864 +17
==========================================
+ Hits 24491 24500 +9
- Misses 12184 12192 +8
Partials 2172 2172 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@3d534a39ee93a4cb3610cff14557e6034083c20d🧩 Skill updatenpx skills add larksuite/cli#feat/drive-export-file-name -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (4)
shortcuts/drive/drive_export.go (3)
36-36: Optional: enrich--file-nameflag description for AI agents.The current description (
preferred output filename (optional)) does not communicate the two non-obvious behaviors that are documented inskills/lark-drive/references/lark-drive-export.md: (1) the value is sanitized, and (2) the--file-extensionsuffix is auto-appended when missing. Since CLI flags here are designed for AI-agent consumption, surfacing this in--helpreduces the chance of agents passing redundant or invalid extensions.Proposed tweak
- {Name: "file-name", Desc: "preferred output filename (optional)"}, + {Name: "file-name", Desc: "preferred local output filename; sanitized and auto-extended with --file-extension when missing"},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_export.go` at line 36, Update the CLI flag help for the flag with Name: "file-name" in drive_export.go to explicitly state the two non-obvious behaviors: that the provided filename will be sanitized and that the value will have the --file-extension suffix auto-appended if the extension is missing; change the Desc from "preferred output filename (optional)" to a concise sentence that mentions sanitization and automatic extension appending (using the --file-extension flag) so AI agents see the constraints and avoid redundant extensions.
58-90: Recommended: align dry-runfile_namereporting with execute’s default-naming behavior.In
Execute, stdout always carries afile_name(preferred name, remote title, server status name, or token-fallback). InDryRun,file_nameis only emitted when--file-nameis supplied. As a result, agents using--dry-runto preview the plan will silently miss the default filename branch, which is the more common case. Consider always settingfile_namein dry-run, falling back to<token><suffix>(or, where applicable, a placeholder describing the title-lookup fallback) so the dry-run shape matches execute.If you’d rather keep dry-run light (no remote title fetch), at minimum populate
file_namefrom<sanitized token><.ext>whenever--file-nameis empty so the field is always present and parseable.Sketch
- if name := strings.TrimSpace(runtime.Str("file-name")); name != "" { - dr.Set("file_name", ensureExportFileExtension(sanitizeExportFileName(name, spec.Token), spec.FileExtension)) - } + name := strings.TrimSpace(runtime.Str("file-name")) + if name == "" { + name = spec.Token // dry-run cannot resolve remote title; surface a deterministic placeholder + } + dr.Set("file_name", ensureExportFileExtension(sanitizeExportFileName(name, spec.Token), spec.FileExtension))🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_export.go` around lines 58 - 90, Dry-run currently only sets file_name when runtime.Str("file-name") is non-empty; update the DryRun builder logic in the Drive export branch (the code that constructs dr via common.NewDryRunAPI() in drive_export.go) to always populate the "file_name" field: if runtime.Str("file-name") is non-empty use the existing sanitizeExportFileName(...)/ensureExportFileExtension(...) path, otherwise set file_name to a sanitized token fallback (sanitizeExportFileName(spec.Token, spec.Token) or similar) with ensureExportFileExtension(..., spec.FileExtension) so the dry-run output shape matches Execute's default-naming behavior; keep the rest of the GET/POST dry-run flows and don't perform remote title lookup.
250-265: PropagatelastStatus.FileNamein the timeout result when available.The timeout block already extracts several fields from
lastStatus(failed,jobStatus,jobStatusLabel) to allow callers to resume from a known state. AddinglastStatus.FileNamewould be consistent: the success path (line 37–38) already usesstatus.FileNameas a fallback whenpreferredFileNameis empty, indicating the server populates it during polling, not only at job completion. Surfacing it on timeout would let downstream+export-downloadcalls inherit the same filename without re-prompting the user.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/drive/drive_export.go` around lines 250 - 265, The timeout result map should include lastStatus.FileName when available so callers can resume with the server-provided filename: in the timeout handling where result := map[string]interface{}{...} is constructed, use lastStatus.FileName as the fallback for preferredFileName (or set preferredFileName = lastStatus.FileName when non-empty) before the block that sets result["file_name"] and call ensureExportFileExtension(sanitizeExportFileName(preferredFileName, spec.Token), spec.FileExtension) as currently done; ensure you reference lastStatus.FileName, preferredFileName, ensureExportFileExtension, sanitizeExportFileName, spec.FileExtension, and runtime.Out when making the change so the timeout path mirrors the success path filename behavior.skills/lark-drive/references/lark-drive-export.md (1)
42-48: Optional: example does not actually exercise the auto-append behavior it describes.The comment promises
会按导出格式自动补扩展名, but the example passesweekly-report.pdf, which already has the extension and so won’t trigger the append. Consider showing the bare name to make the behavior obvious to readers/agents (and keep the resume workflow at lines 100-101 / 112 consistent with whichever form is canonical).Proposed example tweak
- --file-name "weekly-report.pdf" \ + --file-name "weekly-report" \ --output-dir ./exports🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@skills/lark-drive/references/lark-drive-export.md` around lines 42 - 48, The example claims the CLI will auto-append the export extension but uses "--file-name \"weekly-report.pdf\"" so the behavior isn't demonstrated; change the example to use a bare name (e.g., "--file-name \"weekly-report\"") so the auto-append for doc-type/docx is exercised, and update the matching resume workflow examples referenced at lines 100-101 and 112 to use the same canonical form (bare filename) for consistency; locate the flags in the shown lark-cli drive +export snippet and make the corresponding replacements.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@shortcuts/drive/drive_export.go`:
- Line 36: Update the CLI flag help for the flag with Name: "file-name" in
drive_export.go to explicitly state the two non-obvious behaviors: that the
provided filename will be sanitized and that the value will have the
--file-extension suffix auto-appended if the extension is missing; change the
Desc from "preferred output filename (optional)" to a concise sentence that
mentions sanitization and automatic extension appending (using the
--file-extension flag) so AI agents see the constraints and avoid redundant
extensions.
- Around line 58-90: Dry-run currently only sets file_name when
runtime.Str("file-name") is non-empty; update the DryRun builder logic in the
Drive export branch (the code that constructs dr via common.NewDryRunAPI() in
drive_export.go) to always populate the "file_name" field: if
runtime.Str("file-name") is non-empty use the existing
sanitizeExportFileName(...)/ensureExportFileExtension(...) path, otherwise set
file_name to a sanitized token fallback (sanitizeExportFileName(spec.Token,
spec.Token) or similar) with ensureExportFileExtension(..., spec.FileExtension)
so the dry-run output shape matches Execute's default-naming behavior; keep the
rest of the GET/POST dry-run flows and don't perform remote title lookup.
- Around line 250-265: The timeout result map should include lastStatus.FileName
when available so callers can resume with the server-provided filename: in the
timeout handling where result := map[string]interface{}{...} is constructed, use
lastStatus.FileName as the fallback for preferredFileName (or set
preferredFileName = lastStatus.FileName when non-empty) before the block that
sets result["file_name"] and call
ensureExportFileExtension(sanitizeExportFileName(preferredFileName, spec.Token),
spec.FileExtension) as currently done; ensure you reference lastStatus.FileName,
preferredFileName, ensureExportFileExtension, sanitizeExportFileName,
spec.FileExtension, and runtime.Out when making the change so the timeout path
mirrors the success path filename behavior.
In `@skills/lark-drive/references/lark-drive-export.md`:
- Around line 42-48: The example claims the CLI will auto-append the export
extension but uses "--file-name \"weekly-report.pdf\"" so the behavior isn't
demonstrated; change the example to use a bare name (e.g., "--file-name
\"weekly-report\"") so the auto-append for doc-type/docx is exercised, and
update the matching resume workflow examples referenced at lines 100-101 and 112
to use the same canonical form (bare filename) for consistency; locate the flags
in the shown lark-cli drive +export snippet and make the corresponding
replacements.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba71c11d-cc03-4834-bfd1-aac94be9afab
📒 Files selected for processing (6)
shortcuts/drive/drive_export.goshortcuts/drive/drive_export_test.goskills/lark-drive/SKILL.mdskills/lark-drive/references/lark-drive-export.mdtests/cli_e2e/drive/coverage.mdtests/cli_e2e/drive/drive_export_dryrun_test.go
1 participant
Summary
Adds local filename support to
drive +export, matching the existingdrive +export-download --file-namebehavior so agents can choose the primary export command without losing filename control. The provided name is sanitized and extended according to--file-extension.Changes
--file-nametodrive +exportfor markdown direct export, completed async export downloads, and timeout metadata.file_name/output_dirwhile keeping local-only filename data out of the export task request body.Test Plan
make unit-testlark xxxcommand works as expected:go test ./tests/cli_e2e/drive -run TestDriveExportDryRun_FileNameMetadata -count=1go vet ./...gofmt -l .go mod tidywith nogo.mod/go.sumdiffgo run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@v2.1.6 run --new-from-rev=origin/mainRelated Issues
Summary by CodeRabbit
Release Notes
New Features
--file-nameflag to customize exported file names for drive exports.Documentation
--file-nameflag.Tests