Rosey / markdown-draft-js

A tool to convert content created in DraftJS to markdown and vice versa.
https://rosey.github.io/markdown-draft-js/
MIT License
318 stars 69 forks source link

Add support for passing preset with additional options to Remarkable #88

Closed kamilbielawski closed 5 years ago

kamilbielawski commented 5 years ago

When using Remarkable directly, we we can pass two arguments to its constructor - the first is the template and the second is an object with additional options.

markdown-draft-js only lets us pass a single value to the remarkableOptions, so we need to choose whether to use a preset or an options object.

This PR makes it possible to pass both (in an array).

Rosey commented 5 years ago

Ah interesting, thank you @kamilbielawski ! I may make a few tweaks before merging this, in the interest of maintaining backwards compatibility. 🤔 Not entirely decided though on the best API for adding this. Thinking maybe instead of an array passing in something like this:

{
  remarkablePreset: 'preset name here if you want to include one',
  remarkableOptions: { }
}
kamilbielawski commented 5 years ago

Hey @Rosey, thanks for a quick response!

I have to admit I wasn't really sure what would be the best API for this.

I guess there are two main criteria to use when comparing them:

So yeah, I agree that the API you're suggesting is probably better overall. I'm happy to adjust the code to follow it, just let me know if that's the final API you want to use :)

Rosey commented 5 years ago

I am endlessly impressed by people who contribute to open source and how thorough they are in thinking things through and being kind and thoughtful and smart 🙂 Thank you! I think let's go with remarkablePreset/remarkableOptions and I'll merge with that change or if you end up getting too busy I can make the change myself.

kamilbielawski commented 5 years ago

Thank you for the kind words :) I've pushed a new commit. It changes the API to the one with separate remarkablePreset and remarkableOptions properties and adds one more test case, to verify that things work as expected both with and without the preset. I think it will do the trick, let me know if you see any issues with the code.

Rosey commented 5 years ago

LGTM! Thank you! 🙂