feat(doc): warn when callout uses type= without background-color (#467)

herbertliu · web-flow · commit cfd89e0e2887 · 2026-04-30T17:51:08.000+08:00
* feat(doc): expand callout type= shorthand into background-color and border-color When users write <callout type="warning" emoji="📝"> without an explicit background-color, the Feishu doc renders the block with no color. This commit adds fixCalloutType() which maps the semantic type= attribute to the corresponding background-color/border-color pair accepted by create-doc. - warning → light-yellow/yellow - info/note → light-blue/blue - tip/success/check → light-green/green - error/danger → light-red/red - caution → light-orange/orange - important → light-purple/purple Explicit background-color or border-color attributes are always preserved. The fix is applied via prepareMarkdownForCreate() in both +create and +update paths, and also inside fixExportedMarkdown() for round-trip fidelity. * refactor(doc): replace silent callout type→color injection with hint output Per reviewer feedback (SunPeiYang996), silently rewriting user Markdown is the wrong layer for this adaptation. The type→color mapping is not part of the Feishu spec, and covert transforms make debugging harder. Replace fixCalloutType() (which rewrote the Markdown) with WarnCalloutType() which leaves the Markdown unchanged and instead writes a hint line to stderr for each callout tag that has type= but no background-color, telling the user the recommended explicit attributes to add: hint: callout type="warning" has no background-color; consider: background-color="light-yellow" border-color="yellow" Also fixes CodeRabbit feedback: the type= regex now accepts both single-quoted and double-quoted attribute values (type='warning' and type="warning"). * fix(doc): harden background-color detection in WarnCalloutType CodeRabbit flagged that the previous strings.Contains(attrs, "background-color=") check missed forms like 'background-color = "light-red"' with whitespace around the equals sign. Replace with a regex that tolerates optional whitespace, and add a regression test. * fix(doc): close real review gaps left over after rebase PR #467's review thread had three substantive comments (`fangshuyu-768`, 2026-04-21) that the prior reply messages claimed were fixed in commit 7d4b556 — but that commit no longer exists on the branch (lost in a rebase / squash), and the head still ships the original buggy code. This commit makes the fixes real. Three behavior fixes in shortcuts/doc/markdown_fix.go: 1. (#5) Tighten the type= and background-color= regex anchors. \b sits at any word/non-word boundary, and `-` is a non-word char, so `\btype=` also matched the suffix of `data-type=` — a tag like `<callout data-type="warning">` would emit a bogus light-yellow hint. Switched both regexes to `(?:^|\s)…` so a real attribute separator is required. The same anchor on background-color closes the symmetric case where a `data-background-color=` attribute would silently suppress the real hint. 2. (#4) WarnCalloutType is now a fence-aware line walker. Previously the regex ran over the entire markdown body, so a callout sample inside a documentation code fence (```markdown … ```) would generate a phantom stderr hint every time the docs mentioned the feature. The walker tracks fence state via the existing codeFenceOpenMarker / isCodeFenceClose helpers from docs_update_check.go, which handle both backtick and tilde fences per CommonMark §4.5. 3. (#3) Drop the ReplaceAllStringFunc-as-iterator pattern. The previous code routed callout iteration through a rewrite primitive whose rebuilt-string return value was discarded, then ran the same regex a second time inside the callback to recover the capture groups. New scanCalloutTagsForWarning helper uses FindAllStringSubmatch — one pass, no thrown-away allocation, intent matches the surface (read-only scan, not a mutator). Tests: 5 new TestWarnCalloutType subtests pin each contract: - data-type attribute does not trigger hint (#5) - data-background-color does not suppress hint (#5, symmetric) - callout inside backtick fence emits no hint (#4) - callout inside tilde fence emits no hint (#4) - callout after fence close still emits hint (#4, fence-state reset) All 14 TestWarnCalloutType cases pass; go vet / golangci-lint --new-from-rev=origin/main both clean.
diff --git a/shortcuts/doc/docs_create.go b/shortcuts/doc/docs_create.go
@@ -111,6 +111,11 @@ func dryRunCreateV1(_ context.Context, runtime *common.RuntimeContext) *common.D
 
 func executeCreateV1(_ context.Context, runtime *common.RuntimeContext) error {
 	warnDeprecatedV1(runtime, "+create")
+	// Surface callout type= hint so users know to switch to background-color/
+	// border-color when they want a colored callout. Non-blocking, advisory.
+	if md := runtime.Str("markdown"); md != "" {
+		WarnCalloutType(md, runtime.IO().ErrOut)
+	}
 	args := buildCreateArgsV1(runtime)
 	result, err := common.CallMCPTool(runtime, "create-doc", args)
 	if err != nil {
@@ -123,8 +128,9 @@ func executeCreateV1(_ context.Context, runtime *common.RuntimeContext) error {
 }
 
 func buildCreateArgsV1(runtime *common.RuntimeContext) map[string]interface{} {
+	md := runtime.Str("markdown")
 	args := map[string]interface{}{
-		"markdown": runtime.Str("markdown"),
+		"markdown": md,
 	}
 	if v := runtime.Str("title"); v != "" {
 		args["title"] = v
diff --git a/shortcuts/doc/docs_update.go b/shortcuts/doc/docs_update.go
@@ -160,6 +160,12 @@ func executeUpdateV1(_ context.Context, runtime *common.RuntimeContext) error {
 		fmt.Fprintf(runtime.IO().ErrOut, "warning: %s\n", w)
 	}
 
+	// Surface callout type= hint so users know to switch to background-color/
+	// border-color when they want a colored callout. Non-blocking, advisory.
+	if md := runtime.Str("markdown"); md != "" {
+		WarnCalloutType(md, runtime.IO().ErrOut)
+	}
+
 	args := buildUpdateArgsV1(runtime)
 
 	result, err := common.CallMCPTool(runtime, "update-doc", args)
diff --git a/shortcuts/doc/markdown_fix.go b/shortcuts/doc/markdown_fix.go
@@ -4,6 +4,8 @@
 package doc
 
 import (
+	"fmt"
+	"io"
 	"regexp"
 	"strings"
 	"unicode"
@@ -306,6 +308,113 @@ func fixSetextAmbiguity(md string) string {
 	return setextRe.ReplaceAllString(md, "$1\n\n$2")
 }
 
+// calloutTypeColors maps the semantic type= shorthand to a recommended
+// [background-color, border-color] pair for Feishu callout blocks.
+// Used only for hint messages — the Markdown itself is never rewritten.
+var calloutTypeColors = map[string][2]string{
+	"warning":   {"light-yellow", "yellow"},
+	"caution":   {"light-orange", "orange"},
+	"note":      {"light-blue", "blue"},
+	"info":      {"light-blue", "blue"},
+	"tip":       {"light-green", "green"},
+	"success":   {"light-green", "green"},
+	"check":     {"light-green", "green"},
+	"error":     {"light-red", "red"},
+	"danger":    {"light-red", "red"},
+	"important": {"light-purple", "purple"},
+}
+
+// calloutOpenTagRe matches a <callout …> opening tag.
+var calloutOpenTagRe = regexp.MustCompile(`<callout(\s[^>]*)?>`)
+
+// calloutTypeAttrRe extracts the value of a type= attribute (single or
+// double quoted) from a callout opening tag's attribute string. The
+// (?:^|\s) anchor instead of \b is intentional: \b sits at any
+// word/non-word boundary, and `-` is a non-word character, so
+// `\btype=` would also match the suffix of `data-type=` and yield a
+// bogus type lookup. Anchoring on start-of-string-or-whitespace
+// requires a real attribute separator before the name.
+var calloutTypeAttrRe = regexp.MustCompile(`(?:^|\s)type=(?:"([^"]*)"|'([^']*)')`)
+
+// calloutBackgroundColorAttrRe matches a background-color= attribute
+// name with optional whitespace around the equals sign, so forms like
+// `background-color="..."` and `background-color = "..."` are both
+// accepted. Same (?:^|\s) anchor as calloutTypeAttrRe, for the same
+// reason: `data-background-color="..."` must not look like a present
+// background-color and silently suppress the hint.
+var calloutBackgroundColorAttrRe = regexp.MustCompile(`(?:^|\s)background-color\s*=`)
+
+// WarnCalloutType scans md for callout tags that carry a type= attribute but
+// no background-color= attribute, then writes a hint line to w for each one
+// suggesting the explicit Feishu color attributes to use instead.
+//
+// Callout tags inside fenced code blocks (``` or ~~~) are skipped — they
+// are documentation samples, not real callouts the user wants Feishu to
+// render. Fence detection uses the shared codeFenceOpenMarker /
+// isCodeFenceClose helpers so both backtick and tilde fences are handled
+// (matching CommonMark §4.5).
+//
+// The Markdown is not modified — the caller is responsible for acting on
+// the hints or ignoring them. This keeps the create/update path
+// transparent: user input reaches create-doc exactly as written.
+func WarnCalloutType(md string, w io.Writer) {
+	fenceMarker := ""
+	for _, line := range strings.Split(md, "\n") {
+		if fenceMarker != "" {
+			// Inside a fenced block — skip everything until the matching
+			// closer. Code samples that show literal <callout type=...>
+			// must not produce a phantom hint.
+			if isCodeFenceClose(line, fenceMarker) {
+				fenceMarker = ""
+			}
+			continue
+		}
+		if marker := codeFenceOpenMarker(line); marker != "" {
+			fenceMarker = marker
+			continue
+		}
+		scanCalloutTagsForWarning(line, w)
+	}
+}
+
+// scanCalloutTagsForWarning emits a hint to w for every <callout type="...">
+// tag in s that lacks an explicit background-color= attribute. Pulled out
+// of WarnCalloutType so the line walker only handles fence state and the
+// per-tag scan is its own readable unit.
+//
+// The previous implementation routed the tag iteration through
+// calloutOpenTagRe.ReplaceAllStringFunc with a callback that always
+// returned the original tag and threw the rebuilt string away — using a
+// rewrite primitive purely for its iteration side-effect, plus a second
+// regex execution to recover the capture groups inside the callback.
+// FindAllStringSubmatch hands us both the iteration and the groups in one
+// pass, no allocation thrown away.
+func scanCalloutTagsForWarning(s string, w io.Writer) {
+	for _, m := range calloutOpenTagRe.FindAllStringSubmatch(s, -1) {
+		attrs := m[1]
+		// Skip tags that already carry an explicit background-color.
+		if calloutBackgroundColorAttrRe.MatchString(attrs) {
+			continue
+		}
+		parts := calloutTypeAttrRe.FindStringSubmatch(attrs)
+		if len(parts) < 3 {
+			continue // no type= attribute
+		}
+		// parts[1] is the double-quoted capture, parts[2] is single-quoted.
+		typeName := parts[1]
+		if typeName == "" {
+			typeName = parts[2]
+		}
+		colors, ok := calloutTypeColors[typeName]
+		if !ok {
+			continue // unknown type — no hint to give
+		}
+		fmt.Fprintf(w,
+			"hint: callout type=%q has no background-color; consider: background-color=%q border-color=%q\n",
+			typeName, colors[0], colors[1])
+	}
+}
+
 // calloutEmojiAliases maps named emoji strings that fetch-doc emits to actual
 // Unicode emoji characters that create-doc accepts.
 var calloutEmojiAliases = map[string]string{
diff --git a/shortcuts/doc/markdown_fix_test.go b/shortcuts/doc/markdown_fix_test.go
@@ -359,6 +359,135 @@ func TestFixExportedMarkdown(t *testing.T) {
 	}
 }
 
+func TestWarnCalloutType(t *testing.T) {
+	tests := []struct {
+		name         string
+		input        string
+		wantHint     bool   // whether a hint line is expected
+		hintContains string // substring the hint must contain
+	}{
+		{
+			name:         "warning type without background-color emits hint",
+			input:        `<callout type="warning" emoji="📝">`,
+			wantHint:     true,
+			hintContains: `background-color="light-yellow"`,
+		},
+		{
+			name:         "info type without background-color emits hint",
+			input:        `<callout type="info" emoji="ℹ️">`,
+			wantHint:     true,
+			hintContains: `background-color="light-blue"`,
+		},
+		{
+			name:         "single-quoted type attribute emits hint",
+			input:        `<callout type='warning' emoji="📝">`,
+			wantHint:     true,
+			hintContains: `background-color="light-yellow"`,
+		},
+		{
+			name:     "explicit background-color suppresses hint",
+			input:    `<callout type="warning" emoji="📝" background-color="light-red">`,
+			wantHint: false,
+		},
+		{
+			name:     "whitespace around equals is tolerated in background-color",
+			input:    `<callout type="warning" emoji="📝" background-color = "light-red">`,
+			wantHint: false,
+		},
+		{
+			name:     "unknown type emits no hint",
+			input:    `<callout type="custom" emoji="🔥">`,
+			wantHint: false,
+		},
+		{
+			name:     "no type attribute emits no hint",
+			input:    `<callout emoji="💡" background-color="light-green">`,
+			wantHint: false,
+		},
+		{
+			name:     "non-callout tag emits no hint",
+			input:    `<div type="warning">`,
+			wantHint: false,
+		},
+		{
+			name:         "hint includes border-color suggestion",
+			input:        `<callout type="error" emoji="❌">`,
+			wantHint:     true,
+			hintContains: `border-color="red"`,
+		},
+		{
+			// Regression: the old `\btype=` regex matched the suffix of
+			// `data-type=` because `-` is a non-word character, so a tag
+			// carrying only data-attrs would silently get a bogus hint.
+			// The (?:^|\s) anchor requires a real attribute separator.
+			name:     "data-type attribute does not trigger hint",
+			input:    `<callout data-type="warning" emoji="📝">`,
+			wantHint: false,
+		},
+		{
+			// Symmetric guard for the background-color regex: a future
+			// `data-background-color=` attribute must not be mistaken
+			// for a present background-color and silently suppress the
+			// hint that the real type= would otherwise produce.
+			name:         "data-background-color does not suppress hint",
+			input:        `<callout type="warning" data-background-color="anything">`,
+			wantHint:     true,
+			hintContains: `background-color="light-yellow"`,
+		},
+		{
+			// Regression for the code-fence skip: a documentation sample
+			// inside a ``` fence is NOT a real callout the user wants
+			// rendered, so it must produce no stderr noise.
+			name: "callout inside backtick fence emits no hint",
+			input: "```markdown\n" +
+				`<callout type="warning" emoji="📝">` + "\n" +
+				"```\n",
+			wantHint: false,
+		},
+		{
+			// Same skip works for tilde fences (CommonMark §4.5 makes
+			// `~~~` an equivalent fence character).
+			name: "callout inside tilde fence emits no hint",
+			input: "~~~markdown\n" +
+				`<callout type="info" emoji="ℹ️">` + "\n" +
+				"~~~\n",
+			wantHint: false,
+		},
+		{
+			// Closing the fence must restore normal scanning: a real
+			// callout that follows a documentation block still gets a
+			// hint. Pins that fenceMarker is reset, not stuck.
+			name: "callout after fence close still emits hint",
+			input: "```markdown\n" +
+				`<callout type="warning">sample</callout>` + "\n" +
+				"```\n" +
+				`<callout type="error" emoji="❌">real</callout>` + "\n",
+			wantHint:     true,
+			hintContains: `border-color="red"`,
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			var buf strings.Builder
+			WarnCalloutType(tt.input, &buf)
+			got := buf.String()
+			if tt.wantHint {
+				if got == "" {
+					t.Errorf("WarnCalloutType(%q): expected hint, got no output", tt.input)
+					return
+				}
+				if tt.hintContains != "" && !strings.Contains(got, tt.hintContains) {
+					t.Errorf("WarnCalloutType(%q): hint %q missing %q", tt.input, got, tt.hintContains)
+				}
+			} else {
+				if got != "" {
+					t.Errorf("WarnCalloutType(%q): expected no output, got %q", tt.input, got)
+				}
+			}
+		})
+	}
+}
+
 func TestFixCalloutEmoji(t *testing.T) {
 	tests := []struct {
 		name  string
