From 7f9c6d9bfb91f057bbe9f171d95340a6d75b2603 Mon Sep 17 00:00:00 2001 From: niezhiwei Date: Thu, 23 Jul 2026 01:56:10 +0800 Subject: [PATCH] fix(slides): reindent xml-get output with a stdlib-only formatter MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reland of #1987 (reverted in #2013 over the BSD attribution gap of github.com/beevik/etree) with the formatter rebuilt on the Go standard library only — no third-party dependency. encoding/xml serves purely as a tokenizer; the output is assembled exclusively from verbatim byte slices of the server content plus indentation inserted between structural elements. Nothing is parsed-and-reserialized, so CDATA sections, whitespace character references in any spelling, entity lexical forms, attribute quoting, and in-tag whitespace survive byte-for-byte — the character-reference masking machinery of the etree implementation is no longer needed. Behavior is unchanged from the reverted PR: --raw stdout and --output files are reindented (never inside the schema's mixed-content text-bearing elements), the default JSON envelope carries the server's XML verbatim without parsing, and formatting failures fall back to the original content with a stderr warning and pretty_printed: false in --output file metadata. All contract tests carry over unweakened; a differential probe against the etree implementation over 53 inputs was byte-identical except six cases where the new formatter preserves the original bytes more faithfully (each pinned in tests). --- shortcuts/slides/slides_xml_get.go | 45 +- shortcuts/slides/slides_xml_get_test.go | 407 ++++++++++++++++- shortcuts/slides/slides_xml_prettyprint.go | 260 +++++++++++ .../slides/slides_xml_prettyprint_test.go | 416 ++++++++++++++++++ 4 files changed, 1104 insertions(+), 24 deletions(-) create mode 100644 shortcuts/slides/slides_xml_prettyprint.go create mode 100644 shortcuts/slides/slides_xml_prettyprint_test.go diff --git a/shortcuts/slides/slides_xml_get.go b/shortcuts/slides/slides_xml_get.go index 8a18b0997b..0f07a1fdce 100644 --- a/shortcuts/slides/slides_xml_get.go +++ b/shortcuts/slides/slides_xml_get.go @@ -16,9 +16,10 @@ import ( ) // SlidesXMLGet fetches the full XML presentation content. When --output is -// provided it writes to a local file; otherwise it returns the XML in the -// standard JSON envelope. Use --slide-id or --slide-number to fetch one page, -// and use --raw for direct XML stdout. +// provided it writes reindented XML to a local file, and --raw prints +// reindented XML to stdout; otherwise it returns the server's original +// content unmodified in the standard JSON envelope. Use --slide-id or +// --slide-number to fetch one page. var SlidesXMLGet = common.Shortcut{ Service: "slides", Command: "+xml-get", @@ -30,8 +31,8 @@ var SlidesXMLGet = common.Shortcut{ AuthTypes: []string{"user", "bot"}, Flags: []common.Flag{ {Name: "presentation", Desc: "xml_presentation_id, slides URL, or wiki URL that resolves to slides", Required: true}, - {Name: "output", Desc: "local XML output path; must be a relative path within the current directory; existing file is overwritten; omit to return XML in the JSON envelope"}, - {Name: "raw", Type: "bool", Desc: "print raw XML to stdout instead of the JSON envelope; incompatible with --output and --jq"}, + {Name: "output", Desc: "local XML output path; the saved file is formatted for readability; must be a relative path within the current directory; existing file is overwritten; omit to return the server's original XML in the JSON envelope"}, + {Name: "raw", Type: "bool", Desc: "print formatted XML to stdout without the JSON envelope; incompatible with --output and --jq"}, {Name: "slide-id", Desc: "slide page identifier; omit both slide selectors to fetch full presentation XML"}, {Name: "slide-number", Type: "int", Desc: "1-based slide page number; omit both slide selectors to fetch full presentation XML"}, {Name: "revision-id", Type: "int", Default: "-1", Desc: "presentation revision_id; -1 means latest"}, @@ -108,10 +109,10 @@ var SlidesXMLGet = common.Shortcut{ } dry.GET(path).Params(params) if outputPath := strings.TrimSpace(runtime.Str("output")); outputPath != "" { - return dry.Set("output", outputPath).Set("stdout_content", "suppressed; XML content is saved to --output during execution") + return dry.Set("output", outputPath).Set("stdout_content", "suppressed; formatted XML content is saved to --output during execution") } if runtime.Bool("raw") { - return dry.Set("output", "").Set("stdout_content", "raw XML content is printed to stdout during execution") + return dry.Set("output", "").Set("stdout_content", "formatted XML content is printed to stdout during execution") } return dry.Set("output", "").Set("stdout_content", "JSON envelope with XML content is printed to stdout during execution") }, @@ -250,22 +251,31 @@ func fetchSlidesXMLGetContent(runtime *common.RuntimeContext, presentationID str return content, out, nil } +// outputSlidesXMLGetContent routes the fetched XML to its output surface. +// Only the text surfaces are reindented: --raw stdout and --output files are +// read directly by humans and line tools. The JSON envelope carries the +// server content verbatim instead -- inside a JSON string every newline is +// escaped to \n, so formatting there buys no readability and only inflates +// the payload, while passthrough keeps that read path byte-exact without +// even parsing the content. func outputSlidesXMLGetContent(runtime *common.RuntimeContext, content string, outputPath string, out map[string]interface{}) error { if outputPath == "" { if !runtime.Bool("raw") { runtime.OutFormatRaw(out, nil, nil) return nil } - if _, err := fmt.Fprint(runtime.IO().Out, content); err != nil { + formatted, _ := prettyPrintXMLOrOriginal(runtime, content) + if _, err := fmt.Fprint(runtime.IO().Out, formatted); err != nil { return errs.NewInternalError(errs.SubtypeFileIO, "write XML content to stdout: %v", err).WithCause(err) } return nil } + formatted, prettyPrinted := prettyPrintXMLOrOriginal(runtime, content) result, err := runtime.FileIO().Save(outputPath, fileio.SaveOptions{ ContentType: "application/xml", - ContentLength: int64(len(content)), - }, bytes.NewReader([]byte(content))) + ContentLength: int64(len(formatted)), + }, bytes.NewReader([]byte(formatted))) if err != nil { return common.WrapSaveErrorTyped(err) } @@ -280,6 +290,7 @@ func outputSlidesXMLGetContent(runtime *common.RuntimeContext, content string, o "path": resolvedPath, "size": result.Size(), "content_saved": true, + "pretty_printed": prettyPrinted, } for _, key := range []string{"revision_id", "remove_attr_id", "slide_id", "slide_number"} { if value, ok := out[key]; ok { @@ -289,3 +300,17 @@ func outputSlidesXMLGetContent(runtime *common.RuntimeContext, content string, o runtime.Out(fileOut, nil) return nil } + +// prettyPrintXMLOrOriginal keeps xml-get best-effort: if the server returns +// content that is not strictly valid XML, callers still receive the original +// content and a warning on stderr instead of losing the read path. The bool +// reports whether pretty-printing succeeded, surfaced as pretty_printed in +// --output file metadata. +func prettyPrintXMLOrOriginal(runtime *common.RuntimeContext, xmlContent string) (string, bool) { + out, err := prettyPrintXML(xmlContent) + if err != nil { + fmt.Fprintf(runtime.IO().ErrOut, "warning: XML pretty-print skipped; returning original server content: %v\n", err) + return xmlContent, false + } + return out, true +} diff --git a/shortcuts/slides/slides_xml_get_test.go b/shortcuts/slides/slides_xml_get_test.go index 145434db59..40fa9bcc5d 100644 --- a/shortcuts/slides/slides_xml_get_test.go +++ b/shortcuts/slides/slides_xml_get_test.go @@ -23,6 +23,10 @@ func TestSlidesXMLGetWritesContentToFileAndSuppressesXML(t *testing.T) { withSlidesTestWorkingDir(t, dir) xml := `hello` + // Golden value computed independently of prettyPrintXML (not derived by + // calling it): a bug in prettyPrintXML itself must not be able to make + // this assertion pass by construction. + wantXML := "\n \n hello\n \n\n" var capturedQuery url.Values f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) reg.Register(&httpmock.Stub{ @@ -60,10 +64,10 @@ func TestSlidesXMLGetWritesContentToFileAndSuppressesXML(t *testing.T) { if err != nil { t.Fatalf("read saved XML: %v", err) } - if string(got) != xml { - t.Fatalf("saved XML = %q, want %q", got, xml) + if string(got) != wantXML { + t.Fatalf("saved XML = %q, want %q", got, wantXML) } - if strings.Contains(stdout.String(), xml) { + if strings.Contains(stdout.String(), wantXML) { t.Fatalf("stdout leaked full XML content: %s", stdout.String()) } if got := capturedQuery.Get("revision_id"); got != "7" { @@ -80,8 +84,11 @@ func TestSlidesXMLGetWritesContentToFileAndSuppressesXML(t *testing.T) { if data["revision_id"] != float64(7) { t.Fatalf("revision_id = %v, want 7", data["revision_id"]) } - if data["size"] != float64(len(xml)) { - t.Fatalf("size = %v, want %d", data["size"], len(xml)) + if data["pretty_printed"] != true { + t.Fatalf("pretty_printed = %v, want true", data["pretty_printed"]) + } + if data["size"] != float64(len(wantXML)) { + t.Fatalf("size = %v, want %d", data["size"], len(wantXML)) } gotPath, _ := data["path"].(string) if !filepath.IsAbs(gotPath) { @@ -96,7 +103,12 @@ func TestSlidesXMLGetReturnsContentEnvelopeWhenOutputOmitted(t *testing.T) { dir := t.TempDir() withSlidesTestWorkingDir(t, dir) - xml := `hello` + // The JSON envelope carries the server content verbatim: no reindentation + // and no parse/reserialize cycle. Reintroducing the in-repo formatter + // would fail this by inserting indentation; the reference + // additionally guards against a naive parse-and-reserialize round trip, + // which would decode it to a literal space. + xml := `

Hello World

` f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) reg.Register(&httpmock.Stub{ Method: "GET", @@ -122,11 +134,14 @@ func TestSlidesXMLGetReturnsContentEnvelopeWhenOutputOmitted(t *testing.T) { data := decodeShortcutData(t, stdout) presentation := data["xml_presentation"].(map[string]interface{}) if got := presentation["content"]; got != xml { - t.Fatalf("content = %q, want %q", got, xml) + t.Fatalf("content = %q, want the server content verbatim %q", got, xml) } if got := data["xml_presentation_id"]; got != "pres_abc" { t.Fatalf("xml_presentation_id = %v, want pres_abc", got) } + if _, ok := data["pretty_printed"]; ok { + t.Fatalf("pretty_printed should not appear in the envelope: %#v", data) + } if strings.Contains(stdout.String(), "content_saved") { t.Fatalf("stdout should not contain file metadata: %s", stdout.String()) } @@ -136,6 +151,8 @@ func TestSlidesXMLGetJqFiltersContentEnvelopeWhenOutputOmitted(t *testing.T) { dir := t.TempDir() withSlidesTestWorkingDir(t, dir) + // --jq extracts fields from the envelope, and the envelope carries the + // server content verbatim, so the filter yields the single-line original. xml := `hello` f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) reg.Register(&httpmock.Stub{ @@ -161,15 +178,18 @@ func TestSlidesXMLGetJqFiltersContentEnvelopeWhenOutputOmitted(t *testing.T) { t.Fatalf("unexpected error: %v", err) } if got := strings.TrimSpace(stdout.String()); got != xml { - t.Fatalf("stdout = %q, want XML content %q", got, xml) + t.Fatalf("stdout = %q, want the server content verbatim %q", got, xml) } } -func TestSlidesXMLGetPrintsRawContentWhenRaw(t *testing.T) { +func TestSlidesXMLGetPrintsFormattedContentWithoutEnvelopeWhenRaw(t *testing.T) { dir := t.TempDir() withSlidesTestWorkingDir(t, dir) xml := `hello` + // Golden value computed independently of prettyPrintXML; see the comment + // in TestSlidesXMLGetWritesContentToFileAndSuppressesXML. + wantXML := "\n \n hello\n \n\n" f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) reg.Register(&httpmock.Stub{ Method: "GET", @@ -193,16 +213,32 @@ func TestSlidesXMLGetPrintsRawContentWhenRaw(t *testing.T) { if err != nil { t.Fatalf("unexpected error: %v", err) } - if got := stdout.String(); got != xml { - t.Fatalf("stdout = %q, want raw XML %q", got, xml) + if got := stdout.String(); got != wantXML { + t.Fatalf("stdout = %q, want formatted XML %q", got, wantXML) } } +func TestSlidesXMLGetRawFlagDocumentsFormattedOutput(t *testing.T) { + for _, flag := range SlidesXMLGet.Flags { + if flag.Name != "raw" { + continue + } + if !strings.Contains(flag.Desc, "formatted XML") || strings.Contains(flag.Desc, "raw XML") { + t.Fatalf("--raw description = %q, want formatted XML without a raw-payload claim", flag.Desc) + } + return + } + t.Fatal("--raw flag not found") +} + func TestSlidesXMLGetFetchesSingleSlideByIDToFile(t *testing.T) { dir := t.TempDir() withSlidesTestWorkingDir(t, dir) xml := `` + // Golden value computed independently of prettyPrintXML; see the comment + // in TestSlidesXMLGetWritesContentToFileAndSuppressesXML. + wantXML := "\n \n \n \n\n" var capturedQuery url.Values f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) reg.Register(&httpmock.Stub{ @@ -244,8 +280,8 @@ func TestSlidesXMLGetFetchesSingleSlideByIDToFile(t *testing.T) { if err != nil { t.Fatalf("read saved slide XML: %v", err) } - if string(got) != xml { - t.Fatalf("saved XML = %q, want %q", got, xml) + if string(got) != wantXML { + t.Fatalf("saved XML = %q, want %q", got, wantXML) } data := decodeShortcutData(t, stdout) if data["scope"] != "slide" { @@ -263,6 +299,8 @@ func TestSlidesXMLGetFetchesSingleSlideByNumberEnvelope(t *testing.T) { dir := t.TempDir() withSlidesTestWorkingDir(t, dir) + // The slide envelope carries the server content verbatim, like the + // presentation envelope. xml := `` var capturedQuery url.Values f, stdout, _, reg := cmdutil.TestFactory(t, slidesTestConfig(t, "")) @@ -305,11 +343,14 @@ func TestSlidesXMLGetFetchesSingleSlideByNumberEnvelope(t *testing.T) { } slide := data["slide"].(map[string]interface{}) if slide["content"] != xml { - t.Fatalf("content = %q, want %q", slide["content"], xml) + t.Fatalf("content = %q, want the server content verbatim %q", slide["content"], xml) } if slide["slide_id"] != "slide_2" { t.Fatalf("slide.slide_id = %v, want slide_2", slide["slide_id"]) } + if _, ok := data["pretty_printed"]; ok { + t.Fatalf("pretty_printed should not appear in the envelope: %#v", data) + } } func TestSlidesXMLGetResolvesWikiPresentation(t *testing.T) { @@ -515,3 +556,341 @@ func TestSlidesXMLGetRejectsRemoveAttrIDForSingleSlide(t *testing.T) { t.Fatalf("param = %q, want --remove-attr-id", validationErr.Param) } } + +func TestPrettyPrintXML(t *testing.T) { + input := `` + + got, err := prettyPrintXML(input) + if err != nil { + t.Fatalf("prettyPrintXML: %v", err) + } + if !strings.Contains(got, "\n") { + t.Fatalf("expected reindented output with newlines, got %q", got) + } + if n := strings.Count(got, `xmlns="http://www.larkoffice.com/sml/2.0"`); n != 1 { + t.Fatalf("expected the xmlns declaration to appear exactly once, got %d occurrences in %q", n, got) + } + if !strings.Contains(got, "") { + t.Fatalf("expected empty to stay self-closing, got %q", got) + } + if !strings.Contains(got, ``) { + t.Fatalf("expected attributes to be preserved on their element, got %q", got) + } +} + +func TestPrettyPrintXMLRejectsMalformedInput(t *testing.T) { + if _, err := prettyPrintXML(``); err == nil { + t.Fatal("expected an error for malformed XML, got nil") + } +} + +// TestPrettyPrintXMLPreservesEscapedWhitespaceReferences covers the schema's +// documented space/tab escape idiom (slides_xml_schema_definition.xml,

+// element docs) and CR/LF references whose lexical form is needed to avoid +// XML line-ending normalization on a later parse. An XML parser decodes the +// references into literal whitespace. The formatter must preserve their +// lexical representation for safe read-modify-write workflows. +func TestPrettyPrintXMLPreservesEscapedWhitespaceReferences(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + {"space in p", `

`, "\n

\n\n"}, + {"tab in p", `

`, "\n

\n\n"}, + {"space in nested span", `

`, "\n

\n\n"}, + {"hex space", `

`, "\n

\n\n"}, + {"zero-padded tab", `

`, "\n

\n\n"}, + {"carriage return", `

A B

`, "\n

A B

\n\n"}, + {"line feed", `

A B

`, "\n

A B

\n\n"}, + {"hex carriage return", `

A B

`, "\n

A B

\n\n"}, + {"hex line feed", `

A B

`, "\n

A B

\n\n"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := prettyPrintXML(tt.input) + if err != nil { + t.Fatalf("prettyPrintXML(%q): %v", tt.input, err) + } + if got != tt.want { + t.Fatalf("prettyPrintXML(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + +func TestPrettyPrintXMLPreservesTextOnlyLeafWhitespace(t *testing.T) { + tests := []struct { + name string + input string + want string + }{ + { + name: "title literal space", + input: ` `, + want: "\n \n \n\n", + }, + { + name: "title escaped space", + input: ` `, + want: "\n \n \n\n", + }, + { + name: "title whitespace CDATA", + input: `<![CDATA[ ]]>`, + want: "\n <![CDATA[ ]]>\n \n\n", + }, + { + name: "chart field literal space", + input: ` `, + want: "\n \n\n", + }, + { + name: "title adjacent text and CDATA", + input: ` <![CDATA[ ]]>`, + want: "\n <![CDATA[ ]]>\n \n\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := prettyPrintXML(tt.input) + if err != nil { + t.Fatalf("prettyPrintXML(%q): %v", tt.input, err) + } + if got != tt.want { + t.Fatalf("prettyPrintXML(%q) = %q, want %q", tt.input, got, tt.want) + } + }) + } +} + +// TestPrettyPrintXMLPreservesEscapedSpaceBetweenInlineSiblings is the +// critical case: sitting as a bare sibling text node directly between +// two inline elements, not wrapped in its own tag -- the literal reading of +// the schema's "标签之间...请使用 " guidance, e.g. a plain-styled space +// between two differently formatted words at a pptx run boundary. A fix +// that only special-cases "element whose sole content is whitespace" does +// not cover this: the whitespace here is one of several children of

, +// not the sole child of . +func TestPrettyPrintXMLPreservesEscapedSpaceBetweenInlineSiblings(t *testing.T) { + input := `

Hello World

` + want := "\n

Hello World

\n\n" + got, err := prettyPrintXML(input) + if err != nil { + t.Fatalf("prettyPrintXML: %v", err) + } + if got != want { + t.Fatalf("prettyPrintXML(%q) = %q, want %q", input, got, want) + } +} + +func TestPrettyPrintXMLPreservesCDATA(t *testing.T) { + input := `

b & ]]>

` + want := "\n

b & ]]>

\n\n" + got, err := prettyPrintXML(input) + if err != nil { + t.Fatalf("prettyPrintXML: %v", err) + } + if got != want { + t.Fatalf("prettyPrintXML(%q) = %q, want %q", input, got, want) + } +} + +// TestPrettyPrintXMLSeparatesParagraphsWithoutTouchingTheirText is the +// feature's actual point: a shape with many paragraphs becomes navigable +// (each

on its own indented line), while every paragraph's own rich +// text -- including an inline formatting boundary -- stays byte-for-byte +// unchanged. +func TestPrettyPrintXMLSeparatesParagraphsWithoutTouchingTheirText(t *testing.T) { + input := `

First paragraph.

Second paragraph.

` + want := "\n

First paragraph.

\n

Second paragraph.

\n\n" + got, err := prettyPrintXML(input) + if err != nil { + t.Fatalf("prettyPrintXML: %v", err) + } + if got != want { + t.Fatalf("prettyPrintXML(%q) = %q, want %q", input, got, want) + } +} + +func TestPrettyPrintXMLIdempotent(t *testing.T) { + input := `

A B C D E