zestedesavoir / zmarkdown

Live demo: https://zestedesavoir.github.io/zmarkdown/
MIT License
224 stars 52 forks source link

[remark-numbered-footnotes] Add labelTemplate option #510

Closed TimonPeng closed 4 months ago

StaloneLab commented 4 months ago

Hello there :wave:

good PR indeed, especially for a first one! I am not against this feature, and since you already added a test, there is not much to be done. However, I would prefer allowing to set a prefix and a suffix as configuration parameters instead of blindly replacing. It should allow the same as what you are trying to achieve without requiring throwing an error and would be easier to understand.

If you are ready to make this small change, then the feature will be ready to be merged. Please do not worry because of the CI failing, it should be resolved soon (see PR #509 if interested). As long as tests are running fine, I will manually test linting when the PR will be ready (you can also do it by running eslint . at the root of the repository).

One last thing to be done is building the package by running npm run build -- --scope=remark-numbered-footnotes. There is no need to take much time to make it work however: if it does not work at first run, please tell me and I'll do it for you, but seems like you should be able to do it ;)

Thanks again for this nice PR and awaiting your feedback. -- ლიკა

TimonPeng commented 4 months ago

Hi @LikaKavkasidze 👋

Thanks for the good advice! I've changed the options and finished compiling, testing and linting are working fine at the same time, hopefully the CI toolset update will be done soon!

StaloneLab commented 4 months ago

Thanks for making the requested changes, I will merge your change and release version 3.1.0 of the package in a few minutes!