markdig icon indicating copy to clipboard operation
markdig copied to clipboard

Markdown fragment files

Open WattsC-90 opened this issue 5 years ago • 11 comments

Is it possible, with the library as it is, to suport fragmented markdown files? I am looking to hse this library as a documentation tool, and the only feature i cant see listed is file fragments! If not, ill have a dig through the code when i get a moment..

WattsC-90 avatar Mar 01 '20 19:03 WattsC-90

Can you show a small example of what you mean? Like C#'s partial classes, but for markdown?

MihaZupan avatar Mar 01 '20 19:03 MihaZupan

Kind of, but with the ability for the partial file to be unpackaged at the inclusion line.. See this plugin for vs code https://marketplace.visualstudio.com/items?itemName=yzane.markdown-pdf in the section markdown-it-include. Unfortunately i cant permalink

WattsC-90 avatar Mar 01 '20 19:03 WattsC-90

There is no out-of-the-box support for something like this, but it should be rather straight forward to implement as an extension.

For example, you can implement a parser to catch the include syntax and emit a MarkdownIncludeBlock. Then implement an HtmlRenderer<MarkdownIncludeBlock> to emit the Markdown.ToHtml of the specified file.

MihaZupan avatar Mar 01 '20 21:03 MihaZupan

@MihaZupan I have a preliminary cut of the code working, I have found some restrictions though with potential rendering implementation.. I dont want to ruin the commit history though.. is it OK to commit and push on a branch to the repo?

WattsC-90 avatar Mar 04 '20 22:03 WattsC-90

If you have a local change, you can fork this repo and push your changes there.

MihaZupan avatar Mar 04 '20 23:03 MihaZupan

done =] welcome the feedback (but will add a disclaimer.. it was late and i was tired) not that a coder has used THAT excuse before ;)

The SimpleExample project has two markdown files and shows the include line working/rendering

WattsC-90 avatar Mar 05 '20 07:03 WattsC-90

it was late and i was tired

That's when the cool code appears.

Thanks. Yes, that would be the general idea for approaching it. From the comments in the code, I see you've also thought about a few problems I'll mention below.

This could be a neat extension, but it would require a few more usability additions, at minimum:

  1. Specifying the root directory for include files (a parameter when adding calling UseMarkdownInclude or somthing similar), or alternatively a callback that fetches the filepath/source based on the path (just a Func<string, string>).
  2. Recursion detection (with throw/ignore behavior) - this one might be more tricky since we don't get the source file name, but we can still detect it later, eg. main.md > header.md > main.md > header.md (boom), as we see the included file name.

I would be careful about using :[alternate-text](includeTest.md) as the syntax as it could be easy to produce false-positives with links to other files. What the extension you linked to does is !!!include(header.md)!!!, but that's up to personal preference and could be made configurable if needed.

Comments about the implementation:

  • Instead of adding the parser/renderer manually, you can make an extension. The extension then adds the parser and renderer to the pipeline, but you get the benefit of easily flowing configuration. For example you have MarkdownPipelineBuilder.UseMarkdownInclude(SomeConfig) which adds a new MarkdownIncludeExtension(SomeConfig) to the pipeline. The extension then adds the parser and renderer while flowing the configuration to them. It can also capture the pipeline, so that you can use it in the renderer.
  • In the parser, we can first parse just the indexes/lengths and only allocate substrings when we know we hit some valid include syntax. Alternatively a simple win is to replace new StringBuilder() with StringBuilderCache.Local(). A side note on StringBuilder: it's more efficient to reuse it by doing sb.Length = 0 than to allocate a new one.

To concider: Optionally cache the included files? I would expect the common use case for this to include the header/footer that doesn't change.

MihaZupan avatar Mar 05 '20 15:03 MihaZupan

I would be careful about using :[alternate-text](includeTest.md) as the syntax as it could be easy to produce false-positives with links to other files. What the extension you linked to does is !!!include(header.md)!!!, but that's up to personal preference and could be made configurable if needed.

That's weird.. this confused me so I looked at the page i linked to, the markdown-it-include uses the format you suggest, but the markdown pdf extension for VS Code uses the one I implemented.. I agree the format include(path) is better given the closeness to the link style/format. I will switch it over to use the markdown-it-include format instead, in addition the "alternate name" doesnt actually make sense in this context either so its just plain unnecessary.

The path is assumed to always be local/relative, but a check could be added to enable that not being the case.

A side note on StringBuilder: it's more efficient to reuse it by doing sb.Length = 0 than to allocate a new one.

See what I mean about coding tired! I knew that ! :( I didn't know about the StringBuilderCache though, that will be a definite improvement and reduce the allocations.

So moving forward:

  1. Change the pattern to !!!include(file)!!!
  2. Move to an extension
  3. Handle passing in the pipeline/context
  4. Setting include root path as a configuration (default to .)
  5. [further down the line] recursion/loop detection
  6. [further down the line] caching for footers etc.

Thanks for the feedback!

WattsC-90 avatar Mar 05 '20 16:03 WattsC-90

I have suggested this in the past, but sometimes this problem is handled differently through a text templating (e.g Jekyll with GitHub pages). You can use something like scriban for example (I did use it along Markdig to implement a prototype of static website generator for instance).

But if a markdown syntax is not enough common (supported by a few of the major Markdown engine), I would prefer not to be included in Markdig default extensions. For example, they are using their own include syntax in DocFx and it is working fine for their case.

xoofx avatar Mar 05 '20 17:03 xoofx

@WattsC-90 - Do you have your latest changes committed anywhere? I'd love to leverage what you've created so far and extend upon it if need be. Nice work!!!

ipoley-r7 avatar Apr 07 '20 20:04 ipoley-r7

Unfortunately not, given everything covid related at the moment iv been working from home and as such tried to stay away from code in the evenings! Wasnt ever leaving the screen otherwise. My intention was to try do a couple of hours this weekend which will move it to an extension as opposed to fully inside the main engine. I will let you know where I get to..

WattsC-90 avatar Apr 07 '20 20:04 WattsC-90