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

Body wrapper customization. Fix #280 #281

Closed
wants to merge 1 commit into from
Closed

Body wrapper customization. Fix #280 #281

wants to merge 1 commit into from

Conversation

wyuenho
Copy link
Contributor

@wyuenho wyuenho commented Nov 5, 2017

This allows the addition of a body preamble and epilogue to further customize the generated HTML.

Copy link
Owner

@jrblevin jrblevin left a comment

Choose a reason for hiding this comment

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

Thanks for this PR! I have a few small comments. Two lines seem to break the Markdown in the README, so those are more important.

If you are also able to add some simple tests and update CHANGES.md, then I'll be able to get this merged more quickly.

@@ -758,6 +759,14 @@
;; * `markdown-xhtml-header-content' - additional content to include
;; in the XHTML `<head>` block (default: `""`).
;;
;; * `markdown-xhtml-body-preamble' - additional content to include in the
;; XHTML <body> block, before the output. This is useful if you'd like to
;; wrap the Markdown output around extra elements (default: `""`')."
Copy link
Owner

Choose a reason for hiding this comment

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

Stray single quote (`) here.

;;
;; * `markdown-xhtml-body-epilogue' - additional content to include in the
;; XHTML <body> block, after the output. This is useful if you'd like to
;; wrap the Markdown output around extra elements (default: `""`').
Copy link
Owner

Choose a reason for hiding this comment

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

Stray single quote (`) here also.

@@ -7,6 +7,7 @@
;; Maintainer: Jason R. Blevins <jblevins@xbeta.org>
;; Created: May 24, 2007
;; Version: 2.4-dev
;; Package-Version: 20171031.1725
Copy link
Owner

Choose a reason for hiding this comment

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

Please remove this Package-Version header.

@@ -7770,7 +7791,13 @@ Standalone XHTML output is identified by an occurrence of
(insert markdown-xhtml-header-content))
(insert "\n</head>\n\n"
"<body>\n\n")
(when (> (length markdown-xhtml-body-preamble) 0)
(insert markdown-xhtml-body-preamble))
(insert "\n")
Copy link
Owner

Choose a reason for hiding this comment

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

It might be better to put this newline inside the when block, so that existing output for those who haven't customized this variable will be exactly the same.

(goto-char (point-max))
(insert "\n")
Copy link
Owner

Choose a reason for hiding this comment

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

Same for this newline (see above).

@jrblevin
Copy link
Owner

jrblevin commented Nov 6, 2017

Here is a sketch of a test that I started working on. I didn't test the test yet though, but hopefully if it doesn't work as is it can be a good starting point.

(ert-deftest test-markdown-export/body-preamble ()
  "Test `markdown-xhtml-body-preamble'."
  (markdown-test-temp-file "export-test.md"
    (let* ((str "<!-- body preamble test -->")
           (markdown-xhtml-body-preamble str)
           (ofile (markdown-export))
           (obuffer (get-file-buffer ofile)))
      (with-current-buffer obuffer
        (goto-char (point-min))
        (should (search-forward str)))
      (kill-buffer obuffer)
      (delete-file ofile))))

jrblevin added a commit that referenced this pull request Dec 30, 2017
Add markdown-xhtml-body-preamble and markdown-xhtml-body-epilogue
variables for wrapping additional XHTML tags around the output.

Based on pull request GH-281 by Jimmy Yuen Ho Wong (@wyuenho).
Closes GH-280.
@wyuenho
Copy link
Contributor Author

wyuenho commented Dec 30, 2017

Ah I've completely forgotten about this PR. Thanks for fixing.

@wyuenho wyuenho closed this Dec 30, 2017
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.

2 participants