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

Custom HTML templates #38

Merged
merged 7 commits into from Jun 16, 2018
Merged

Custom HTML templates #38

merged 7 commits into from Jun 16, 2018

Conversation

tylerjpeterson
Copy link
Contributor

Resolves #33 by allowing users to provide a path to a custom markup template when rendering output as html. More specifically:

  • updated README.md with a list of variables that can be used within a custom template
  • updated main.js with the -t, --template option to use a custom html
  • updated cli.js to use template set in user config, otherwise defaults to existing template
  • imported fs from cli.js to confirm existence of custom template, otherwise throws an error
  • tests are passing

I realize this means your authorship may no longer appear in the generated output, so I fully understand if you're not comfortable accepting this PR.
Thanks!

@rtfpessoa
Copy link
Owner

@tylerjpeterson thanks for your contribution.

The only thing I am worried is the flag name since the diff2html library already has a template parameter that represents the actual templates of the diff and I think this would be confusing.

This template is a different thing that I never migrated to hogan.js and is still using a hardcoded replaces. What do you think of changing the name to html-wrapper-template, or wrapper-template?

@tylerjpeterson
Copy link
Contributor Author

@rtfpessoa - makes sense to me - I'm happy to update the flag. I'll try to get to it sometime this evening. Thanks!

@tylerjpeterson
Copy link
Contributor Author

tylerjpeterson commented Jun 11, 2018

@rtfpessoa - had a chance to update it now. Sorry for the sloppy commits, I didn't realize there was support for multi-character flags (which further reduces chance of confusion / collision with other dependencies). Let me know if this requires further updates.

@rtfpessoa rtfpessoa merged commit 7d84175 into rtfpessoa:master Jun 16, 2018
@rtfpessoa
Copy link
Owner

@dpetrov
Copy link

dpetrov commented Jun 19, 2018

Thanks @rtfpessoa, release looks good and thanks for the help.

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

3 participants