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

Allow Modifiers for plain text #45

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

bensyverson
Copy link

Summary

This PR allows Modifiers to transform unformatted text—for example, the contents of an h1 heading, the text of a link, or the bulk of a paragraph.

Discussion

In my Publish site, I need to examine the text of my markdown files to do some dynamic transformation. For simplicity, imagine I want to provide dynamic stock prices based on ticker symbols (this is a bad example, but bear with me).

So given the markdown Apple trades under the ticker $AAPL, I would like to generate:

<p>Apple trades under the ticker AAPL ($324.34 ▲ 6.65 | 2.09%)</p>

Currently there doesn't seem to be a clean way to do this in Publish.

This change

This change adds a Modifier.Target case (.text) which allows Modifiers to target plain text, so you could write:

parser.addModifier(Modifier(target: .text) {
    $0.html.replacingOccurrences(of: "$AAPL", with: "AAPL (" + currentTicker + ")")
})

Implications

This is accomplished by extending Substring to conform to HTMLConvertible and Modifiable. It works fine, but I admit it's a little strange at the calling site to request .html for a plain string.

Although Ink is smart enough to avoid code blocks and HTML within markdown, the user could still run into problems if they try to add HTML elements in their Modifier. For example, if the user tries to add links to plain text, they should be aware that their incoming text might itself be within another link, so you could accidentally end up with:

<a href="https://apple.com/">View the <a href="https://apple.com/">AAPL</a> site</a>

However, this is a danger with any Modifier, so I don't think it's unique to this PR.

Thanks for considering this! 🙏

Copy link
Contributor

@john-mueller john-mueller left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've spent a while mulling this over, and I think that it makes sense to add this functionality. I don't see any problem with the implementation, but @JohnSundell should probably look it over. For what it's worth, I ran benchmarking tests, and it does not affect parsing speed or spec compatibility.

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