From c9d11e1556f87a45bcf842b551d43f349fb92c74 Mon Sep 17 00:00:00 2001 From: Niklas Fasching Date: Mon, 21 Oct 2019 00:56:52 +0200 Subject: [PATCH] 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...))) } }