Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve ease of extending writers #15

Merged
merged 2 commits into from
Nov 2, 2019
Merged

Conversation

niklasfasching
Copy link
Owner

No description provided.

@lafriks
Copy link

lafriks commented Nov 1, 2019

Just tested and for me it seems to work correctly

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.
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.
@niklasfasching niklasfasching changed the title WIP: Export WriteNodesAsString Improve ease of extending writers Nov 2, 2019
@niklasfasching niklasfasching merged commit 466903d into master Nov 2, 2019
@niklasfasching niklasfasching deleted the nodes-as-string branch November 2, 2019 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants