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

[SuperEditor] - Attributions not placed according to markdown standard when serializing document #2424

Open
edhom opened this issue Nov 27, 2024 · 3 comments
Labels
type_bug Something isn't working

Comments

@edhom
Copy link

edhom commented Nov 27, 2024

The AttributedTextMarkdownSerializer places markdown formatting symbols according to the offset of attribution markers. This results in invalid markdown, as the markdown standard doesn't allow leading/trailing whitespaces.

I'm unsure whether the correct fix is in the AttributedTextMarkdownSerializer, or instead to update the attribution markers already while editing to exclude whitespace.

To Reproduce

Screen.Recording.2024-11-27.at.09.23.20.mov

Actual behavior
Super** editor**

Expected behavior
Super editor

@edhom edhom added the type_bug Something isn't working label Nov 27, 2024
@edhom
Copy link
Author

edhom commented Dec 6, 2024

I'm unsure whether the correct fix is in the AttributedTextMarkdownSerializer, or instead to update the attribution markers already while editing to exclude whitespace.

@matthew-carroll If you could guide me on this question, I would try to come up with a PR:)

@matthew-carroll
Copy link
Contributor

Hi @edhom - can you please link to the part of the Markdown spec that says this?

WRT a PR/work - the best place to start is to write a bundle of failing tests that demonstrate all the edge cases you can think of for this rule. We already have many tests written in the super_editor_markdown package. You can check if there's a place within the existing files where these tests would fit. If for some reason these tests are sufficiently different in purpose from all existing tests, then you can create a new file.

If you put up a draft PR and tag me and @angelosilvestre with those tests then we can better understand the implications of this change, and help figure out where the change should be implemented.

@edhom
Copy link
Author

edhom commented Dec 10, 2024

@matthew-carroll Thanks a lot, I followed up here: #2452

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type_bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants