Skip to content

Commit f77b7ee

Browse files
BD-ZEROBD-ZERO
authored
fix(slides): support CSV multi-value for --slide-id in screenshot (#2047)
--slide-id used the cobra StringArray flag type, which only accepts repeated flags and does not split comma-separated values, unlike --slide-number (int_array -> cobra IntSlice) which already supported CSV input. This made the two selector flags inconsistent. Switch --slide-id to the string_slice flag type (cobra StringSlice), which natively supports both comma-separated and repeated values, and update the flag readers from StrArray to StrSlice. normalizeSlideIDs already trims/dedupes/filters blanks, and validateSlidesScreenshotSelectorLimit already caps the combined selector count, so both continue to apply unchanged to CSV input. Add tests covering --slide-id CSV parsing, whitespace/duplicate normalization, and the >10 selector limit via CSV, mirroring the existing --slide-number coverage. Address review feedback: - Fix "comma-separate" -> "comma-separated" wording in the --slide-id flag description (CodeRabbit). - Set LARKSUITE_CLI_CONFIG_DIR to t.TempDir() in the new screenshot tests, per the AGENTS.md testing convention, so local configuration state cannot leak into or be modified by the suite. - Add a dry-run E2E test (tests/cli_e2e/slides) that pins --slide-id CSV parsing through the built CLI binary and asserts the emitted slide_ids request body, per the AGENTS.md dry-run E2E requirement for shortcut flag/param changes. - Update the lark-slides skill reference to document that --slide-id and --slide-number both accept comma-separated values, not just repeated flags, so agents can discover the new syntax.
1 parent dd7f741 commit f77b7ee

4 files changed

Lines changed: 213 additions & 10 deletions

File tree

‎shortcuts/slides/slides_screenshot.go‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ var SlidesScreenshot = common.Shortcut{
4343
AuthTypes: []string{"user", "bot"},
4444
Flags: []common.Flag{
4545
{Name: "presentation", Desc: "xml_presentation_id, slides URL, or wiki URL that resolves to slides; list mode only"},
46-
{Name: "slide-id", Type: "string_array", Desc: "slide page identifier (repeat for multiple slides; max 10 pages per request)"},
46+
{Name: "slide-id", Type: "string_slice", Desc: "slide page identifier (repeat or comma-separated for multiple slides; max 10 pages per request)"},
4747
{Name: "slide-number", Type: "int_array", Desc: "slide page number (repeat for multiple slides; max 10 pages per request)"},
4848
{Name: "content", Desc: "slide XML content to render directly instead of fetching existing slides", Input: []string{common.File, common.Stdin}},
4949
{Name: "output-dir", Default: defaultSlidesScreenshotDir, Desc: "relative directory for saved screenshots"},
@@ -55,7 +55,7 @@ var SlidesScreenshot = common.Shortcut{
5555
if strings.TrimSpace(runtime.Str("content")) == "" {
5656
return slidesScreenshotFlagErrorf("--content cannot be empty")
5757
}
58-
if len(normalizeSlideIDs(runtime.StrArray("slide-id"))) > 0 || len(runtime.IntArray("slide-number")) > 0 {
58+
if len(normalizeSlideIDs(runtime.StrSlice("slide-id"))) > 0 || len(runtime.IntArray("slide-number")) > 0 {
5959
return slidesScreenshotFlagErrorf("--content cannot be used with --slide-id or --slide-number")
6060
}
6161
if runtime.Changed("presentation") {
@@ -71,7 +71,7 @@ var SlidesScreenshot = common.Shortcut{
7171
return err
7272
}
7373
}
74-
slideIDs := normalizeSlideIDs(runtime.StrArray("slide-id"))
74+
slideIDs := normalizeSlideIDs(runtime.StrSlice("slide-id"))
7575
slideNumbers, err := normalizeSlideNumbers(runtime.IntArray("slide-number"))
7676
if err != nil {
7777
return err
@@ -96,7 +96,7 @@ var SlidesScreenshot = common.Shortcut{
9696
if err != nil {
9797
return common.NewDryRunAPI().Set("error", err.Error())
9898
}
99-
slideIDs := normalizeSlideIDs(runtime.StrArray("slide-id"))
99+
slideIDs := normalizeSlideIDs(runtime.StrSlice("slide-id"))
100100
slideNumbers, err := normalizeSlideNumbers(runtime.IntArray("slide-number"))
101101
if err != nil {
102102
return common.NewDryRunAPI().Set("error", err.Error())
@@ -146,7 +146,7 @@ var SlidesScreenshot = common.Shortcut{
146146
return err
147147
}
148148

149-
slideIDs := normalizeSlideIDs(runtime.StrArray("slide-id"))
149+
slideIDs := normalizeSlideIDs(runtime.StrSlice("slide-id"))
150150
slideNumbers, err := normalizeSlideNumbers(runtime.IntArray("slide-number"))
151151
if err != nil {
152152
return err
@@ -198,7 +198,7 @@ func dryRunRenderScreenshot(runtime *common.RuntimeContext) *common.DryRunAPI {
198198
if strings.TrimSpace(content) == "" {
199199
return common.NewDryRunAPI().Set("error", "--content cannot be empty")
200200
}
201-
if len(normalizeSlideIDs(runtime.StrArray("slide-id"))) > 0 || len(runtime.IntArray("slide-number")) > 0 {
201+
if len(normalizeSlideIDs(runtime.StrSlice("slide-id"))) > 0 || len(runtime.IntArray("slide-number")) > 0 {
202202
return common.NewDryRunAPI().Set("error", "--content cannot be used with --slide-id or --slide-number")
203203
}
204204
if runtime.Changed("presentation") {
@@ -217,7 +217,7 @@ func executeRenderScreenshot(runtime *common.RuntimeContext) error {
217217
if strings.TrimSpace(content) == "" {
218218
return slidesScreenshotFlagErrorf("--content cannot be empty")
219219
}
220-
if len(normalizeSlideIDs(runtime.StrArray("slide-id"))) > 0 || len(runtime.IntArray("slide-number")) > 0 {
220+
if len(normalizeSlideIDs(runtime.StrSlice("slide-id"))) > 0 || len(runtime.IntArray("slide-number")) > 0 {
221221
return slidesScreenshotFlagErrorf("--content cannot be used with --slide-id or --slide-number")
222222
}
223223
if runtime.Changed("presentation") {

‎shortcuts/slides/slides_screenshot_test.go‎

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -185,6 +185,139 @@ func TestSlidesScreenshotListBySlideNumber(t *testing.T) {
185185
}
186186
}
187187

188+
func TestSlidesScreenshotListBySlideIDCSV(t *testing.T) {
189+
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())
190+
dir := t.TempDir()
191+
withSlidesTestWorkingDir(t, dir)
192+
193+
f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, ""))
194+
stub := &httpmock.Stub{
195+
Method: "POST",
196+
URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide_images",
197+
Body: map[string]interface{}{
198+
"code": 0,
199+
"data": map[string]interface{}{
200+
"slide_images": []map[string]interface{}{
201+
{
202+
"slide_id": "slide_1",
203+
"format": 1,
204+
"data": base64.StdEncoding.EncodeToString([]byte("png-bytes-1")),
205+
},
206+
{
207+
"slide_id": "slide_2",
208+
"format": 1,
209+
"data": base64.StdEncoding.EncodeToString([]byte("png-bytes-2")),
210+
},
211+
},
212+
},
213+
},
214+
}
215+
reg.Register(stub)
216+
217+
err := runSlidesShortcut(t, f, stdout, SlidesScreenshot, []string{
218+
"+screenshot",
219+
"--presentation", "pres_abc",
220+
"--slide-id", "slide_1,slide_2",
221+
"--as", "user",
222+
})
223+
if err != nil {
224+
t.Fatalf("unexpected error: %v", err)
225+
}
226+
227+
var body struct {
228+
SlideIDs []string `json:"slide_ids"`
229+
}
230+
if err := json.Unmarshal(stub.CapturedBody, &body); err != nil {
231+
t.Fatalf("decode request body: %v", err)
232+
}
233+
if len(body.SlideIDs) != 2 || body.SlideIDs[0] != "slide_1" || body.SlideIDs[1] != "slide_2" {
234+
t.Fatalf("slide_ids = %#v, want [slide_1 slide_2]", body.SlideIDs)
235+
}
236+
237+
path1 := filepath.Join(dir, defaultSlidesScreenshotDir, "pres_abc_slide_1.png")
238+
if _, err := os.ReadFile(path1); err != nil {
239+
t.Fatalf("read first CSV slide screenshot: %v", err)
240+
}
241+
path2 := filepath.Join(dir, defaultSlidesScreenshotDir, "pres_abc_slide_2.png")
242+
if _, err := os.ReadFile(path2); err != nil {
243+
t.Fatalf("read second CSV slide screenshot: %v", err)
244+
}
245+
}
246+
247+
func TestSlidesScreenshotListBySlideIDCSVDeduplicatesAndTrims(t *testing.T) {
248+
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())
249+
dir := t.TempDir()
250+
withSlidesTestWorkingDir(t, dir)
251+
252+
f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, ""))
253+
stub := &httpmock.Stub{
254+
Method: "POST",
255+
URL: "/open-apis/slides_ai/v1/xml_presentations/pres_abc/slide_images",
256+
Body: map[string]interface{}{
257+
"code": 0,
258+
"data": map[string]interface{}{
259+
"slide_images": []map[string]interface{}{
260+
{
261+
"slide_id": "slide_1",
262+
"format": 1,
263+
"data": base64.StdEncoding.EncodeToString([]byte("png-bytes-1")),
264+
},
265+
{
266+
"slide_id": "slide_2",
267+
"format": 1,
268+
"data": base64.StdEncoding.EncodeToString([]byte("png-bytes-2")),
269+
},
270+
},
271+
},
272+
},
273+
}
274+
reg.Register(stub)
275+
276+
// CSV with a duplicate and blank segments should normalize the same way
277+
// normalizeSlideIDs already does for repeated --slide-id flags.
278+
err := runSlidesShortcut(t, f, stdout, SlidesScreenshot, []string{
279+
"+screenshot",
280+
"--presentation", "pres_abc",
281+
"--slide-id", "slide_1, slide_2,slide_1,",
282+
"--as", "user",
283+
})
284+
if err != nil {
285+
t.Fatalf("unexpected error: %v", err)
286+
}
287+
288+
var body struct {
289+
SlideIDs []string `json:"slide_ids"`
290+
}
291+
if err := json.Unmarshal(stub.CapturedBody, &body); err != nil {
292+
t.Fatalf("decode request body: %v", err)
293+
}
294+
if len(body.SlideIDs) != 2 || body.SlideIDs[0] != "slide_1" || body.SlideIDs[1] != "slide_2" {
295+
t.Fatalf("slide_ids = %#v, want deduplicated [slide_1 slide_2]", body.SlideIDs)
296+
}
297+
}
298+
299+
func TestSlidesScreenshotListRejectsMoreThanTenSlideIDsCSV(t *testing.T) {
300+
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())
301+
f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, ""))
302+
303+
err := runSlidesShortcut(t, f, stdout, SlidesScreenshot, []string{
304+
"+screenshot",
305+
"--presentation", "pres_abc",
306+
"--slide-id", "s1,s2,s3,s4,s5,s6,s7,s8,s9,s10,s11",
307+
"--as", "user",
308+
})
309+
if err == nil {
310+
t.Fatal("expected error")
311+
}
312+
problem, ok := errs.ProblemOf(err)
313+
if !ok {
314+
t.Fatalf("error = %v, want typed validation error", err)
315+
}
316+
if problem.Hint != "request at most 10 pages at a time" {
317+
t.Fatalf("hint = %q, want max 10 pages guidance", problem.Hint)
318+
}
319+
}
320+
188321
func TestSlidesScreenshotAvoidsOverwritingExistingFile(t *testing.T) {
189322
dir := t.TempDir()
190323
withSlidesTestWorkingDir(t, dir)
@@ -387,6 +520,27 @@ func TestSlidesScreenshotRenderRejectsSlideSelectors(t *testing.T) {
387520
}
388521
}
389522

523+
func TestSlidesScreenshotRenderRejectsSlideNumberSelector(t *testing.T) {
524+
t.Setenv("LARKSUITE_CLI_CONFIG_DIR", t.TempDir())
525+
f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, ""))
526+
527+
// Exercises the --slide-number-only side of the --content conflict check
528+
// (TestSlidesScreenshotRenderRejectsSlideSelectors above only covers the
529+
// --slide-id side of that same `||` condition).
530+
err := runSlidesShortcut(t, f, stdout, SlidesScreenshot, []string{
531+
"+screenshot",
532+
"--content", `<slide xmlns="http://www.larkoffice.com/sml/2.0"><data></data></slide>`,
533+
"--slide-number", "1",
534+
"--as", "user",
535+
})
536+
if err == nil {
537+
t.Fatal("expected error")
538+
}
539+
if !strings.Contains(err.Error(), "--content cannot be used with --slide-id or --slide-number") {
540+
t.Fatalf("error = %v, want content/slide selector conflict", err)
541+
}
542+
}
543+
390544
func TestSlidesScreenshotRenderRejectsListOnlyFlags(t *testing.T) {
391545
f, stdout, _, _ := cmdutil.TestFactory(t, slidesTestConfig(t, ""))
392546

‎skills/lark-slides/references/lark-slides-screenshot.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,8 @@ lark-cli slides +screenshot --as user \
2626
| 参数 | 必需 | 说明 |
2727
|------|------|------|
2828
| `--presentation` | list 模式必需 | `xml_presentation_id`、`/slides/` URL，或解析后为 slides 的 `/wiki/` URL。传 `--content` 时不能使用 |
29-
| `--slide-id` | list 模式至少提供 `--slide-id` / `--slide-number` 之一 | 页面 short ID；多页截图时重复传入；一次最多 10 页（`--slide-id` + `--slide-number` 合计小于等于 10） |
30-
| `--slide-number` | list 模式至少提供 `--slide-id` / `--slide-number` 之一 | 页面页号；多页截图时重复传入；一次最多 10 页（`--slide-id` + `--slide-number` 合计小于等于 10） |
29+
| `--slide-id` | list 模式至少提供 `--slide-id` / `--slide-number` 之一 | 页面 short ID；多页截图时重复传入，或用逗号分隔一次传多个（如 `--slide-id slide_1,slide_2`）；一次最多 10 页（`--slide-id` + `--slide-number` 合计小于等于 10） |
30+
| `--slide-number` | list 模式至少提供 `--slide-id` / `--slide-number` 之一 | 页面页号；多页截图时重复传入，或用逗号分隔一次传多个（如 `--slide-number 1,2,3`）；一次最多 10 页（`--slide-id` + `--slide-number` 合计小于等于 10） |
3131
| `--content` | render 模式必需 | 要直接渲染的 `<slide>` XML 片段；支持直接传值、`@file`、`-` stdin。传入后不能同时传 `--slide-id` / `--slide-number` |
3232
| `--output-dir` | 否 | 输出目录，默认 `.lark-slides/screenshots`；必须是当前目录内的相对路径 |
3333
| `--output-name` | 否 | render 模式的输出文件名 stem；未指定时优先用返回的 `slide_id`，否则用 `rendered-slide`。若目标文件已存在，会自动追加递增后缀避免覆盖 |
@@ -44,7 +44,7 @@ lark-cli slides +screenshot --as user \
4444

4545
### 多页截图
4646

47-
一次不要超过 10 页；如需更多页面，分批调用。
47+
一次不要超过 10 页；如需更多页面，分批调用。可以重复传参，也可以用逗号分隔一次传多个：
4848

4949
```bash
5050
lark-cli slides +screenshot --as user \

‎tests/cli_e2e/slides/slides_screenshot_dryrun_test.go‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
// Copyright (c) 2026 Lark Technologies Pte. Ltd.
2+
// SPDX-License-Identifier: MIT
3+
4+
package slides
5+
6+
import (
7+
"context"
8+
"testing"
9+
"time"
10+
11+
clie2e "github.com/larksuite/cli/tests/cli_e2e"
12+
"github.com/stretchr/testify/require"
13+
"github.com/tidwall/gjson"
14+
)
15+
16+
// TestSlidesScreenshotSlideIDCSVDryRunE2E pins the CSV multi-value parsing for
17+
// --slide-id through the built CLI: a single comma-separated flag value must
18+
// expand into the same slide_ids request body that repeating the flag would
19+
// produce.
20+
func TestSlidesScreenshotSlideIDCSVDryRunE2E(t *testing.T) {
21+
setSlidesDryRunEnv(t)
22+
23+
ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)
24+
t.Cleanup(cancel)
25+
26+
result, err := clie2e.RunCmd(ctx, clie2e.Request{
27+
Args: []string{
28+
"slides", "+screenshot",
29+
"--presentation", "presScreenshotDryRun",
30+
"--slide-id", "slide_1,slide_2",
31+
"--dry-run",
32+
},
33+
DefaultAs: "bot",
34+
})
35+
require.NoError(t, err)
36+
result.AssertExitCode(t, 0)
37+
38+
require.Equal(t, "POST", gjson.Get(result.Stdout, "data.api.0.method").String(), result.Stdout)
39+
require.Equal(t,
40+
"/open-apis/slides_ai/v1/xml_presentations/presScreenshotDryRun/slide_images",
41+
gjson.Get(result.Stdout, "data.api.0.url").String(),
42+
result.Stdout,
43+
)
44+
45+
slideIDs := gjson.Get(result.Stdout, "data.api.0.body.slide_ids").Array()
46+
require.Len(t, slideIDs, 2, result.Stdout)
47+
require.Equal(t, "slide_1", slideIDs[0].String(), result.Stdout)
48+
require.Equal(t, "slide_2", slideIDs[1].String(), result.Stdout)
49+
}

0 commit comments

Comments
 (0)