search

tommoor / slate-md-serializer

A Markdown serializer for the Slate editor framework
MIT License
64 stars 36 forks source link

Update to latest version of Slate #36

Open eduardoboucas opened 5 years ago

eduardoboucas commented 5 years ago

Hi all.

I've recently started using Slate and came across this Markdown serialiser. After reading #27, I learned that this fork is not compatible with the latest version (0.47), so I had a go at making the necessary changes, which are included in this PR.

The main change was to remove the leaves property from text objects. Slate was throwing deprecation warnings before, which this PR fixes.

I've also made a change to how list items are processed (https://github.com/tommoor/slate-md-serializer/compare/master...edithq:master#diff-6947033678b93d106e25614dd972e66fL1151), so that list items are not wrapped inside a paragraph node. I'm not sure this is a result of any changes in Slate core, but the reality is that my list items were not being rendered properly and this seems to fix it.

@tommoor I assume you're probably not interested in merging this at this point, because from what I gather you're pegged to an older version of Slate, but if you could kindly glance over this PR and let me know if there's something blatantly wrong in the implementation, that would be much appreciated.

If you're happy with it, perhaps we can leave the PR open and we can merge our forks again once you're happy to upgrade.

Thanks!

tommoor commented 5 years ago

Hey, thanks for the work – I'll definitely be upgrading Slate at some point but tend to do it in batches as there are too many breaking changes to continuously keep up.

The paragraph tags are definitely there purposefully to fix earlier bugs so this will be difficult to merge with that removal. I'd recommend you leave them in and adjust you CSS to account for them, but up to you.

eduardoboucas commented 5 years ago

I'm currently working on some changes which will hopefully fix #35. Once that's done and stable, I'll revisit the paragraph tags and, at that point, we can discuss whether this can be merged or not.

In the meantime, I'll be publishing my fork as @edithq/slate-md-serializer, in case anyone is interested (and brave!).

samuelcolvin commented 5 years ago

@edithq/slate-md-serializer seems to be working well for me, would be wonderful if this could be merged and released.

Aerlinger commented 5 years ago

@tommoor Any reason this can't be merged? FWIW, it'd be really helpful for me as This PR does fix some issues with nested bold/italic marks and some other list item issues.

tommoor commented 5 years ago

@Aerlinger unfortunately there seems to be too many unrelated changes in this PR to merge cleanly. It also changes the paragraph handling as commented previously. If the fork works for you, I'd definitely encourage you to use it until this repo is updated

Aerlinger commented 5 years ago

@tommoor I see. Out of curiosity, why is the paragraph rendering necessary? Is it for legacy reasons with how the original source was written or is it for compatibility with Slate? Wouldn't it be simpler just to render text leaves instead of wrapping them in a paragraph if it's not necessary?

DominikLevitsky commented 4 years ago

@eduardoboucas I have stumbled upon this repository, as I was looking for a Slate MD serializer, and now it turns out, this repo supports only earlier versions of Slate. I have also seen that '@edithq/slate-md-serializer' is also available on npm, but I have installed it and it does not seem to work. Should it actually work at this point?

eduardoboucas commented 4 years ago

@DominikLevitsky it should work, yes. What's failing for you?

samuelcolvin commented 4 years ago

It would be great to get this merged and released.

Is there any idea of a timeline?