feat(doc): normalize exported sheet and video tags#672
feat(doc): normalize exported sheet and video tags#672HomyeeKing wants to merge 2 commits intolarksuite:mainfrom
Conversation
📝 WalkthroughWalkthroughThis PR introduces a bidirectional content normalization layer for Feishu document operations. It adds a compatibility transformation system that converts Feishu-specific video markup to file tokens on input (create/update operations) and reverses this conversion on output (fetch operations) to maintain format compatibility across different doc formats. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
shortcuts/doc/docs_fetch_v2.go (2)
83-87: Order coupling:<file>regex requirestokenbeforename.
fetchedVideoFileTagReonly matches<file>tags wheretoken="…"appears beforename="…". That happens to match whatnormalizeInputVideoTagsinshortcuts/doc/docs_content_compat.goemits (line 42-49), and the test fixtures all use that order. If the docs_ai export ever serializes attributes in a different order, the file→video rewrite will silently no-op (the tag passes through unchanged) rather than fail — which would be a hard-to-spot regression. Two low-cost options:
- Decouple the regex (match
<file …/>) and pulltoken/nameviafetchedAttrValue, mirroring howview-typeis already extracted at line 150.- Or add a regression test asserting that
<file name="…" token="…"/>is also rewritten, so the coupling is at least pinned.Not blocking since current callers are consistent.
♻️ Sketch: attribute-order-independent file regex
- fetchedVideoFileTagRe = regexp.MustCompile(`<file\s+([^>]*\btoken="([^"]+)"[^>]*\bname="([^"]+)"[^>]*)/>`) + fetchedVideoFileTagRe = regexp.MustCompile(`<file\s+[^>]*/>`)Then in
normalizeFetchedVideoTagsextract viafetchedAttrValue(tag, "token")/fetchedAttrValue(tag, "name")and early-return when either is empty.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/doc/docs_fetch_v2.go` around lines 83 - 87, The <file> tag regex fetchedVideoFileTagRe is order-coupled (requires token before name) and can miss tags with attributes in different orders; update normalizeFetchedVideoTags to use a relaxed regex for fetchedVideoFileTagRe that simply matches <file .../> (mirror how view-type is handled) and then extract token and name using fetchedAttrValue(tag, "token") and fetchedAttrValue(tag, "name"), returning early if either is empty; alternatively add a regression test ensuring a tag like <file name="..." token="..."/> is rewritten to pin the current behavior.
185-203: Optional: consider a single regex over the suffix list.Six
strings.HasSuffixbranches read fine, but a singleswitchoverfilepath.Ext(name)would compress this and make the supported-extensions list scannable. Purely a style/readability nit.♻️ Suggested simplification
func isFetchedVideoFilename(name string) bool { - name = strings.ToLower(strings.TrimSpace(name)) - switch { - case strings.HasSuffix(name, ".mp4"): - return true - case strings.HasSuffix(name, ".mov"): - return true - case strings.HasSuffix(name, ".m4v"): - return true - case strings.HasSuffix(name, ".webm"): - return true - case strings.HasSuffix(name, ".avi"): - return true - case strings.HasSuffix(name, ".mkv"): - return true - default: - return false - } + switch strings.ToLower(filepath.Ext(strings.TrimSpace(name))) { + case ".mp4", ".mov", ".m4v", ".webm", ".avi", ".mkv": + return true + default: + return false + } }Requires adding
"path/filepath"to imports.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/doc/docs_fetch_v2.go` around lines 185 - 203, The isFetchedVideoFilename function currently checks many suffixes with repeated strings.HasSuffix calls; replace that with a single extraction of the file extension (use filepath.Ext(name) after trimming/ToLower) and switch or map over the extension (e.g., case ".mp4", ".mov", etc.) to return true for supported extensions; update imports to include "path/filepath" and keep the existing normalization (strings.TrimSpace/ToLower) before calling filepath.Ext.shortcuts/doc/docs_content_compat.go (1)
12-12: Optional: name overlap with fetch-side regexes.
fetchedVideoTagRehere matches input<video>tags being normalized back into<file/>, whilefetchedVideoFileTagRe/fetchedVideoFigureTagReinshortcuts/doc/docs_fetch_v2.gooperate on fetch output. The sharedfetched*prefix is technically accurate (these tags originate from a prior fetch) but on a quick read it can look like input/output regexes are intermixed. A name likeinputVideoTagRe(or moving all three regexes into a single file with directional prefixesinboundVideoTagRe/outboundVideo*TagRe) would make the direction obvious. No behavior change.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@shortcuts/doc/docs_content_compat.go` at line 12, Rename the ambiguous regex variable fetchedVideoTagRe to a direction-explicit name (e.g., inputVideoTagRe or inboundVideoTagRe) to avoid confusion with fetchedVideoFileTagRe and fetchedVideoFigureTagRe; update the declaration (var fetchedVideoTagRe = ...) and all references/usages across the codebase to the new identifier so the regex that normalizes incoming <video> tags is clearly distinguished from the fetch-output regexes.
🤖 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/doc/docs_content_compat.go`:
- Line 12: Rename the ambiguous regex variable fetchedVideoTagRe to a
direction-explicit name (e.g., inputVideoTagRe or inboundVideoTagRe) to avoid
confusion with fetchedVideoFileTagRe and fetchedVideoFigureTagRe; update the
declaration (var fetchedVideoTagRe = ...) and all references/usages across the
codebase to the new identifier so the regex that normalizes incoming <video>
tags is clearly distinguished from the fetch-output regexes.
In `@shortcuts/doc/docs_fetch_v2.go`:
- Around line 83-87: The <file> tag regex fetchedVideoFileTagRe is order-coupled
(requires token before name) and can miss tags with attributes in different
orders; update normalizeFetchedVideoTags to use a relaxed regex for
fetchedVideoFileTagRe that simply matches <file .../> (mirror how view-type is
handled) and then extract token and name using fetchedAttrValue(tag, "token")
and fetchedAttrValue(tag, "name"), returning early if either is empty;
alternatively add a regression test ensuring a tag like <file name="..."
token="..."/> is rewritten to pin the current behavior.
- Around line 185-203: The isFetchedVideoFilename function currently checks many
suffixes with repeated strings.HasSuffix calls; replace that with a single
extraction of the file extension (use filepath.Ext(name) after trimming/ToLower)
and switch or map over the extension (e.g., case ".mp4", ".mov", etc.) to return
true for supported extensions; update imports to include "path/filepath" and
keep the existing normalization (strings.TrimSpace/ToLower) before calling
filepath.Ext.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2374a61d-e7dc-4673-a740-6e0af88020f1
📒 Files selected for processing (8)
shortcuts/doc/docs_content_compat.goshortcuts/doc/docs_content_compat_test.goshortcuts/doc/docs_create_test.goshortcuts/doc/docs_create_v2.goshortcuts/doc/docs_fetch_v2.goshortcuts/doc/docs_fetch_v2_test.goshortcuts/doc/docs_update_test.goshortcuts/doc/docs_update_v2.go
2 participants
变更说明
sheet-id规范化为id<figure><source .../></figure>规范化为<video>标签导出格式变更对比
电子表格
<sheet sheet-id="jkxrFs" token="FhJTsZVIihsBEStE5RVcr6Fbnrf"></sheet><sheet id="jkxrFs" token="FhJTsZVIihsBEStE5RVcr6Fbnrf"></sheet>视频
<figure view-type="Preview"><source href="..." mime="video/quicktime" origin-height="1892.000000" origin-width="3016.000000" token="RXwJbGRG9orDqhxuYKvch0TqnBf"/></figure><video controls src="feishu://media/RXwJbGRG9orDqhxuYKvch0TqnBf" data-mime="video/quicktime" data-view-type="Preview"></video>测试
go test ./shortcuts/doc