[ISSUE-18]Adds support for custom HTML template provision via command arguments or configuration#19
Conversation
…cting html reference
… arguments or configuration
|
|
||
| self._templates = { | ||
| 'head': assets.get('head.tmpl.html').decode('utf8'), | ||
| 'foot': assets.get('foot.tmpl.html').decode('utf8'), | ||
| 'search': assets.get('search.tmpl.html').decode('utf8'), | ||
| 'head': open(head_template, 'r', encoding='utf8').read() if head_template else assets.get('head.tmpl.html').decode('utf8'), | ||
| 'foot': open(foot_template, 'r', encoding='utf8').read() if foot_template else assets.get('foot.tmpl.html').decode('utf8'), | ||
| 'search': open(search_template, 'r', encoding='utf8').read() if search_template else assets.get('search.tmpl.html').decode('utf8'), |
There was a problem hiding this comment.
We can't assume UTF-8 here when reading the user-provided templates. We can assume UTF-8 when they're a distributed asset since encoding is under our control, but when it's external and user-defined, it could be a different encoding.
Input encoding is configurable, so you can grab the config file for encoding per
Line 222 in a5378e4
| help='Custom CSS file (html renderer)') | ||
| p.add_argument('--favicon', action='store', type=str, metavar='FILE', | ||
| help='Path to favicon file (html renderer)') | ||
| p.add_argument('--head_template', action='store', type=str, metavar='FILE', |
There was a problem hiding this comment.
Don't use underscores for command line flags, use dashes instead. So --head-template, --foot-template, and --search-template.
| css = self.config.get('project', 'css', fallback=None) | ||
| if css: | ||
| # The stylesheet is always copied to doc root, so take only the filename | ||
| _, css = os.path.split(css) |
There was a problem hiding this comment.
This commit slipped in from the other PR. Ideally PRs would be independent from one another, which requires a separate branch per PR on your fork. This allows PRs to be merged independent of one another. It's not a big deal in this one, because I already merged #17 so this will be ignored, but it's more of a problem for the other PRs.
|
@jtackaberry, there is a new commit with review changes. Thanks! P.S. I will wait for merge and rebase the next PR's as they only really make sense on top of this. Is that fine? |
2 participants
Refer to ISSUE-18
@jtackaberry Could you please review and consider this change for inclusion in main repo?