Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions shortcuts/doc/docs_create_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,11 @@
if runtime.Str("parent-token") != "" && runtime.Str("parent-position") != "" {
return common.FlagErrorf("--parent-token and --parent-position are mutually exclusive")
}
if runtime.Str("doc-format") == "markdown" {
if msg := CheckV2MarkdownCustomTags(runtime.Str("content")); msg != "" {
return common.FlagErrorf("%s", msg)

Check warning on line 32 in shortcuts/doc/docs_create_v2.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/doc/docs_create_v2.go#L31-L32

Added lines #L31 - L32 were not covered by tests
}
}
return nil
}

Expand Down
80 changes: 80 additions & 0 deletions shortcuts/doc/docs_update_check.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,11 @@ import (
// _**text**_ *__text__*
// Lark stores only one of the two emphases (usually italic), silently
// dropping the other. The user wanted both; they will get one.
//
// 3. Whole-paragraph style markers are not rendered by Lark. A line that
// consists entirely of *…* or **…** (no other content) is treated as a
// literal string rather than an italic or bold paragraph. The markers
// appear verbatim in the document instead of applying style.
func docsUpdateWarnings(mode, markdown string) []string {
var warnings []string
if w := checkDocsUpdateReplaceMultilineMarkdown(mode, markdown); w != "" {
Expand All @@ -42,6 +47,9 @@ func docsUpdateWarnings(mode, markdown string) []string {
if w := checkDocsUpdateBoldItalic(markdown); w != "" {
warnings = append(warnings, w)
}
if w := checkDocsUpdateWholeParagraphStyle(markdown); w != "" {
warnings = append(warnings, w)
}
return warnings
}

Expand Down Expand Up @@ -279,3 +287,75 @@ func leadingRun(s string, c byte) string {
}
return s[:i]
}

// wholeParagraphStyleRe matches a line whose entire content is a single
// emphasis span: *…* or **…** (at least one non-whitespace char inside).
// CJK and ASCII content both match; we deliberately do NOT match ***…***
// because that is already covered by checkDocsUpdateBoldItalic.
// wholeParagraphStyleRe matches a line whose entire content is exactly one
// *italic* or **bold** span. Requirements: opener and closer must be the same
// number of asterisks (1 or 2), content has no * or newline, and at least one
// non-whitespace non-asterisk character is present (prevents `* *` matches).
var wholeParagraphStyleRe = regexp.MustCompile(
`^(?:\*[^*\n]*[^\s*\n][^*\n]*\*|\*\*[^*\n]*[^\s*\n][^*\n]*\*\*)$`,
)

// checkDocsUpdateWholeParagraphStyle warns when a markdown paragraph
// consists entirely of *italic* or **bold** markers. Lark's markdown
// parser treats such lines as literal strings (the markers appear verbatim)
// rather than styled paragraphs. This is distinct from inline emphasis
// embedded in mixed-content lines, which Lark handles correctly.
func checkDocsUpdateWholeParagraphStyle(markdown string) string {
if markdown == "" {
return ""
}
sanitized := stripMarkdownCodeRegions(markdown)
for _, line := range strings.Split(sanitized, "\n") {
trimmed := strings.TrimSpace(line)
if wholeParagraphStyleRe.MatchString(trimmed) {
return "line consisting entirely of *…* or **…** markers will not be rendered as italic/bold by Lark; " +
"the markers appear as literal characters. " +
"Mix the styled text with surrounding prose, or use Lark XML: <em>…</em> / <b>…</b>."
}
}
return ""
}

// v2MarkdownUnsupportedTags lists Lark custom XML tags that the v2
// markdown parser does not understand. When present in markdown-mode
// content, the parser silently strips or corrupts them — collapsing grid
// columns, discarding lark-table rows, or escaping text-color tags to
// literal characters. Use --doc-format xml to preserve these constructs.
var v2MarkdownUnsupportedTags = []struct {
prefix string
display string
}{
{"<grid", "<grid>"},
{"<column", "<column>"},
{"<lark-table", "<lark-table>"},
{"<text color=", "<text color=...>"},
{"<text colour=", "<text colour=...>"},
}

// CheckV2MarkdownCustomTags returns a non-empty error message when content
// contains Lark custom tags that the v2 markdown parser silently corrupts.
// Callers in Validate should return this as an error; callers in DryRun
// may choose to surface it as a warning instead.
func CheckV2MarkdownCustomTags(content string) string {
if content == "" {
return ""
}
var found []string
for _, t := range v2MarkdownUnsupportedTags {
if strings.Contains(content, t.prefix) {
found = append(found, t.display)
}
}
if len(found) == 0 {
return ""
}
return "--doc-format markdown does not support Lark custom tags (" +
strings.Join(found, ", ") + "); " +
"the v2 API will silently strip or corrupt them. " +
"Switch to --doc-format xml (the default) to preserve these blocks."
}
Comment on lines +344 to +361
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Custom-tag check ignores fenced code blocks, can hard-fail on legitimate documentation payloads.

checkDocsUpdateWholeParagraphStyle runs stripMarkdownCodeRegions before scanning so that *not a paragraph* inside a ``` fence does not warn. CheckV2MarkdownCustomTags does not — it scans the raw content with strings.Contains. Since the result is wired into v2 create/update Validate (per PR description, returns a hard error), a user pushing markdown that describes these tags inside a code fence (e.g., a tutorial paragraph containing ```html\n<grid cols="2">…\n```) will be blocked even though the parser would render the fence as inert code text rather than corrupt it.

Consider sanitizing the same way the sibling check does so the gating only fires on tags that the v2 parser would actually see as live markup.

🛠️ Proposed fix
 func CheckV2MarkdownCustomTags(content string) string {
 	if content == "" {
 		return ""
 	}
+	scanned := stripMarkdownCodeRegions(content)
 	var found []string
 	for _, t := range v2MarkdownUnsupportedTags {
-		if strings.Contains(content, t.prefix) {
+		if strings.Contains(scanned, t.prefix) {
 			found = append(found, t.display)
 		}
 	}

A regression test for the fenced-code case (parallel to the whole paragraph style inside code fence is ignored case in TestCheckDocsUpdateWholeParagraphStyle) would lock this in.

🤖 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/docs_update_check.go` around lines 344 - 361,
CheckV2MarkdownCustomTags currently scans raw content and can false-positive on
tags inside fenced code; modify it to first call
stripMarkdownCodeRegions(content) (same preprocessing used by
checkDocsUpdateWholeParagraphStyle) and run the strings.Contains checks against
the sanitized output so code fences are ignored, and add a regression test
mirroring the "whole paragraph style inside code fence is ignored" case in
TestCheckDocsUpdateWholeParagraphStyle to ensure tags inside ``` fences do not
trigger the hard validation error.

156 changes: 156 additions & 0 deletions shortcuts/doc/docs_update_check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,19 @@ func TestDocsUpdateWarningsAggregates(t *testing.T) {
}
}

func TestDocsUpdateWarningsWholeParagraphStyle(t *testing.T) {
t.Parallel()

// Whole-paragraph italic should surface via docsUpdateWarnings.
warnings := docsUpdateWarnings("append", "*整段斜体段落*")
if len(warnings) != 1 {
t.Fatalf("expected 1 warning, got %d: %v", len(warnings), warnings)
}
if !strings.Contains(warnings[0], "*…*") {
t.Fatalf("unexpected warning text: %q", warnings[0])
}
}

func TestDocsUpdateWarningsEmpty(t *testing.T) {
t.Parallel()

Expand All @@ -373,3 +386,146 @@ func TestDocsUpdateWarningsEmpty(t *testing.T) {
t.Fatalf("expected no warnings, got: %v", warnings)
}
}

func TestCheckV2MarkdownCustomTags(t *testing.T) {
t.Parallel()

tests := []struct {
name string
content string
wantHint bool
wantTag string
}{
{
name: "empty content produces no warning",
content: "",
wantHint: false,
},
{
name: "plain markdown without custom tags is fine",
content: "# Title\n\nSome **bold** paragraph.",
wantHint: false,
},
{
name: "callout tag is safe (supported by v2)",
content: `<callout emoji="💡">content</callout>`,
wantHint: false,
},
{
name: "grid tag triggers warning",
content: `<grid cols="2"><column width="50">A</column></grid>`,
wantHint: true,
wantTag: "<grid>",
},
{
name: "column tag triggers warning",
content: `<column width="50">col content</column>`,
wantHint: true,
wantTag: "<column>",
},
{
name: "lark-table tag triggers warning",
content: "<lark-table>\n<lark-tr><lark-td>cell</lark-td></lark-tr>\n</lark-table>",
wantHint: true,
wantTag: "<lark-table>",
},
{
name: "text color tag triggers warning",
content: `<text color="red">colored text</text>`,
wantHint: true,
wantTag: "<text color=...>",
},
{
name: "multiple custom tags all appear in message",
content: `<grid cols="2"><lark-table><lark-tr></lark-tr></lark-table></grid>`,
wantHint: true,
wantTag: "<grid>",
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got := CheckV2MarkdownCustomTags(tt.content)
hasHint := got != ""
if hasHint != tt.wantHint {
t.Fatalf("CheckV2MarkdownCustomTags(%q) = %q, wantHint=%v", tt.content, got, tt.wantHint)
}
if tt.wantTag != "" && !strings.Contains(got, tt.wantTag) {
t.Fatalf("expected message to contain %q, got: %q", tt.wantTag, got)
}
})
}
}

func TestCheckDocsUpdateWholeParagraphStyle(t *testing.T) {
t.Parallel()

tests := []struct {
name string
markdown string
wantHint bool
}{
{
name: "plain paragraph no warning",
markdown: "This is a normal paragraph.",
wantHint: false,
},
{
name: "inline bold within mixed line is fine",
markdown: "See **important** detail here.",
wantHint: false,
},
{
name: "whole line italic triggers warning",
markdown: "*为什么大家都选代码?*",
wantHint: true,
},
{
name: "whole line bold triggers warning",
markdown: "**回到最初的问题:要用长期的耐心去打磨。**",
wantHint: true,
},
{
name: "whole line italic inside larger doc triggers warning",
markdown: "# Title\n\nSome prose.\n\n*整段斜体行*\n\nMore prose.",
wantHint: true,
},
{
name: "triple asterisk not matched (covered by bold+italic check)",
markdown: "***text***",
wantHint: false,
},
{
name: "asymmetric markers not matched (*text**)",
markdown: "*text**",
wantHint: false,
},
{
name: "whitespace-only inside markers not matched",
markdown: "* *",
wantHint: false,
},
{
name: "whole paragraph style inside code fence is ignored",
markdown: "```\n*not a paragraph*\n```",
wantHint: false,
},
{
name: "empty markdown no warning",
markdown: "",
wantHint: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
t.Parallel()
got := checkDocsUpdateWholeParagraphStyle(tt.markdown)
hasHint := got != ""
if hasHint != tt.wantHint {
t.Fatalf("checkDocsUpdateWholeParagraphStyle(%q) = %q, wantHint=%v", tt.markdown, got, tt.wantHint)
}
})
}
}
5 changes: 5 additions & 0 deletions shortcuts/doc/docs_update_v2.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@
return common.FlagErrorf("--command append requires --content")
}
}
if runtime.Str("doc-format") == "markdown" && content != "" {
if msg := CheckV2MarkdownCustomTags(content); msg != "" {
return common.FlagErrorf("%s", msg)

Check warning on line 108 in shortcuts/doc/docs_update_v2.go

View check run for this annotation

Codecov / codecov/patch

shortcuts/doc/docs_update_v2.go#L106-L108

Added lines #L106 - L108 were not covered by tests
}
}
return nil
}

Expand Down
Loading