[chore] Switch toc generation to doctoc#3463
Conversation
chalin
left a comment
There was a problem hiding this comment.
Nice work!
I'd prefer to keep the current markers <!-- toc --> and <!-- tocstop --> without any extra comments, at least for this PR -- it would help us focus on the essential change in this PR (which is very minimal). Besides, IMHO folks generally know that the toc is generated by now. I like to keep things DRY and avoid repeating update instructions in every file.
|
No worries @chalin, Totally agree with you about keeping things dry, unfortunately those comments are currently hard code by doctoc. 😔 When reviewing this PR I did have a similiar concern about the noise from those comments hence I raised thlorenz/doctoc#315 In relation to the indicator |
|
Btw, great job on the improvements you contributed to doctoc -- https://github.com/thlorenz/doctoc/commits?author=thompson-tomo! Thanks for doing that 🙌🏻
I see 🤔
Ok, I 👍🏻 that issue. Might you be planning a PR to fix that? If so, what's the usual turnaround? |
|
Thanks, hopefully the updates makes thing easier/more efficient and importantly more reliable for users. Yes I was thinking about tackling it after a refactor which included breaking changes as it became clear all the additional settings are not scaling nicely and it caused me grief when adding due to merge conflicts. I could do a partial implementation ie the remove option if we needed to sooner but a review usually takes weeks with nudges. Let me know your preference as who knows we might get lucky. |
|
I'd like to keep this PR (and our git history) as clean as we can. We probably don't want to wait a few more weeks. Let's assume that thlorenz/doctoc#315 will get fixed ... in the meantime, to streamline this PR, we can resort to a simple Perl search/replace for prep- and post-processing before/after calling doctoc. (I'd prefer Perl over sed because it's more portable, but given that that Makefile has a |
|
@chalin just following up on this PR. |
9d5f937 to
eff1636
Compare
|
@chalin could you please take another review of this. |
|
This PR has been labeled as stale due to lack of activity. It will be automatically closed if there is no further activity over the next 7 days. |
5bc334a to
20025a4
Compare
|
@chalin i have now removed the scripts as doctoc now natively supports the compact pragma like we were achieving using the scripts. |
chalin
left a comment
There was a problem hiding this comment.
@thompson-tomo - thanks for sticking with this and following up. My earlier concerns look addressed now:
- compact/doctoc markers without the extra noise
- markdown-toc kept as the mutating target
- separate markdown-toc-check for CI validation
- temporary workaround scripts removed
As final prep, pls:
- rebase, and
- regenerate a new package lock file
Note: using
.as the target path shouldn't cause problems, but we might need to adjust that if it does.
d216d80 to
8a1a8aa
Compare
|
No worries at all @chalin i have just performed a rebase & updated doctoc to the latest which includes updating the lock file. |
7b0c0fe
|
Woot! 🎉 Thanks you @thompson-tomo! Thanks all! |
Changes
This switches to doctoc for toc generation.
The benefit of this switch includes:
Important
Pull requests acceptance are subject to the triage process as described in Issue and PR Triage Management.
PRs that do not follow the guidance above, may be automatically rejected and closed.
Merge requirement checklist
[chore]