From 00c01325949fe154014a4c91ce78702ffd04710b Mon Sep 17 00:00:00 2001 From: Xavier Coulon Date: Wed, 20 Feb 2019 07:57:02 +0100 Subject: [PATCH] fix(renderer): avoid double encoding of document attributes (#296) render all elements and apply substitutions, then if applicable (ie, there were some substitutions), then parse the inline elements again and render the elements, but with plain string output for the StringElements to avoid double HTML escaping also: - do not treat `++` as an empty single plus passthrough (but keep `++++++` as an empty triple plus passthrough for now) - render `++` as `++` - accept that `InlineElementsWithoutSubtitution` can be empty (which happens if the only content was `{blank}` which is trimmed after rendering) fixes #295 Signed-off-by: Xavier Coulon --- pkg/parser/asciidoc-grammar.peg | 4 +- pkg/parser/asciidoc_parser.go | 4 +- pkg/parser/passthrough_test.go | 5 +- .../html5/document_attribute_substitution.go | 1 - .../document_attribute_substitution_test.go | 61 ++++++++++----- pkg/renderer/html5/inline_elements.go | 74 +++++++++++++------ pkg/renderer/html5/passthrough_test.go | 6 +- pkg/renderer/html5/renderer.go | 2 + 8 files changed, 108 insertions(+), 49 deletions(-) diff --git a/pkg/parser/asciidoc-grammar.peg b/pkg/parser/asciidoc-grammar.peg index baf3ad16..69e3fbe2 100644 --- a/pkg/parser/asciidoc-grammar.peg +++ b/pkg/parser/asciidoc-grammar.peg @@ -678,7 +678,7 @@ InlineElement <- !EOL !LineBreak // special case for re-parsing a group of elements after a document substitution: // we should treat substitution that did not happen (eg: missing attribute) as regular // strings - (used by the inline element renderer) -InlineElementsWithoutSubtitution <- !BlankLine !BlockDelimiter elements:(InlineElementWithoutSubtitution)+ linebreak:(LineBreak)? EOL { // absorbs heading and trailing spaces +InlineElementsWithoutSubtitution <- !BlankLine !BlockDelimiter elements:(InlineElementWithoutSubtitution)* linebreak:(LineBreak)? EOL { // absorbs heading and trailing spaces return types.NewInlineElements(append(elements.([]interface{}), linebreak)) } @@ -857,7 +857,7 @@ Passthrough <- TriplePlusPassthrough / SinglePlusPassthrough / PassthroughMacro SinglePlusPassthrough <- "+" content:(Alphanums / Spaces / (!NEWLINE !"+" . ){ return string(c.text), nil -})* "+" { +})+ "+" { return types.NewPassthrough(types.SinglePlusPassthrough, content.([]interface{})) } diff --git a/pkg/parser/asciidoc_parser.go b/pkg/parser/asciidoc_parser.go index 3fd112d0..b23a9168 100644 --- a/pkg/parser/asciidoc_parser.go +++ b/pkg/parser/asciidoc_parser.go @@ -58684,7 +58684,7 @@ var g = &grammar{ &labeledExpr{ pos: position{line: 681, col: 64, offset: 24502}, label: "elements", - expr: &oneOrMoreExpr{ + expr: &zeroOrMoreExpr{ pos: position{line: 681, col: 73, offset: 24511}, expr: &ruleRefExpr{ pos: position{line: 681, col: 74, offset: 24512}, @@ -66875,7 +66875,7 @@ var g = &grammar{ &labeledExpr{ pos: position{line: 858, col: 30, offset: 34105}, label: "content", - expr: &zeroOrMoreExpr{ + expr: &oneOrMoreExpr{ pos: position{line: 858, col: 38, offset: 34113}, expr: &choiceExpr{ pos: position{line: 858, col: 39, offset: 34114}, diff --git a/pkg/parser/passthrough_test.go b/pkg/parser/passthrough_test.go index ac4a13d2..db8f1ffb 100644 --- a/pkg/parser/passthrough_test.go +++ b/pkg/parser/passthrough_test.go @@ -158,9 +158,8 @@ var _ = Describe("passthroughs", func() { Attributes: types.ElementAttributes{}, Lines: []types.InlineElements{ { - types.Passthrough{ - Kind: types.SinglePlusPassthrough, - Elements: types.InlineElements{}, + types.StringElement{ + Content: "++", }, }, }, diff --git a/pkg/renderer/html5/document_attribute_substitution.go b/pkg/renderer/html5/document_attribute_substitution.go index 43bea184..f42ed0b4 100644 --- a/pkg/renderer/html5/document_attribute_substitution.go +++ b/pkg/renderer/html5/document_attribute_substitution.go @@ -8,7 +8,6 @@ import ( "github.com/bytesparadise/libasciidoc/pkg/types" ) - func processAttributeDeclaration(ctx *renderer.Context, attr types.DocumentAttributeDeclaration) error { ctx.Document.Attributes.AddDeclaration(attr) return nil diff --git a/pkg/renderer/html5/document_attribute_substitution_test.go b/pkg/renderer/html5/document_attribute_substitution_test.go index ef4a215b..db263d53 100644 --- a/pkg/renderer/html5/document_attribute_substitution_test.go +++ b/pkg/renderer/html5/document_attribute_substitution_test.go @@ -1,7 +1,10 @@ package html5_test import ( + "fmt" + . "github.com/onsi/ginkgo" + . "github.com/onsi/ginkgo/extensions/table" ) var _ = Describe("document with attributes", func() { @@ -92,23 +95,46 @@ author is {author}.` }) }) - Context("predefined elements", func() { - - It("single space", func() { - actualContent := `a {sp} here.` - expectedResult := `
-

a here.

-
` - verify(GinkgoT(), expectedResult, actualContent) - }) - - It("blank", func() { - actualContent := `a {blank} here.` - expectedResult := `
-

a here.

-
` - verify(GinkgoT(), expectedResult, actualContent) - }) + Context("predefined attributes", func() { + + DescribeTable("predefined attributes in a paragraph", + func(code, rendered string) { + actualContent := fmt.Sprintf(`the {%s} symbol`, code) + expectedResult := fmt.Sprintf(`
+

the %s symbol

+
`, rendered) + verify(GinkgoT(), expectedResult, actualContent) + }, + Entry("sp symbol", "sp", " "), + Entry("blank symbol", "blank", ""), + Entry("empty symbol", "empty", ""), + Entry("nbsp symbol", "nbsp", " "), + Entry("zwsp symbol", "zwsp", "​"), + Entry("wj symbol", "wj", "⁠"), + Entry("apos symbol", "apos", "'"), + Entry("quot symbol", "quot", """), + Entry("lsquo symbol", "lsquo", "‘"), + Entry("rsquo symbol", "rsquo", "’"), + Entry("ldquo symbol", "ldquo", "“"), + Entry("rdquo symbol", "rdquo", "”"), + Entry("deg symbol", "deg", "°"), + Entry("plus symbol", "plus", "+"), + Entry("brvbar symbol", "brvbar", "¦"), + Entry("vbar symbol", "vbar", "|"), + Entry("amp symbol", "amp", "&"), + Entry("lt symbol", "lt", "<"), + Entry("gt symbol", "gt", ">"), + Entry("startsb symbol", "startsb", "["), + Entry("endsb symbol", "endsb", "]"), + Entry("caret symbol", "caret", "^"), + Entry("asterisk symbol", "asterisk", "*"), + Entry("tilde symbol", "tilde", "~"), + Entry("backslash symbol", "backslash", `\`), + Entry("backtick symbol", "backtick", "`"), + Entry("two-colons symbol", "two-colons", "::"), + Entry("two-semicolons symbol", "two-semicolons", ";"), + Entry("cpp symbol", "cpp", "C++"), + ) It("overriding predefined attribute", func() { actualContent := `:blank: foo @@ -120,4 +146,5 @@ a {blank} here.` verify(GinkgoT(), expectedResult, actualContent) }) }) + }) diff --git a/pkg/renderer/html5/inline_elements.go b/pkg/renderer/html5/inline_elements.go index 926faa64..a28eca8a 100644 --- a/pkg/renderer/html5/inline_elements.go +++ b/pkg/renderer/html5/inline_elements.go @@ -4,6 +4,8 @@ import ( "bytes" "strings" + "github.com/davecgh/go-spew/spew" + "github.com/bytesparadise/libasciidoc/pkg/parser" "github.com/bytesparadise/libasciidoc/pkg/renderer" @@ -12,17 +14,6 @@ import ( log "github.com/sirupsen/logrus" ) -// // renderLines renders all lines (i.e, all `InlineElements`` - each `InlineElements` being a slice of elements to generate a line) -// // and includes an `\n` character in-between, until the last one. -// // Trailing spaces are removed for each line. -// func renderLinesWithHardbreak(ctx *renderer.Context, elements []types.InlineElements, hardbreak bool) (string, error) { -// r, err := renderLines(ctx, elements) -// if err != nil { -// return "", err -// } -// return string(r), nil -// } - func renderLinesAsString(ctx *renderer.Context, elements []types.InlineElements, hardbreak bool) (string, error) { result, err := renderLines(ctx, elements, renderElement, hardbreak) return string(result), err @@ -64,11 +55,8 @@ func renderLines(ctx *renderer.Context, elements []types.InlineElements, renderE func renderLine(ctx *renderer.Context, elements types.InlineElements, renderElementFunc rendererFunc) ([]byte, error) { log.Debugf("rendering line with %d element(s)...", len(elements)) - elements, err := applySubstitutions(ctx, elements) - if err != nil { - return nil, errors.Wrapf(err, "unable to render line") - } + // first pass or rendering, using the provided `renderElementFunc`: buff := bytes.NewBuffer(nil) for i, element := range elements { renderedElement, err := renderElementFunc(ctx, element) @@ -87,7 +75,48 @@ func renderLine(ctx *renderer.Context, elements types.InlineElements, renderElem buff.Write(renderedElement) } } - log.Debugf("rendered elements: '%s'", buff.String()) + + log.Debugf("rendered line elements after 1st pass: '%s'", buff.String()) + + // check if the line has some substitution + if !hasSubstitutions(elements) { + log.Debug("no substitution in the line of elements") + return buff.Bytes(), nil + } + // otherwise, parse the rendered line, in case some new elements (links, etc.) "appeared" after document attribute substitutions + r, err := parser.Parse("", buff.Bytes(), + parser.Entrypoint("InlineElementsWithoutSubtitution")) // parse a single InlineElements + if err != nil { + return []byte{}, errors.Wrap(err, "failed process elements after substitution") + } + elements, ok := r.(types.InlineElements) + if !ok { + return []byte{}, errors.Errorf("failed process elements after substitution") + } + if log.IsLevelEnabled(log.DebugLevel) { + log.Debug("post-substitution line of elements:") + spew.Dump(elements) + } + buff = bytes.NewBuffer(nil) + // render all elements of the line, but StringElement must be rendered plain-text now, to avoid double HTML escape + for i, element := range elements { + switch element := element.(type) { + case types.StringElement: + if i == len(elements)-1 { + buff.WriteString(strings.TrimRight(element.Content, " ")) + } else { + buff.WriteString(element.Content) + } + default: + renderedElement, err := renderElement(ctx, element) + if err != nil { + return nil, errors.Wrapf(err, "unable to render line") + } + buff.Write(renderedElement) + } + } + + log.Debugf("rendered line elements: '%s'", buff.String()) return buff.Bytes(), nil } @@ -103,7 +132,7 @@ func hasSubstitutions(e types.InlineElements) bool { func applySubstitutions(ctx *renderer.Context, e types.InlineElements) (types.InlineElements, error) { if !hasSubstitutions(e) { - log.Debug("no subsitution in the line of elements") + log.Debug("no substitution in the line of elements") return e, nil } log.Debugf("applying substitutions on %v (%d)", e, len(e)) @@ -116,7 +145,8 @@ func applySubstitutions(ctx *renderer.Context, e types.InlineElements) (types.In if err != nil { return types.InlineElements{}, errors.Wrap(err, "failed to apply substitution") } - s = append(s, types.NewStringElement(string(r))) + s = append(s, types.NewStringElement(string(r))) // this string element will eventually be merged with surroundings StringElement(s) + default: s = append(s, element) } @@ -128,10 +158,10 @@ func applySubstitutions(ctx *renderer.Context, e types.InlineElements) (types.In return types.InlineElements{}, errors.Wrap(err, "failed to apply substitution") } log.Debugf("substitution(s) result: %v (%d)", s, len(s)) - // now parse the StringElements + // now parse the StringElements again to look for new blocks (eg: external link appeared) result := make([]interface{}, 0) for _, element := range s { - log.Debugf("now processing element of type %T", element) + log.Debugf("now processing element of type %[1]T: %[1]v", element) switch element := element.(type) { case types.StringElement: r, err := parser.Parse("", @@ -141,10 +171,10 @@ func applySubstitutions(ctx *renderer.Context, e types.InlineElements) (types.In return types.InlineElements{}, errors.Wrap(err, "failed process elements after substitution") } if r, ok := r.(types.InlineElements); ok { - // here the doc should have directly an InlineElements since we specified a specific entrypoint for the parser + // here the is an InlineElements since we specified a specific entrypoint for the parser result = append(result, r...) } else { - return types.InlineElements{}, errors.Errorf("expected an groupg of elements, but got a result of type %T", r) + return types.InlineElements{}, errors.Errorf("expected an group of InlineElements, but got a result of type %T", r) } default: result = append(result, element) diff --git a/pkg/renderer/html5/passthrough_test.go b/pkg/renderer/html5/passthrough_test.go index 02b8283b..7c5fb0b6 100644 --- a/pkg/renderer/html5/passthrough_test.go +++ b/pkg/renderer/html5/passthrough_test.go @@ -41,14 +41,16 @@ var _ = Describe("passthroughs", func() { It("an empty standalone singleplus passthrough", func() { actualContent := `++` - expectedResult := `` + expectedResult := `
+

++

+
` verify(GinkgoT(), expectedResult, actualContent) }) It("an empty singleplus passthrough in a paragraph", func() { actualContent := `++ with more content afterwards...` expectedResult := `
-

with more content afterwards…​

+

++ with more content afterwards…​

` verify(GinkgoT(), expectedResult, actualContent) }) diff --git a/pkg/renderer/html5/renderer.go b/pkg/renderer/html5/renderer.go index 77aa7561..3a86eac9 100644 --- a/pkg/renderer/html5/renderer.go +++ b/pkg/renderer/html5/renderer.go @@ -121,6 +121,8 @@ func renderElement(ctx *renderer.Context, element interface{}) ([]byte, error) { case types.DocumentAttributeReset: // 'process' function do not return any rendered content, but may return an error return nil, processAttributeReset(ctx, e) + case types.DocumentAttributeSubstitution: + return renderAttributeSubstitution(ctx, e) case types.LineBreak: return renderLineBreak() case types.SingleLineComment: