Skip to content
This repository has been archived by the owner on Oct 16, 2024. It is now read-only.

Fixes #12: Add ability to output markdown #25

Open
wants to merge 24 commits into
base: master
Choose a base branch
from

Conversation

postatum
Copy link
Contributor

@postatum postatum commented Mar 3, 2020

Fixes #12

Try it out with a new option --syntax:

npm run aml2doc -- ./outdir --indir=./test_data --syntax=md

We still need to rename the repo after this PR is merged.


I've fixed this issue by simply declaring a second set of templates specifically for Markdown.

While working on it, I've considered few other options:

  1. Convert HTML->Markdown. This didn't work out because I've only found few decent converters (turndown, showdown) and none of them properly handled our html structure (tables, navigation block, etc.).
  2. Convert Markdown->HTML. I decided to not follow this path because if we had templates in md, rendered them and then converted to html, it would break multiple things: css classes/ids, using custom css files.

Choosing either of these options would have added unnecessary complexity to the code (to make out layout with those libs) and increased number of bugs in the future. On the other hand, I think keeping templates up to date is straightforward and add no code complexity.


Migration

  1. Move your custom HTML templates to html folder under usual templates path. E.g. if your templates resided at /somewhere/mytemplates, move them to /somewhere/mytemplates/html.
  2. In your custom HTML templates switch from {{{htmlName}}} to {{{pageName}}}.html for links and to {{{pageName}}} for other things. Notice that while htmlName variable values looked like foo.html, pageName values look like foo.
  3. Same as 2 but switch from {{{rangeHtmlName}}} to {{{rangePageName}}}

@postatum
Copy link
Contributor Author

postatum commented Mar 3, 2020

@antoniogarrote please review.

Copy link
Contributor

@jstoiko jstoiko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@postatum: can you please try to run CIM (Cloud Information Model) using this and compare the output before vs after this PR?

Copy link
Contributor

@antoniogarrote antoniogarrote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@postatum
Copy link
Contributor Author

postatum commented Mar 24, 2020

@postatum: can you please try to run CIM (Cloud Information Model) using this and compare the output before vs after this PR?

I've compared CIM output HTML using master branch of this repo vs this PR branch and the only change in output HTML looks like this:

-                          <tr id="schema_shipmententitygroup_shippingmethod.html-schemas-tr">
+                          <tr id="schema_shipmententitygroup_shippingmethod-schemas-tr">

This is because previously there were variables like {{pageHtml}} values of which included .html in them while now these are called {{pageName}} and don't have .html in them.


I've created a PR to help migrate it to aml2doc: cloudinformationmodel/cloudinformationmodel#9


I've also added migration guide for this PR for projects that use custom HTML templates.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ability to output mardown
3 participants