From mboxrd@z Thu Jan 1 00:00:00 1970 From: Nicolas Goaziou Subject: Re: [patch, ox] Unnumbered headlines Date: Fri, 26 Sep 2014 09:51:09 +0200 Message-ID: <87d2aiesce.fsf@nicolasgoaziou.fr> References: <87lhqzyubg.fsf@gmx.us> <87bnrrp0tb.fsf@nicolasgoaziou.fr> <87r40n6nrg.fsf@gmx.us> <87egwmaxte.fsf@nicolasgoaziou.fr> <87k34y701i.fsf@gmx.us> <87fvfl86ct.fsf@nicolasgoaziou.fr> <87k34xghtt.fsf@gmx.us> <87ppeon4mw.fsf@nicolasgoaziou.fr> <87tx4020lh.fsf@gmx.us> <87vboflkil.fsf@nicolasgoaziou.fr> <87iokfdvi3.fsf@gmx.us> Mime-Version: 1.0 Content-Type: text/plain Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:33118) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XXQIf-0003JJ-0v for emacs-orgmode@gnu.org; Fri, 26 Sep 2014 03:50:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1XXQIW-0006q1-Sm for emacs-orgmode@gnu.org; Fri, 26 Sep 2014 03:50:44 -0400 Received: from relay6-d.mail.gandi.net ([2001:4b98:c:538::198]:49779) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1XXQIW-0006om-K7 for emacs-orgmode@gnu.org; Fri, 26 Sep 2014 03:50:36 -0400 In-Reply-To: <87iokfdvi3.fsf@gmx.us> (rasmus@gmx.us's message of "Tue, 23 Sep 2014 02:35:16 +0200") 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: Rasmus Cc: emacs-orgmode@gnu.org Hello, Rasmus writes: > Another couple of small changes. Thank you. > Using this file: > > * h1 > :PROPERTIES: > :CUSTOM_ID: h1 > :END: > ** h2 > :PROPERTIES: > :unnumbered: t > :CUSTOM_ID: h2 > :END: > *** h3 > *** h4 > * h5 > :PROPERTIES: > :CUSTOM_ID: h5 > :END: > [[*h1]] [[#h2]] [[*h4]] [[#h5]] > ** h6 > > The output is now > > \section{h1} > \label{sec-1} > \subsection*{h2} > \label{unnumbered-1} > \subsubsection*{h3} > \label{unnumbered-2} > \subsubsection*{h4} > \label{unnumbered-3} > \section{h5} > \label{sec-2} > \ref{sec-1} \hyperref[unnumbered-1]{h2} \hyperref[unnumbered-3]{h4} \ref{sec-2} > \subsection{h6} > \label{sec-2-1} > > Which I think is quite good. I agree. > I don't know if the global unnumbered counter is made in the best way. > I add another plist to info with the number. This approach is cleaner > than before since it's the numbering of unnumbered headlines is not in > `org-export--collect-headline-numbering' which is complicated enough > as it is. 14 locs long functions do not play in the "complicated enough" league. Anyway, your implementation is fine, too. > Should I write tests for the new behavior? If so, tests for each > backend or only for vanilla-ox functions? Tests for "ox.el" are mandatory. See "test-ox.el" > * ox.el (org-export--collect-headline-numbering): Return nil > if unnumbered headline. This is not exactly true: "Ignore unnumbered headlines." would be more appropriate. > (org-export-get-headline-id): New defun that returns a unique > ID to a headline. "New function." is enough. > + (if number > (if (atom number) (number-to-string number) > - (mapconcat 'number-to-string number ".")))))))) > + (mapconcat 'number-to-string number ".")) > + ;; unnumbered headline > + (when (eq 'headline (org-element-type destination)) > + (format "[%s]" (org-export-data (org-element-property :title destination) info))))))))) While you're at it: #'number-to-string. > (ids (delq nil > (list (org-element-property :CUSTOM_ID headline) > - (concat "sec-" section-number) > + (and section-number (concat "sec-" section-number)) > (org-element-property :ID headline)))) > - (preferred-id (car ids)) > + (preferred-id (org-export-get-headline-id headline info)) I think the following is more in the spirit of the code (you don't ignore :custom-id property): (ids (delq nil (list (org-element-property :CUSTOM_ID headline) (org-export-get-headline-id headline info) (org-element-property :ID headline)))) (preferred-id (car ids)) > (extra-ids (mapconcat > (lambda (id) > (org-html--anchor > @@ -2807,21 +2807,7 @@ INFO is a plist holding contextual information. See > (org-element-property :raw-link link) info)))) > ;; Link points to a headline. > (headline > - (let ((href > - ;; What href to use? > - (cond > - ;; Case 1: Headline is linked via it's CUSTOM_ID > - ;; property. Use CUSTOM_ID. > - ((string= type "custom-id") > - (org-element-property :CUSTOM_ID destination)) > - ;; Case 2: Headline is linked via it's ID property > - ;; or through other means. Use the default href. > - ((member type '("id" "fuzzy")) > - (format "sec-%s" > - (mapconcat 'number-to-string > - (org-export-get-headline-number > - destination info) "-"))) > - (t (error "Shouldn't reach here")))) > + (let ((href (org-export-get-headline-id destination info)) This chuck needs to be updated since headline-id doesn't replace :custom-id or :id properties. > (headline-label > - (let ((custom-label > - (and (plist-get info :latex-custom-id-labels) > - (org-element-property :CUSTOM_ID headline)))) > - (if custom-label (format "\\label{%s}\n" custom-label) > - (format "\\label{sec-%s}\n" > - (mapconcat > - #'number-to-string > - (org-export-get-headline-number headline info) > - "-"))))) > + (format "\\label{%s}\n" (org-export-get-headline-id headline info))) Ditto. > - (org-html--anchor > - (or (org-element-property :CUSTOM_ID headline) > - (concat "sec-" > - (mapconcat 'number-to-string > - (org-export-get-headline-number > - headline info) "-"))) > + (org-html--anchor (org-export-get-headline-id headline info) Ditto. > (format "(%s)" > (format (org-export-translate "See section %s" :html info) > - (mapconcat 'number-to-string > - (org-export-get-headline-number > - destination info) > - "."))))))) > + (if (org-export-numbered-headline-p destination info) > + (mapconcat 'number-to-string > + (org-export-get-headline-number > + destination info) > + ".") > + (org-export-get-alt-title headline info)))))))) No alt title please, as discussed before. (org-export-data (org-element-property :title headline) info) > ((org-export-inline-image-p link org-html-inline-image-rules) > (let ((path (let ((raw-path (org-element-property :path link))) > (if (not (file-name-absolute-p raw-path)) raw-path > @@ -354,9 +351,13 @@ a communication channel." > (if (org-string-nw-p contents) contents > (when destination > (let ((number (org-export-get-ordinal destination info))) > - (when number > + (if number > (if (atom number) (number-to-string number) > - (mapconcat 'number-to-string number ".")))))))) > + (mapconcat 'number-to-string number ".")) > + ;; unnumbered headline > + (and (eq 'headline (org-element-type destination)) > + ;; BUG: shouldn't headlines have a form like [ref](name) in md > + (org-export-data (org-export-get-alt-title destination info) info)))))))) Ditto. Also, #'number-to-string while you're at it. > (let* ((headline-no > - (org-export-get-headline-number destination info)) > - (label > - (format "sec-%s" > - (mapconcat 'number-to-string headline-no "-")))) > + (if (org-export-numbered-headline-p destination info) > + (org-export-get-headline-number destination info) > + (org-element-property :title destination))) (org-export-data (org-element-property :title destination) info) However, I'm not sure this the expected headline-no. > + (unless (or (not (org-export-numbered-headline-p headline options)) > + (org-element-property :footnote-section-p headline)) (when (and (not (org-element-type :footnote-section-p headline)) (org-export-numbered-headline-p headline options)) ...) > +(defun org-export--collect-unnumbered-headline-id (data options) > + "Return a numbering of all unnumbered headlines. > + > +Unnumbered headlines are given numbered after occurrence." You need to give details about the arguments, e.g., "Associate a global counter to all unnumbered headlines DATA is the parse tree. OPTIONS is a plist containing export options." > + (let ((num 0)) > + (org-element-map data 'headline > + (lambda (headline) > + (unless (org-export-numbered-headline-p headline options) > + (cons headline (list (setq num (1+ num))))))))) Last line: (list headline (incf num)) > (defun org-export-get-headline-number (headline info) > - "Return HEADLINE numbering as a list of numbers. > + "Return numbered HEADLINE numbering as a list of numbers. > +INFO is a plist holding contextual information." > + (and (org-export-numbered-headline-p headline info) > + (cdr (assoc headline (plist-get info :headline-numbering))))) Use `assq' instead of `assoc'. > +(defun org-export-get-unnumberd-headline-id (headline info) > + "Return unnumbered HEADLINE id as list of numbers. > INFO is a plist holding contextual information." > - (cdr (assoc headline (plist-get info :headline-numbering)))) > + (and (not (org-export-numbered-headline-p headline info)) > + (cdr (assoc headline (plist-get info :unnumbered-headline-id))))) I don't think it is worth to make this function standalone. I don't see any use case outside `org-export-get-headline-id'. I suggest to move it there. Also, `assoc' -> `assq'. > + (unless > + (or (org-export-get-node-property :UNNUMBERED headline) > + (loop for parent in (org-export-get-genealogy headline) > + when (org-export-get-node-property :UNNUMBERED parent) > + return t)) (unless (org-some (lambda (h) (org-not-nil (org-element-property :UNNUMBERED h))) (org-export-get-genealogy headline)) ...) Regards, -- Nicolas Goaziou