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

Pass plain text to the modifier closure where available #65

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mrtom
Copy link

@mrtom mrtom commented Mar 8, 2021

Overview

It seems to me like it would be valuable to have the plain text provided as part of the Modifier's closure. Currently the closure is passed the HTML generated by Ink (which is useful if one wants to append a prefix or suffix) and the original markdown (which is useful if one wants to perform a completely custom parsing I guess?).

But you don't get the raw text content from the markdown. I can see this being useful in places where eg you want to generate really specific HTML or (as in my use case) where I want to generate a plain text version of the string to go along with the HTML version (for additional analysis like applying Hemmingway Editor style rules to the document).

Given I already have the original markdown I could always parse it again, but that feels like unnecessary work.

Testing steps

I ran swift build from the command line and confirmed the build built correctly using MacOS 11.2 (Big Sur). I also ran swift test and confirmed the tests pass.

I amended the testModifierInput test to also check that the plain text was generated successfully.

I have not tested on Linux.

Considerations

Firstly, the API has now been changed to return an optional String, as not all implementations of HTMLConvertible are implementations of PlainTextConvertible. (Although in practise I think they all are currently, right?). This is simplified from an API point of view if we instead change the extension to:

extension HTMLConvertible where Self: Modifiable & PlainTextConvertible, but then you're coupling the Modifiable and PlainTextConvertabile protocols, which seems undesirable.

This is also a breaking change as existing modifiers will need to be updated to include plainText (or _) in the closure argument list.

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.

1 participant