Skip to content

Commit

Permalink
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(renderer): avoid double encoding of document attributes
Browse files Browse the repository at this point in the history
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 `&bytesparadise#43;&bytesparadise#43;`
- accept that `InlineElementsWithoutSubtitution` can be empty
(which happens if the only content was `{blank}` which is trimmed
after rendering)

fixes bytesparadise#295

Signed-off-by: Xavier Coulon <xcoulon@redhat.com>
xcoulon committed Feb 19, 2019
1 parent 6d9eb0e commit 7a002c5
Showing 8 changed files with 108 additions and 49 deletions.
4 changes: 2 additions & 2 deletions pkg/parser/asciidoc-grammar.peg
Original file line number Diff line number Diff line change
@@ -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{}))
}

4 changes: 2 additions & 2 deletions pkg/parser/asciidoc_parser.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions pkg/parser/passthrough_test.go
Original file line number Diff line number Diff line change
@@ -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: "++",
},
},
},
1 change: 0 additions & 1 deletion pkg/renderer/html5/document_attribute_substitution.go
Original file line number Diff line number Diff line change
@@ -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
61 changes: 44 additions & 17 deletions pkg/renderer/html5/document_attribute_substitution_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
})

})
74 changes: 52 additions & 22 deletions pkg/renderer/html5/inline_elements.go
Original file line number Diff line number Diff line change
@@ -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)
6 changes: 4 additions & 2 deletions pkg/renderer/html5/passthrough_test.go
Original file line number Diff line number Diff line change
@@ -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)
})
2 changes: 2 additions & 0 deletions pkg/renderer/html5/renderer.go
Original file line number Diff line number Diff line change
@@ -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:

0 comments on commit 7a002c5

Please sign in to comment.