feat: support whiteboard inline file import in docs create/update#1780
Open
SunPeiYang996 wants to merge 3 commits into
Open
feat: support whiteboard inline file import in docs create/update#1780SunPeiYang996 wants to merge 3 commits into
SunPeiYang996 wants to merge 3 commits into
Conversation
Allow `<whiteboard type="plantuml" path="@./diagram.puml">` in --content XML for docs +create and docs +update. CLI reads the file and replaces the tag body with the file content, consistent with the existing html5-block path resolution pattern. - New whiteboard_inline.go: parse, validate, and rewrite whiteboard tags with @path attributes; infer type from file extension (.puml→plantuml, .mmd→mermaid, .svg→svg, other→raw) - Wire into prepareDocsV2WriteInput (html5_block_resources.go) after html5-block processing - Validate that path+inner content are mutually exclusive - Add unit tests for type validation, XML parsing, attribute ops, rendering Change-Id: Icf7f9efbec1385b956dd6d2e797895aaaeab141a
github-actions
Bot
added
domain/ccm
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR adds whiteboard element handling to the docs write pipeline. It validates ChangesWhiteboard inline content feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
🚀 PR Preview Install Guide🧰 CLI updatenpm i -g https://pkg.pr.new/larksuite/cli/@larksuite/cli@622a34c5c144df2a7616b4f087952f3acc70ba4a🧩 Skill updatenpx skills add larksuite/cli#feature/lark-cli-whiteboard-relative-import -y -g |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
shortcuts/doc/whiteboard_inline_test.go (2)
94-104: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOnly the non-self-closing render path is tested.
whiteboardStartTag.renderis called withfalsehere; theSelfClosing/render(true)path is not covered by any test in this file.🤖 Prompt for 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. In `@shortcuts/doc/whiteboard_inline_test.go` around lines 94 - 104, The whiteboard start tag tests only cover the non-self-closing path in whiteboardStartTag.render, so add coverage for the SelfClosing/render(true) branch in the whiteboardStartTag.render tests. Use the existing TestRenderWhiteboardStartTag pattern and the whiteboardStartTag.render method to verify the emitted tag when SelfClosing is enabled, so both rendering paths are exercised.
32-76: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMissing error-path coverage for
parseWhiteboardStartTag.All test cases have
wantErr: false. The function has multiple error branches (tag-name mismatch, decode failure, missing start element) that are never exercised, so a regression there (e.g., silently accepting a non-<whiteboard>tag) would go undetected.✅ Suggested additional case
{ name: "no attributes", raw: `<whiteboard>`, wantErr: false, attrs: map[string]string{}, }, + { + name: "wrong tag name", + raw: `<html5-block path="@./x">`, + wantErr: true, + }, }🤖 Prompt for 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. In `@shortcuts/doc/whiteboard_inline_test.go` around lines 32 - 76, Add negative test coverage to TestParseWhiteboardStartTag for the error branches in parseWhiteboardStartTag: include cases that use a non-whiteboard tag, malformed input that should fail XML decoding, and any input that should produce a missing start element error. Keep the existing table-driven style and assert wantErr is true for these cases so regressions in tag validation and parsing are caught.
🤖 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 `@shortcuts/doc/html5_block_resources.go`:
- Around line 125-128: The whiteboard rewrite helper is shadowing the outer
docType, so the detected type never flows into the rewritten <whiteboard> tag.
Update the logic in the helper that prepares whiteboard inline content so it
assigns to the existing docType instead of redeclaring it, and make sure the tag
rewrite preserves type for path="@..." inputs. Add an end-to-end test around the
write path in prepareWhiteboardInlineContent/docsV2WriteInput to verify the type
attribute is retained.
In `@shortcuts/doc/whiteboard_inline.go`:
- Line 54: The whiteboard rewrite logic in rewriteWhiteboardStartTags only
updates the opening tag, which leaves the closing tag handling inconsistent and
breaks both normal and self-closing forms. Update the whiteboard transformation
path so it rewrites the entire <whiteboard> element consistently, including the
opening tag, embedded content, and closing behavior in the code paths around
rewriteWhiteboardStartTags, the closing-tag append logic, and the self-closing
handling branch. Ensure <whiteboard ...></whiteboard> does not get double-closed
and empty/self-closing whiteboards do not emit malformed extra closing markup.
- Around line 81-109: The inferred whiteboard type is computed in the whiteboard
tag rewrite logic but never written back to the tag, so path-only inputs lose
the type attribute. Update the handling in whiteboard_inline.go around the tag
attribute rewrite so that when docType is inferred from the file extension in
this branch, the tag’s attrs are updated to include type with that inferred
value; keep using the existing validation and rewrite flow in the same
whiteboard rewrite function.
- Line 122: Remove the unreachable return in the relevant function in
whiteboard_inline.go: the earlier return already exits the function, so the
trailing return content, nil should be deleted to satisfy go vet. Verify the
control flow around the existing return in the same function and keep only the
reachable exit path.
- Around line 190-193: The whiteboard file read path in the function handling
`@-prefixed` input currently returns a plain wrapped error from
cmdutil.ReadInputFile, but it should return a typed validation error like the
html5 block resource code. Update the read failure branch in
whiteboard_inline.go to use common.ValidationErrorf with the same user-facing
message and attach the underlying err via WithCause(err), and add a test
covering a missing or unreadable whiteboard file path.
---
Nitpick comments:
In `@shortcuts/doc/whiteboard_inline_test.go`:
- Around line 94-104: The whiteboard start tag tests only cover the
non-self-closing path in whiteboardStartTag.render, so add coverage for the
SelfClosing/render(true) branch in the whiteboardStartTag.render tests. Use the
existing TestRenderWhiteboardStartTag pattern and the whiteboardStartTag.render
method to verify the emitted tag when SelfClosing is enabled, so both rendering
paths are exercised.
- Around line 32-76: Add negative test coverage to TestParseWhiteboardStartTag
for the error branches in parseWhiteboardStartTag: include cases that use a
non-whiteboard tag, malformed input that should fail XML decoding, and any input
that should produce a missing start element error. Keep the existing
table-driven style and assert wantErr is true for these cases so regressions in
tag validation and parsing are caught.
🪄 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
Run ID: ed3a755b-bb16-4fc1-9b6f-031986cbbb40
📒 Files selected for processing (3)
shortcuts/doc/html5_block_resources.goshortcuts/doc/whiteboard_inline.goshortcuts/doc/whiteboard_inline_test.go
Comment on lines
+190
to
+193
| data, err := cmdutil.ReadInputFile(runtime.FileIO(), clean) | ||
| if err != nil { | ||
| return "", fmt.Errorf("%s %q cannot be read from the current working directory; check that the file exists: %w", label, clean, err) | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the local errs API shape and nearby file-I/O wrapping patterns.
rg -n "func ProblemOf|ProblemOf\\(|NewInternalError\\(errs\\.SubtypeFileIO" .
rg -n "ReadInputFile\\(|SubtypeFileIO" shortcuts internal cmdRepository: larksuite/cli
Length of output: 48125
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline shortcuts/doc/whiteboard_inline.go --view expanded
sed -n '1,260p' shortcuts/doc/whiteboard_inline.go
echo '--- internal/cmdutil/resolve.go ---'
sed -n '1,180p' internal/cmdutil/resolve.go
echo '--- nearby patterns ---'
sed -n '390,430p' shortcuts/doc/html5_block_resources.goRepository: larksuite/cli
Length of output: 13674
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- shortcuts/doc/whiteboard_errors.go ---'
sed -n '1,120p' shortcuts/doc/whiteboard_errors.go
echo '--- shortcuts/doc/html5_block_resources.go (path helper) ---'
sed -n '400,430p' shortcuts/doc/html5_block_resources.go
echo '--- shortcuts/doc/whiteboard_inline.go imports + file helper ---'
sed -n '1,220p' shortcuts/doc/whiteboard_inline.go
echo '--- search for ValidationErrorf + WithCause in shortcuts/doc ---'
rg -n "ValidationErrorf\\(.*WithCause\\(|WithCause\\(err\\)" shortcuts/docRepository: larksuite/cli
Length of output: 272
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- common validation/error helpers ---'
rg -n "func ValidationErrorf|type ValidationError|WithCause\\(" shortcuts/common errs internal -g '*.go'
echo '--- file-path readers in shortcuts/doc ---'
rg -n "ReadInputFile\\(|ValidationErrorf\\(|NewInternalError\\(errs\\.SubtypeFileIO" shortcuts/doc -g '*.go'
echo '--- whiteboard-related files ---'
fd -a 'whiteboard*' shortcuts/docRepository: larksuite/cli
Length of output: 13655
Return a typed validation error for unreadable whiteboard files. cmdutil.ReadInputFile already returns a plain wrapped error here, so there’s no typed lower-layer error to pass through; match shortcuts/doc/html5_block_resources.go and use common.ValidationErrorf(...).WithCause(err) instead of fmt.Errorf(...) for @... file reads. Add a test for a missing/unreadable path.
🤖 Prompt for 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.
In `@shortcuts/doc/whiteboard_inline.go` around lines 190 - 193, The whiteboard
file read path in the function handling `@-prefixed` input currently returns a
plain wrapped error from cmdutil.ReadInputFile, but it should return a typed
validation error like the html5 block resource code. Update the read failure
branch in whiteboard_inline.go to use common.ValidationErrorf with the same
user-facing message and attach the underlying err via WithCause(err), and add a
test covering a missing or unreadable whiteboard file path.
Source: Coding guidelines
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
The previous approach matched only the start tag and appended a new closing tag, leaving the original </whiteboard>. Use whiteboardElementReplacer that matches the full element to avoid doubled closing tags. Change-Id: Ib32fd487918e464fa875455690c165cc7d1dca6c
When the whiteboard type is inferred from file extension (e.g. .puml → plantuml), the type attribute must be explicitly added to the tag attrs so the server receives a valid <whiteboard type="plantuml"> tag. Change-Id: Icfb4ec349ba47992be02ff90587e281fda143c73
github-actions
Bot
added
size/M
| } | ||
|
|
||
| tag.removeAttrs("path") | ||
| if docType != "" { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Allow
<whiteboard type="plantuml" path="@./diagram.puml">in--contentXML fordocs +createanddocs +update. CLI reads the file and replaces the tag body with the file content, consistent with the existinghtml5-block path="@..."resolution pattern.Implementation
whiteboard_inline.go: parse, validate, and rewrite<whiteboard>tags with@pathattributes; infer type from file extension (.puml→plantuml,.mmd→mermaid,.svg→svg, other→raw)prepareDocsV2WriteInput(html5_block_resources.go) after html5-block processingTest Plan
lark-cli docs +create --content @./doc.xml --dry-runusing a file containing<whiteboard type="plantuml" path="@./diagram.puml">docs +createAPI call to verify whiteboard content is embedded in the created documentSummary by CodeRabbit