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

Allow fancy html5 ARTICLE tag in export #10

Closed

Conversation

titaniumbones
Copy link
Contributor

dynamically set the element we are searching for in the reply tree depending on value of org-html-html5-fancy.

fixes #8

@jeremy-compostella jeremy-compostella changed the title allow fancy html5 article tag in export Allow fancy html5 ARTICLE tag in export Sep 3, 2019
Copy link
Owner

@jeremy-compostella jeremy-compostella left a comment

Choose a reason for hiding this comment

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

Also to match the commit message style I have been following on this project the commit headline should be:
Allow fancy html5 ARTICLE tag in export

@@ -617,7 +618,7 @@ absolute paths."
(if (not original)
(assq-delete-all 'script (assq 'head reply))
(org-msg-improve-reply-header original css)
(push (assq 'div (assq 'body reply)) (cddr (assq 'body original))))
(push (assq 'reply-element (assq 'body reply)) (cddr (assq 'body original))))

Choose a reason for hiding this comment

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

It should not work. You are using the symbol 'reply-element instead of the reply-element variable value (which should be either 'article or 'div).
The correct syntax is:
(push (assq reply-element (assq 'body reply)) (cddr (assq 'body original))))

Choose a reason for hiding this comment

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

Since this is used in only one location, you could also:

(push (assq (if 'org-html-html5-fancy 'article 'div) (assq 'body reply))
            (cddr (assq 'body original))))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops, I screwed up my git history & hand-typed that. Yes, of course that's wrong as written.

would you rather have the second version? happy to switch. will update the commit message on the change.

Choose a reason for hiding this comment

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

Actually your patch was still broken because of an extra ' that I did not noticed.

I fixed it and pushed it. Let me know if it resolves your issue.

@titaniumbones
Copy link
Contributor Author

so we can close this now, right? thx. for pushing the fix.

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.

don't hardocde 'div when parsing html
2 participants