Skip to content
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

Headers #387 #405

Closed
wants to merge 7 commits into from
Closed

Headers #387 #405

wants to merge 7 commits into from

Conversation

jmatsushita
Copy link

Hi @jamesramsay

I've implemented the (hopefully) smart header indentation from issue #387

I've made the call that if the transcluded content is indented and contains a header, then the header is not incremented.

I've also made a small API change which I can remove if you want, which is to pass the entire chunk to resolvers. This would open the door to implementing header incrementation in resolvers, or generally give access to the context of the transclusion.

I ran the benchmarks and it seems that the performance overhead is small:

master branch

hercule#transcludeFile x 538 ops/sec ±2.60% (75 runs sampled)
hercule#TranscludeStream x 564 ops/sec ±2.15% (75 runs sampled)
hercule#transcludeString x 622 ops/sec ±3.42% (79 runs sampled)

headers branch

hercule#transcludeFile x 507 ops/sec ±3.06% (76 runs sampled)
hercule#TranscludeStream x 544 ops/sec ±2.27% (76 runs sampled)
hercule#transcludeString x 639 ops/sec ±1.37% (79 runs sampled)

Let me know what you think!

Cheers,

Jun

@codecov
Copy link

codecov bot commented Oct 6, 2017

Codecov Report

Merging #405 into master will decrease coverage by 0.58%.
The diff coverage is 92%.

@@            Coverage Diff             @@
##           master     #405      +/-   ##
==========================================
- Coverage   99.32%   98.73%   -0.59%     
==========================================
  Files           8        9       +1     
  Lines         296      317      +21     
==========================================
+ Hits          294      313      +19     
- Misses          2        4       +2

@codecov
Copy link

codecov bot commented Oct 6, 2017

Codecov Report

Merging #405 into master will increase coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #405      +/-   ##
==========================================
+ Coverage   99.32%   99.36%   +0.04%     
==========================================
  Files           8        9       +1     
  Lines         296      317      +21     
==========================================
+ Hits          294      315      +21     
  Misses          2        2

@jamesramsay
Copy link
Owner

Thanks @jmatsushita. This is really clever. It would be ideal if we could make it a non-breaking change by making it an opt-in behavior.

Providing the entire chunk to the resolver functions sounds like it opens up quite a few interesting possibilities. Perhaps a more advanced set of markdown specific resolvers could be made available alongside the default resolvers to be used something like this:

import { trancludeFile, markdownResolvers } from 'hercule';
trancludeFile('foo.md', { markdownResolvers }, (err, output) => {
  // Handle exceptions like dead links
  if (err) console.log(err)
  console.log(output);
});

@jamesramsay jamesramsay closed this Jan 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants