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 <[email protected]>

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{}))
 }
 

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: "++",
 						},
 					},
 				},

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

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 := `<div class="paragraph">
-<p>a   here.</p>
-</div>`
-			verify(GinkgoT(), expectedResult, actualContent)
-		})
-
-		It("blank", func() {
-			actualContent := `a {blank} here.`
-			expectedResult := `<div class="paragraph">
-<p>a  here.</p>
-</div>`
-			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(`<div class="paragraph">
+<p>the %s symbol</p>
+</div>`, rendered)
+				verify(GinkgoT(), expectedResult, actualContent)
+			},
+			Entry("sp symbol", "sp", " "),
+			Entry("blank symbol", "blank", ""),
+			Entry("empty symbol", "empty", ""),
+			Entry("nbsp symbol", "nbsp", "&#160;"),
+			Entry("zwsp symbol", "zwsp", "&#8203;"),
+			Entry("wj symbol", "wj", "&#8288;"),
+			Entry("apos symbol", "apos", "&#39;"),
+			Entry("quot symbol", "quot", "&#34;"),
+			Entry("lsquo symbol", "lsquo", "&#8216;"),
+			Entry("rsquo symbol", "rsquo", "&#8217;"),
+			Entry("ldquo symbol", "ldquo", "&#8220;"),
+			Entry("rdquo symbol", "rdquo", "&#8221;"),
+			Entry("deg symbol", "deg", "&#176;"),
+			Entry("plus symbol", "plus", "&#43;"),
+			Entry("brvbar symbol", "brvbar", "&#166;"),
+			Entry("vbar symbol", "vbar", "|"),
+			Entry("amp symbol", "amp", "&amp;"),
+			Entry("lt symbol", "lt", "&lt;"),
+			Entry("gt symbol", "gt", "&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)
 		})
 	})
+
 })

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)

pkg/renderer/html5/passthrough_test.go

@@ -41,14 +41,16 @@ var _ = Describe("passthroughs", func() {
 
 		It("an empty standalone singleplus passthrough", func() {
 			actualContent := `++`
-			expectedResult := ``
+			expectedResult := `<div class="paragraph">
+<p>&#43;&#43;</p>
+</div>`
 			verify(GinkgoT(), expectedResult, actualContent)
 		})
 
 		It("an empty singleplus passthrough in a paragraph", func() {
 			actualContent := `++ with more content afterwards...`
 			expectedResult := `<div class="paragraph">
-<p> with more content afterwards&#8230;&#8203;</p>
+<p>&#43;&#43; with more content afterwards&#8230;&#8203;</p>
 </div>`
 			verify(GinkgoT(), expectedResult, actualContent)
 		})

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: