From c9d11e1556f87a45bcf842b551d43f349fb92c74 Mon Sep 17 00:00:00 2001 From: Niklas Fasching Date: Mon, 21 Oct 2019 00:56:52 +0200 Subject: [PATCH 1/2] Refactor OrgWriter and HTMLWriter: Remove cloning Extension of the org & html writers is made possible by creating circular references between the extending and extended writer - that way the extending writer can forward all methods it doesn't implement to the extended writer and the extended writer can use the extending writer as the root for method calls to make sure methods overridden in the extending writer are used even for nested method calls. This circular reference leads to problems when cloning writers - cloning the extended writer merely copies the pointer to the extending writer - i.e. the extending writer does not get cloned with an updated reference to the extended writer. Thus method calls to the extending writer act as if no cloning took place and things break. The easiest solution is to just get rid of cloning. We could also clone the ExtendingWriter and replace it's reference to the extended writer with the just cloned one but that's harder so we just remove it. As there are a lot of "extending writer" and "extended writer" in the above paragraphs and I'm too lazy to write up something better here's another attempt at a TLDR: Cloning is broken as ExtendingWriter is a reference to a writer that has a reference to the writer we are cloning - that writer would have to have it's reference updated but that's hard. So we solve it it by not cloning at all. --- org/html_writer.go | 25 ++++++++++---------- org/org_writer.go | 59 ++++++++++++++++++++-------------------------- 2 files changed, 38 insertions(+), 46 deletions(-) diff --git a/org/html_writer.go b/org/html_writer.go index 90a48c6..0db3be9 100644 --- a/org/html_writer.go +++ b/org/html_writer.go @@ -69,16 +69,13 @@ func NewHTMLWriter() *HTMLWriter { } } -func (w *HTMLWriter) emptyClone() *HTMLWriter { - wcopy := *w - wcopy.Builder = strings.Builder{} - return &wcopy -} - func (w *HTMLWriter) nodesAsString(nodes ...Node) string { - tmp := w.emptyClone() - WriteNodes(tmp, nodes...) - return tmp.String() + original := w.Builder + w.Builder = strings.Builder{} + WriteNodes(w, nodes...) + out := w.String() + w.Builder = original + return out } func (w *HTMLWriter) WriterWithExtensions() Writer { @@ -104,10 +101,12 @@ func (w *HTMLWriter) WritePropertyDrawer(PropertyDrawer) {} func (w *HTMLWriter) WriteBlock(b Block) { content := "" if isRawTextBlock(b.Name) { - exportWriter := w.emptyClone() - exportWriter.htmlEscape = false - WriteNodes(exportWriter, b.Children...) - content = strings.TrimRightFunc(exportWriter.String(), unicode.IsSpace) + builder, htmlEscape := w.Builder, w.htmlEscape + w.Builder, w.htmlEscape = strings.Builder{}, false + WriteNodes(w, b.Children...) + out := w.String() + w.Builder, w.htmlEscape = builder, htmlEscape + content = strings.TrimRightFunc(out, unicode.IsSpace) } else { content = w.nodesAsString(b.Children...) } diff --git a/org/org_writer.go b/org/org_writer.go index d574cda..5005c51 100644 --- a/org/org_writer.go +++ b/org/org_writer.go @@ -43,39 +43,33 @@ func (w *OrgWriter) WriterWithExtensions() Writer { func (w *OrgWriter) Before(d *Document) {} func (w *OrgWriter) After(d *Document) {} -func (w *OrgWriter) emptyClone() *OrgWriter { - wcopy := *w - wcopy.Builder = strings.Builder{} - return &wcopy -} - func (w *OrgWriter) nodesAsString(nodes ...Node) string { - tmp := w.emptyClone() - WriteNodes(tmp, nodes...) - return tmp.String() + builder := w.Builder + w.Builder = strings.Builder{} + WriteNodes(w, nodes...) + out := w.String() + w.Builder = builder + return out } func (w *OrgWriter) WriteHeadline(h Headline) { - tmp := w.emptyClone() - tmp.WriteString(strings.Repeat("*", h.Lvl)) + start := w.Len() + w.WriteString(strings.Repeat("*", h.Lvl)) if h.Status != "" { - tmp.WriteString(" " + h.Status) + w.WriteString(" " + h.Status) } if h.Priority != "" { - tmp.WriteString(" [#" + h.Priority + "]") + w.WriteString(" [#" + h.Priority + "]") } - tmp.WriteString(" ") - WriteNodes(tmp, h.Title...) - hString := tmp.String() + w.WriteString(" ") + WriteNodes(w, h.Title...) if len(h.Tags) != 0 { tString := ":" + strings.Join(h.Tags, ":") + ":" - if n := w.TagsColumn - len(tString) - len(hString); n > 0 { - w.WriteString(hString + strings.Repeat(" ", n) + tString) + if n := w.TagsColumn - len(tString) - (w.Len() - start); n > 0 { + w.WriteString(strings.Repeat(" ", n) + tString) } else { - w.WriteString(hString + " " + tString) + w.WriteString(" " + tString) } - } else { - w.WriteString(hString) } w.WriteString("\n") if len(h.Children) != 0 { @@ -185,10 +179,11 @@ func (w *OrgWriter) WriteComment(c Comment) { func (w *OrgWriter) WriteList(l List) { WriteNodes(w, l.Items...) } func (w *OrgWriter) WriteListItem(li ListItem) { - liWriter := w.emptyClone() - liWriter.indent = w.indent + strings.Repeat(" ", len(li.Bullet)+1) - WriteNodes(liWriter, li.Children...) - content := strings.TrimPrefix(liWriter.String(), liWriter.indent) + originalBuilder, originalIndent := w.Builder, w.indent + w.Builder, w.indent = strings.Builder{}, w.indent+strings.Repeat(" ", len(li.Bullet)+1) + WriteNodes(w, li.Children...) + content := strings.TrimPrefix(w.String(), w.indent) + w.Builder, w.indent = originalBuilder, originalIndent w.WriteString(w.indent + li.Bullet) if li.Status != "" { w.WriteString(fmt.Sprintf(" [%s]", li.Status)) @@ -211,10 +206,11 @@ func (w *OrgWriter) WriteDescriptiveListItem(di DescriptiveListItem) { w.WriteString(" " + term + " ::") indent = indent + strings.Repeat(" ", len(term)+4) } - diWriter := w.emptyClone() - diWriter.indent = indent - WriteNodes(diWriter, di.Details...) - details := strings.TrimPrefix(diWriter.String(), diWriter.indent) + originalBuilder, originalIndent := w.Builder, w.indent + w.Builder, w.indent = strings.Builder{}, indent + WriteNodes(w, di.Details...) + details := strings.TrimPrefix(w.String(), w.indent) + w.Builder, w.indent = originalBuilder, originalIndent if len(details) > 0 && details[0] == '\n' { w.WriteString(details) } else { @@ -326,9 +322,6 @@ func (w *OrgWriter) WriteRegularLink(l RegularLink) { } else if l.Description == nil { w.WriteString(fmt.Sprintf("[[%s]]", l.URL)) } else { - descriptionWriter := w.emptyClone() - WriteNodes(descriptionWriter, l.Description...) - description := descriptionWriter.String() - w.WriteString(fmt.Sprintf("[[%s][%s]]", l.URL, description)) + w.WriteString(fmt.Sprintf("[[%s][%s]]", l.URL, w.nodesAsString(l.Description...))) } } From 9a0a9c11eb64f8d8eb574bef931fd660514f22d1 Mon Sep 17 00:00:00 2001 From: Niklas Fasching Date: Wed, 30 Oct 2019 22:05:06 +0100 Subject: [PATCH 2/2] Export WriteNodesAsString on writer interface WriteNodesAsString is simple enough to implement but exposing it is helpful in the implementation of extending writers and we don't aim to keep writer a small interface so let's expose it. --- org/block.go | 4 ++-- org/document.go | 2 +- org/drawer.go | 4 ++-- org/footnote.go | 2 +- org/headline.go | 2 +- org/html_writer.go | 14 +++++++------- org/inline.go | 18 +++++++++--------- org/keyword.go | 10 +++++----- org/list.go | 6 +++--- org/org_writer.go | 14 +++++++------- org/paragraph.go | 4 ++-- org/table.go | 2 +- org/writer.go | 1 + 13 files changed, 42 insertions(+), 41 deletions(-) diff --git a/org/block.go b/org/block.go index 0e7a526..78ad9a7 100644 --- a/org/block.go +++ b/org/block.go @@ -80,5 +80,5 @@ func trimIndentUpTo(max int) func(string) string { } } -func (n Example) String() string { return orgWriter.nodesAsString(n) } -func (n Block) String() string { return orgWriter.nodesAsString(n) } +func (n Example) String() string { return orgWriter.WriteNodesAsString(n) } +func (n Block) String() string { return orgWriter.WriteNodesAsString(n) } diff --git a/org/document.go b/org/document.go index e43eb62..a9697c4 100644 --- a/org/document.go +++ b/org/document.go @@ -90,7 +90,7 @@ func New() *Configuration { } // String returns the pretty printed Org mode string for the given nodes (see OrgWriter). -func String(nodes []Node) string { return orgWriter.nodesAsString(nodes...) } +func String(nodes []Node) string { return orgWriter.WriteNodesAsString(nodes...) } // Write is called after with an instance of the Writer interface to export a parsed Document into another format. func (d *Document) Write(w Writer) (out string, err error) { diff --git a/org/drawer.go b/org/drawer.go index 8bb9974..eee590d 100644 --- a/org/drawer.go +++ b/org/drawer.go @@ -93,5 +93,5 @@ func (d *PropertyDrawer) Get(key string) (string, bool) { return "", false } -func (n Drawer) String() string { return orgWriter.nodesAsString(n) } -func (n PropertyDrawer) String() string { return orgWriter.nodesAsString(n) } +func (n Drawer) String() string { return orgWriter.WriteNodesAsString(n) } +func (n PropertyDrawer) String() string { return orgWriter.WriteNodesAsString(n) } diff --git a/org/footnote.go b/org/footnote.go index 660e244..e860421 100644 --- a/org/footnote.go +++ b/org/footnote.go @@ -32,4 +32,4 @@ func (d *Document) parseFootnoteDefinition(i int, parentStop stopFn) (int, Node) return consumed, definition } -func (n FootnoteDefinition) String() string { return orgWriter.nodesAsString(n) } +func (n FootnoteDefinition) String() string { return orgWriter.WriteNodesAsString(n) } diff --git a/org/headline.go b/org/headline.go index 23b986f..749d1ff 100644 --- a/org/headline.go +++ b/org/headline.go @@ -98,4 +98,4 @@ func (parent *Section) add(current *Section) { } } -func (n Headline) String() string { return orgWriter.nodesAsString(n) } +func (n Headline) String() string { return orgWriter.WriteNodesAsString(n) } diff --git a/org/html_writer.go b/org/html_writer.go index 0db3be9..916618c 100644 --- a/org/html_writer.go +++ b/org/html_writer.go @@ -69,7 +69,7 @@ func NewHTMLWriter() *HTMLWriter { } } -func (w *HTMLWriter) nodesAsString(nodes ...Node) string { +func (w *HTMLWriter) WriteNodesAsString(nodes ...Node) string { original := w.Builder w.Builder = strings.Builder{} WriteNodes(w, nodes...) @@ -108,7 +108,7 @@ func (w *HTMLWriter) WriteBlock(b Block) { w.Builder, w.htmlEscape = builder, htmlEscape content = strings.TrimRightFunc(out, unicode.IsSpace) } else { - content = w.nodesAsString(b.Children...) + content = w.WriteNodesAsString(b.Children...) } switch name := b.Name; { case name == "SRC": @@ -193,7 +193,7 @@ func (w *HTMLWriter) writeSection(section *Section) { // NOTE: To satisfy hugo ExtractTOC() check we cannot use `
  • \n` here. Doesn't really matter, just a note. w.WriteString("
  • ") h := section.Headline - title := cleanHeadlineTitleForHTMLAnchorRegexp.ReplaceAllString(w.nodesAsString(h.Title...), "") + title := cleanHeadlineTitleForHTMLAnchorRegexp.ReplaceAllString(w.WriteNodesAsString(h.Title...), "") w.WriteString(fmt.Sprintf("%s\n", h.ID(), title)) if len(section.Children) != 0 { w.WriteString("
      \n") @@ -305,7 +305,7 @@ func (w *HTMLWriter) WriteRegularLink(l RegularLink) { } description := url if l.Description != nil { - description = w.nodesAsString(l.Description...) + description = w.WriteNodesAsString(l.Description...) } switch l.Kind() { case "image": @@ -383,10 +383,10 @@ func (w *HTMLWriter) WriteHorizontalRule(h HorizontalRule) { } func (w *HTMLWriter) WriteNodeWithMeta(n NodeWithMeta) { - out := w.nodesAsString(n.Node) + out := w.WriteNodesAsString(n.Node) if p, ok := n.Node.(Paragraph); ok { if len(p.Children) == 1 && isImageOrVideoLink(p.Children[0]) { - out = w.nodesAsString(p.Children[0]) + out = w.WriteNodesAsString(p.Children[0]) } } for _, attributes := range n.Meta.HTMLAttributes { @@ -398,7 +398,7 @@ func (w *HTMLWriter) WriteNodeWithMeta(n NodeWithMeta) { if i != 0 { caption += " " } - caption += w.nodesAsString(ns...) + caption += w.WriteNodesAsString(ns...) } out = fmt.Sprintf("
      \n%s
      \n%s\n
      \n
      \n", out, caption) } diff --git a/org/inline.go b/org/inline.go index 09c58f1..c33701d 100644 --- a/org/inline.go +++ b/org/inline.go @@ -346,12 +346,12 @@ func (l RegularLink) Kind() string { return "regular" } -func (n Text) String() string { return orgWriter.nodesAsString(n) } -func (n LineBreak) String() string { return orgWriter.nodesAsString(n) } -func (n ExplicitLineBreak) String() string { return orgWriter.nodesAsString(n) } -func (n StatisticToken) String() string { return orgWriter.nodesAsString(n) } -func (n Emphasis) String() string { return orgWriter.nodesAsString(n) } -func (n LatexFragment) String() string { return orgWriter.nodesAsString(n) } -func (n FootnoteLink) String() string { return orgWriter.nodesAsString(n) } -func (n RegularLink) String() string { return orgWriter.nodesAsString(n) } -func (n Timestamp) String() string { return orgWriter.nodesAsString(n) } +func (n Text) String() string { return orgWriter.WriteNodesAsString(n) } +func (n LineBreak) String() string { return orgWriter.WriteNodesAsString(n) } +func (n ExplicitLineBreak) String() string { return orgWriter.WriteNodesAsString(n) } +func (n StatisticToken) String() string { return orgWriter.WriteNodesAsString(n) } +func (n Emphasis) String() string { return orgWriter.WriteNodesAsString(n) } +func (n LatexFragment) String() string { return orgWriter.WriteNodesAsString(n) } +func (n FootnoteLink) String() string { return orgWriter.WriteNodesAsString(n) } +func (n RegularLink) String() string { return orgWriter.WriteNodesAsString(n) } +func (n Timestamp) String() string { return orgWriter.WriteNodesAsString(n) } diff --git a/org/keyword.go b/org/keyword.go index 7762417..3ab8804 100644 --- a/org/keyword.go +++ b/org/keyword.go @@ -177,8 +177,8 @@ func (d *Document) loadSetupFile(k Keyword) (int, Node) { return 1, k } -func (n Comment) String() string { return orgWriter.nodesAsString(n) } -func (n Keyword) String() string { return orgWriter.nodesAsString(n) } -func (n NodeWithMeta) String() string { return orgWriter.nodesAsString(n) } -func (n NodeWithName) String() string { return orgWriter.nodesAsString(n) } -func (n Include) String() string { return orgWriter.nodesAsString(n) } +func (n Comment) String() string { return orgWriter.WriteNodesAsString(n) } +func (n Keyword) String() string { return orgWriter.WriteNodesAsString(n) } +func (n NodeWithMeta) String() string { return orgWriter.WriteNodesAsString(n) } +func (n NodeWithName) String() string { return orgWriter.WriteNodesAsString(n) } +func (n Include) String() string { return orgWriter.WriteNodesAsString(n) } diff --git a/org/list.go b/org/list.go index 6ba28f6..462e9fa 100644 --- a/org/list.go +++ b/org/list.go @@ -109,6 +109,6 @@ func (d *Document) parseListItem(l List, i int, parentStop stopFn) (int, Node) { return i - start, ListItem{bullet, status, nodes} } -func (n List) String() string { return orgWriter.nodesAsString(n) } -func (n ListItem) String() string { return orgWriter.nodesAsString(n) } -func (n DescriptiveListItem) String() string { return orgWriter.nodesAsString(n) } +func (n List) String() string { return orgWriter.WriteNodesAsString(n) } +func (n ListItem) String() string { return orgWriter.WriteNodesAsString(n) } +func (n DescriptiveListItem) String() string { return orgWriter.WriteNodesAsString(n) } diff --git a/org/org_writer.go b/org/org_writer.go index 5005c51..8855df9 100644 --- a/org/org_writer.go +++ b/org/org_writer.go @@ -43,7 +43,7 @@ func (w *OrgWriter) WriterWithExtensions() Writer { func (w *OrgWriter) Before(d *Document) {} func (w *OrgWriter) After(d *Document) {} -func (w *OrgWriter) nodesAsString(nodes ...Node) string { +func (w *OrgWriter) WriteNodesAsString(nodes ...Node) string { builder := w.Builder w.Builder = strings.Builder{} WriteNodes(w, nodes...) @@ -117,7 +117,7 @@ func (w *OrgWriter) WritePropertyDrawer(d PropertyDrawer) { func (w *OrgWriter) WriteFootnoteDefinition(f FootnoteDefinition) { w.WriteString(fmt.Sprintf("[fn:%s]", f.Name)) - content := w.nodesAsString(f.Children...) + content := w.WriteNodesAsString(f.Children...) if content != "" && !unicode.IsSpace(rune(content[0])) { w.WriteString(" ") } @@ -125,7 +125,7 @@ func (w *OrgWriter) WriteFootnoteDefinition(f FootnoteDefinition) { } func (w *OrgWriter) WriteParagraph(p Paragraph) { - content := w.nodesAsString(p.Children...) + content := w.WriteNodesAsString(p.Children...) if len(content) > 0 && content[0] != '\n' { w.WriteString(w.indent) } @@ -135,7 +135,7 @@ func (w *OrgWriter) WriteParagraph(p Paragraph) { func (w *OrgWriter) WriteExample(e Example) { for _, n := range e.Children { w.WriteString(w.indent + ":") - if content := w.nodesAsString(n); content != "" { + if content := w.WriteNodesAsString(n); content != "" { w.WriteString(" " + content) } w.WriteString("\n") @@ -202,7 +202,7 @@ func (w *OrgWriter) WriteDescriptiveListItem(di DescriptiveListItem) { } indent := w.indent + strings.Repeat(" ", len(di.Bullet)+1) if len(di.Term) != 0 { - term := w.nodesAsString(di.Term...) + term := w.WriteNodesAsString(di.Term...) w.WriteString(" " + term + " ::") indent = indent + strings.Repeat(" ", len(term)+4) } @@ -235,7 +235,7 @@ func (w *OrgWriter) WriteTable(t Table) { w.WriteString(`|`) for _, column := range row.Columns { w.WriteString(` `) - content := w.nodesAsString(column.Children...) + content := w.WriteNodesAsString(column.Children...) if content == "" { content = " " } @@ -322,6 +322,6 @@ func (w *OrgWriter) WriteRegularLink(l RegularLink) { } else if l.Description == nil { w.WriteString(fmt.Sprintf("[[%s]]", l.URL)) } else { - w.WriteString(fmt.Sprintf("[[%s][%s]]", l.URL, w.nodesAsString(l.Description...))) + w.WriteString(fmt.Sprintf("[[%s][%s]]", l.URL, w.WriteNodesAsString(l.Description...))) } } diff --git a/org/paragraph.go b/org/paragraph.go index b7d3ea9..24f0554 100644 --- a/org/paragraph.go +++ b/org/paragraph.go @@ -42,5 +42,5 @@ func (d *Document) parseHorizontalRule(i int, parentStop stopFn) (int, Node) { return 1, HorizontalRule{} } -func (n Paragraph) String() string { return orgWriter.nodesAsString(n) } -func (n HorizontalRule) String() string { return orgWriter.nodesAsString(n) } +func (n Paragraph) String() string { return orgWriter.WriteNodesAsString(n) } +func (n HorizontalRule) String() string { return orgWriter.WriteNodesAsString(n) } diff --git a/org/table.go b/org/table.go index a404e1a..395588a 100644 --- a/org/table.go +++ b/org/table.go @@ -127,4 +127,4 @@ func isSpecialRow(rawColumns []string) bool { return isAlignRow } -func (n Table) String() string { return orgWriter.nodesAsString(n) } +func (n Table) String() string { return orgWriter.WriteNodesAsString(n) } diff --git a/org/writer.go b/org/writer.go index c4aebd6..189d72e 100644 --- a/org/writer.go +++ b/org/writer.go @@ -9,6 +9,7 @@ type Writer interface { String() string // String is called at the very end to retrieve the final output. WriterWithExtensions() Writer + WriteNodesAsString(...Node) string WriteKeyword(Keyword) WriteInclude(Include)