-
-
Notifications
You must be signed in to change notification settings - Fork 709
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow custom open and close parameters #110
base: master
Are you sure you want to change the base?
Conversation
Thanks @PrimordialHelios I'll try to review this today. |
Thanks! Take your time with it, I understand you're busy so I don't mind waiting :) |
@PrimordialHelios thanks for the PR. Will you add some tests to this section for testing the Also, I think some changes will be necessary here to ensure the plugin works correctly. I think some tests using it as a remarkable plugin will cover that part. |
@@ -5,7 +5,7 @@ var toc = require('./index.js'); | |||
var utils = require('./lib/utils'); | |||
var args = utils.minimist(process.argv.slice(2), { | |||
boolean: ['i', 'json', 'firsth1', 'stripHeadingTags'], | |||
string: ['append', 'bullets', 'indent'], | |||
string: ['append', 'bullets', 'indent', 'open', 'close'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@PrimordialHelios there was a merge conflict here from a previous PR. I fixed it by adding , 'indent'
.
I've never written unit tests before, so I added some testing with custom open and close options to test the regex. Mind looking at them and letting me know if I should do anything different? EDIT: Updated link with a newer commit |
These are great! I think it looks good. I'll try to get this merged in soon. |
Awesome, thanks! I tried squashing all the commits to make it easier to deal with. I think I got it right, but if there's anything wrong with it, just let me know and I'll sort it out. |
@doowb Just figured I'd ping you in case you'd forgotten about this PR |
Can we get this merged? This would be a very useful feature. |
In reference to #109
Here's what I have so far. The code might not be the greatest, but I thought it would be best to get down a starting point and see what you guys thought.