From mboxrd@z Thu Jan 1 00:00:00 1970 From: Rasmus Subject: Re: [patch][ox-html] Support for level based containers Date: Mon, 17 Mar 2014 01:30:11 +0100 Message-ID: <87ob153bvw.fsf@pank.eu> References: <87ha6z3vbi.fsf@gmx.us> <87vbveqofg.fsf@gmail.com> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:34439) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPLRg-0001LU-6a for emacs-orgmode@gnu.org; Sun, 16 Mar 2014 20:30:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WPLRa-0003EO-De for emacs-orgmode@gnu.org; Sun, 16 Mar 2014 20:30:24 -0400 Received: from mout.gmx.net ([74.208.4.201]:64098) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WPLRa-0003EE-6P for emacs-orgmode@gnu.org; Sun, 16 Mar 2014 20:30:18 -0400 List-Id: "General discussions about Org-mode." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org Sender: emacs-orgmode-bounces+geo-emacs-orgmode=m.gmane.org@gnu.org To: n.goaziou@gmail.com Cc: emacs-orgmode@gnu.org --=-=-= Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Hi, Thanks for the=E2=80=94as always=E2=80=94useful comments. I have attached = a new version of the patch with better doc and a bit better functionality, I think. Nicolas Goaziou 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) (=3D 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.=20 Thanks, Rasmus -- Hooray! --=-=-= Content-Type: text/x-diff Content-Disposition: attachment; filename=0001-Support-for-level-based-containers-in-ox-html.patch >From 3514d5bc9311f7e38fc9518c6a8f450c9fa3a37a Mon Sep 17 00:00:00 2001 From: Rasmus 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 --=-=-=--