Hi,
Thanks for the—as always—useful comments. I have attached a new
version of the patch with better doc and a bit better functionality, I
think.
Nicolas Goaziou <n.goaz...@gmail.com> writes:
> I don't know enough HTML to have an opinion here.
Then we are two. . .
>> and what changes are necessary, if any.
>
> Some comments follow.
>
>> +(defcustom org-html-container-element '(("div" . "section")
>> + ("div" . "article")
>> + ("div" . "div"))
>> + "HTML elements to use for wrapping sections.
>> Can be set with the in-buffer HTML_CONTAINER property or for
>> publishing, with :html-container.
>>
>> -Note that changing the default will prevent you from using
>> -org-info.js for your website."
>> +Should be a list of cons cells with positions corresponding to a
>
> A "list of cons cells" is an "alist".
I think I knew that at some point, but forgot along the way. At least
"list of cons" is better than "list of tuples". . .
>> +section. If `org-html-html5-fancy' is t the cdr is used
>> +otherwise the car.
>
> "is non-nil" is better than "is t". Also, you shouldn't use
> `org-html-html5-fancy': see below.
>> +Note that changing the default will prevent you from
>> +using org-info.js for your website."
>> :group 'org-export-html
>> :version "24.4"
>> :package-version '(Org . "8.0")
>> - :type 'string)
>> + :type '(repeat (cons string string)))
>
> There is an `alist' type. See (info "(elisp) Composite Types")
I went for another solution as I would prefer not to break backward
comparability.
>> + (let* ((hc (plist-get info :html-container))
>> + (n (org-export-get-relative-level headline info)))
>
> You don't need a starred `let' here. Also, I suggest to not use short
> variable names. IMO "container-alist" is better than "hc" and "level"
> better than "n".
Good rule of thumb.
>> + (cond ((listp hc)
>> + (or (funcall (if org-html-html5-fancy 'cdr-safe 'car-safe)
>> + (nth (1- (min n (length hc))) hc)) "div"))
>
> You shouldn't use directly the variable `org-html-html5-fancy' since its
> value can be overridden with external properties (e.g., during
> a publishing process, with a specific setup). Instead, it should be:
>
> (plist-get info :html-html5-fancy)
>
> As a rule of thumb, don't use variables when there's a property in INFO
> for them.
You are right. Thanks for spotting it.
> Also, I don't think you need to use `car-safe' instead of `car' and
> `cdr-safe' instead of `cdr'.
My "defense" is, people could make mistakes, e.g. provide the list
'("div" "div" "div"). But you are right.
> Eventually, due to the (min n (length hc)) (which should be documented
> in the docstring) and the fact that the alist cannot be empty, the
> `funcall' never evaluates to nil. Therefore, the `or' is not necessary.
See below.
>> + ((and (stringp hc) (= 1 n))
>> + (plist-get info :html-container))
>
> Note that this branch is always false since HC shouldn't be a string,
> per the defcustom type, but an alist.
Yeah, but since ATM it's a string, there may be people who've
customized it in there init.el or in project definition. I wouldn't
want to break it for them. . . I have adjusted the docstring to
consider this.
>> + (t "div")))))
>
> Given the recommendations above, the whole `cond' could be rewritten:
>
> (funcall (if (plist-get info :html-html5-fancy) #'cdr #'car)
> (nth (1- (min level (length container-alist))) container-alist))
More elegant.
Thanks,
Rasmus
--
Hooray!
>From 3514d5bc9311f7e38fc9518c6a8f450c9fa3a37a Mon Sep 17 00:00:00 2001
From: Rasmus <w...@pank.eu>
Date: Sun, 16 Mar 2014 00:01:35 +0100
Subject: [PATCH] Support for level-based containers in ox-html
* ox-html.el (org-html-container-element): List of
cons of section-level containers.
(org-html--container): Redefine to consider
org-html-container-element.
---
lisp/ox-html.el | 48 +++++++++++++++++++++++++++++++++++++++---------
1 file changed, 39 insertions(+), 9 deletions(-)
diff --git a/lisp/ox-html.el b/lisp/ox-html.el
index a8c924f..f34b3a0 100644
--- a/lisp/ox-html.el
+++ b/lisp/ox-html.el
@@ -940,17 +940,36 @@ versions 8 and below."
:package-version '(Org . "8.0")
:type 'boolean)
-(defcustom org-html-container-element "div"
- "HTML element to use for wrapping top level sections.
+(defcustom org-html-container-element '(("div" . "section")
+ ("div" . "article")
+ ("div" . "div"))
+ "HTML elements to use for wrapping sections.
Can be set with the in-buffer HTML_CONTAINER property or for
publishing, with :html-container.
-Note that changing the default will prevent you from using
-org-info.js for your website."
+Should be an alist where each cons-cell corresponds to a section
+levels. The first cons-cell is used for top-level headlines, the
+second cons-cell is used for second level headlines and so forth.
+Headlines with levels higher than the length of the alist use the
+last cons-cell.
+
+If the output is \"html5\" (see `org-html-doctype') and
+html5-fancy is non-nil (see `org-html-html5-fancy') then the cdr
+of cons-cell is used. Otherwise the car is used.
+
+May also be a single string corresponding to the top-level
+container for backward comparability.
+
+Note that changing the default will prevent you from
+using org-info.js for your website."
:group 'org-export-html
:version "24.4"
:package-version '(Org . "8.0")
- :type 'string)
+ :type '(choice (repeat :tag "Fine control"
+ (choice (cons :tag "level"
+ (string :tag "Default")
+ (string :tag "Fancy HTML5"))))
+ (string :tag "Control top level only" :value "div")))
(defcustom org-html-divs
'((preamble "div" "preamble")
@@ -2411,10 +2430,21 @@ holding contextual information."
(org-html--container headline info)))))))
(defun org-html--container (headline info)
- (or (org-element-property :HTML_CONTAINER headline)
- (if (= 1 (org-export-get-relative-level headline info))
- (plist-get info :html-container)
- "div")))
+ "Select an appropriate HTML container for a headline.
+
+The container is selected based on level and depend on
+:html-container corresponding to org-html-container-element."
+ (let ((container-alist (plist-get info :html-container))
+ (level (org-export-get-relative-level headline info)))
+ (cond ((listp container-alist)
+ (funcall (if (and (org-html-html5-p info)
+ (plist-get info :html-html5-fancy)) #'cdr #'car)
+ (nth (1- (min level (length container-alist)))
+ container-alist)))
+ ;; Previously :html-container took a string as an argument.
+ ((and (stringp container-alist) (= 1 level))
+ (plist-get info :html-container))
+ (t "div"))))
;;;; Horizontal Rule
--
1.9.0
