Skip to content

fix(slides): normalize presentation flag aliases#2032

Merged
fangshuyu-768 merged 1 commit into
mainfrom
fix/slides-presentation-flag-aliases
Jul 23, 2026
Merged

fix(slides): normalize presentation flag aliases#2032
fangshuyu-768 merged 1 commit into
mainfrom
fix/slides-presentation-flag-aliases

Conversation

@fangshuyu-768

@fangshuyu-768 fangshuyu-768 commented Jul 23, 2026

Copy link
Copy Markdown
Collaborator

Summary

Normalize common agent-generated presentation flag spellings to the canonical --presentation flag across Slides shortcuts. The compatibility aliases are parse-time only, so they stay out of help and completion while avoiding an error/retry round trip.

Changes

  • Normalize --presentation-id, --presentation-token, --token, --presentation_id, --xml-presentation-id, and --url to --presentation on every Slides shortcut that declares the canonical flag.
  • Keep compatibility aliases implicit: no additional flags are registered or shown in --help.
  • Add unit coverage for normalization, help visibility, and automatic attachment to all matching Slides shortcuts.
  • Add dry-run E2E coverage that verifies every alias reaches the same +xml-get API request.

Test Plan

  • Unit tests pass: GIT_CONFIG_GLOBAL=/dev/null GIT_CONFIG_NOSYSTEM=1 make unit-test
  • Slides unit and E2E tests pass: go test ./shortcuts/slides ./tests/cli_e2e/slides
  • go vet ./...
  • gofmt -l produces no output for changed Go files
  • go mod tidy produces no changes
  • golangci-lint reports 0 issues
  • Manual local verification confirms aliases work while lark-cli slides +xml-get --help only exposes --presentation

Related Issues

  • None

Summary by CodeRabbit

  • New Features

    • Added support for presentation-related flag aliases, including token, ID, and URL variants.
    • Aliases now resolve to the standard presentation value without adding duplicate flags to command help.
  • Bug Fixes

    • Slide commands using presentation aliases now correctly generate the intended presentation API request during dry runs.
  • Tests

    • Added coverage for alias parsing and end-to-end slide command behavior.

@github-actions github-actions Bot added the size/M Single-domain feat or fix with limited business impact label Jul 23, 2026
@coderabbitai

coderabbitai Bot commented Jul 23, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Slide shortcuts now normalize presentation-related flag aliases to --presentation. Unit tests cover parsing and shortcut mounting, while an end-to-end dry-run test verifies the generated Slides API request.

Changes

Presentation alias compatibility

Layer / File(s) Summary
Shortcut alias normalization
shortcuts/slides/shortcuts.go
Presentation-bearing shortcuts wrap their PostMount handlers with a pflag normalizer that maps supported aliases to presentation without adding visible flags.
Alias parsing and CLI validation
shortcuts/slides/shortcuts_alias_test.go, tests/cli_e2e/slides/slides_presentation_alias_dryrun_test.go
Unit tests validate alias parsing and shortcut wiring; the dry-run test verifies successful GET requests for multiple aliases.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant SlidesShortcut
  participant pflag
  participant SlidesAPI
  CLI->>SlidesShortcut: provide presentation alias
  SlidesShortcut->>pflag: normalize alias to presentation
  SlidesShortcut->>SlidesAPI: issue GET request for presentation
  SlidesAPI-->>CLI: return dry-run request details
Loading

Possibly related PRs

Suggested labels: feature

Suggested reviewers: liangshuo-1

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title accurately and concisely summarizes the main change: normalizing presentation flag aliases in Slides.
Description check ✅ Passed The description matches the template with Summary, Changes, Test Plan, and Related Issues filled in and sufficiently detailed.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/slides-presentation-flag-aliases

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/cli_e2e/slides/slides_presentation_alias_dryrun_test.go`:
- Around line 16-50: Add a live E2E test alongside
TestSlidesPresentationAliasesDryRunE2E that creates a presentation, uses a
representative alias flag (such as --presentation-id) in a real Slides command,
and cleans up the created resource. Keep the existing dry-run alias matrix
unchanged, and assert the live command succeeds using the created presentation
identifier.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 6199fac5-7864-423f-a642-8041ef558012

📥 Commits

Reviewing files that changed from the base of the PR and between b8f56db and dbfe7e1.

📒 Files selected for processing (3)
  • shortcuts/slides/shortcuts.go
  • shortcuts/slides/shortcuts_alias_test.go
  • tests/cli_e2e/slides/slides_presentation_alias_dryrun_test.go

Comment thread tests/cli_e2e/slides/slides_presentation_alias_dryrun_test.go
@github-actions

Copy link
Copy Markdown

🚀 PR Preview Install Guide

🧰 CLI update

npm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@dbfe7e15a53212027c4b1b0ab19e7d36a8183ccc

🧩 Skill update

npx skills add larksuite/cli#fix/slides-presentation-flag-aliases -y -g

@codecov

codecov Bot commented Jul 23, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 89.47368% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.08%. Comparing base (b8f56db) to head (dbfe7e1).

Files with missing lines Patch % Lines
shortcuts/slides/shortcuts.go 89.47% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2032      +/-   ##
==========================================
+ Coverage   75.06%   75.08%   +0.01%     
==========================================
  Files         905      905              
  Lines       96048    96066      +18     
==========================================
+ Hits        72101    72128      +27     
+ Misses      18397    18387      -10     
- Partials     5550     5551       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Comment thread shortcuts/slides/shortcuts.go
@fangshuyu-768
fangshuyu-768 merged commit 1e682bd into main Jul 23, 2026
34 of 46 checks passed
@fangshuyu-768
fangshuyu-768 deleted the fix/slides-presentation-flag-aliases branch July 23, 2026 10:43
@liangshuo-1 liangshuo-1 mentioned this pull request Jul 24, 2026
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/M Single-domain feat or fix with limited business impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants