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

Prefix Markdown Docs Based On Visible Text #106

Merged
merged 3 commits into from
Aug 29, 2023

Conversation

MahdiBM
Copy link
Collaborator

@MahdiBM MahdiBM commented Aug 28, 2023

Penny should prefix the text content of the markdown documents and not touch the markdown attributes of the document.

For example now, prefixing
```
Hello, how are you?
```

Would result in
```
Hello, how are ...
```

Instead of the previous
```
Hello, how are you?

Which used to remove the ``` markdown attributes.

Resolves a task of #100.

@MahdiBM MahdiBM requested review from 0xTim and gwynne as code owners August 28, 2023 21:20
@@ -71,6 +80,21 @@ extension String {
return lines.joined(separator: "\n")
}

/// Doesn't count markdown attributes towards the limit.
@_disfavoredOverload
Copy link
Member

Choose a reason for hiding this comment

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

Why is this attribute needed? (Disfavored overloads are notoriously finicky at best.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The function names are the same.
Unless you can suggest a good function name for the functions that return both the remaining and the result, then we'll need to roll with this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've basically duplicated the unicodesPrefix() function, one just returns the result, the other returns the remaining too.

@@ -6,12 +6,17 @@ extension String {
) ?? self
}

public func unicodesPrefix(_ maxUnicodeScalars: Int) -> String {
@_disfavoredOverload
Copy link
Member

Choose a reason for hiding this comment

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

Again, why?

Copy link
Member

@gwynne gwynne left a comment

Choose a reason for hiding this comment

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

Overall, looks excellent! Just the two questions

@MahdiBM MahdiBM merged commit c0af1db into main Aug 29, 2023
@MahdiBM MahdiBM deleted the mmbm-better-markdown-prefixing branch August 29, 2023 08:31
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