terraform icon indicating copy to clipboard operation
terraform copied to clipboard

use headerIds to remove header IDs

Open wellingguzman opened this issue 6 years ago • 4 comments

Instead of overrides marked's default header with IDs using the renderer heading function, it uses the headerIds option added in version 0.4.0.

It also pass down the options parameter to marked to add the ability to set the headerIds or any options to marked.

wellingguzman avatar Dec 31 '18 04:12 wellingguzman

Hi @WellingGuzman thank you for your submission. Can you please run the tests, find which tests need to be altered and then make those changes inclusive with your PR?

Also as of this writing there is a relevant update to this issue coming: https://github.com/markedjs/marked/pull/1401 and it looks as though we'll need to wait and first decide if we are going to bump our markedjs version and then if so we'll want to roll in your changes at that time. Does that make sense? Don't want to introduce potential bugs and breaking changes without first having seen those features properly resolved upstream from us.

Super appreciate your contribution!

misterhtmlcss avatar Dec 31 '18 20:12 misterhtmlcss

I ran the tests, they all passed. I believe this line confirms there's not IDs in the header.

At the moment, there's no need to bump the version of marked, as it's already included in the version terraform uses, and because there's no way to enable headingIds through terraform (based on this line).

I believe this is safe to merge without any introducing potential bug, as this is only disabling it using the options instead of overrides the renderer function.

wellingguzman avatar Jan 08 '19 17:01 wellingguzman

I also removed the options from the marked to make this more likely to not be change at the moment. Either way this parameter was not intended to be marked options.

After https://github.com/markedjs/marked/pull/1401 is fixed/implemented, enabling headingIds would be ideal.

wellingguzman avatar Jan 08 '19 17:01 wellingguzman

@misterhtmlcss it's worth to mention that they released 0.6.0 hours after your comment.

wellingguzman avatar Jan 08 '19 18:01 wellingguzman