[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [O] [patch, ox] Unnumbered headlines
From: |
Nicolas Goaziou |
Subject: |
Re: [O] [patch, ox] Unnumbered headlines |
Date: |
Fri, 26 Sep 2014 09:51:09 +0200 |
Hello,
Rasmus <address@hidden> 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
- Re: [O] [patch, ox] Unnumbered headlines, Rasmus, 2014/09/20
- Re: [O] [patch, ox] Unnumbered headlines, Alan L Tyree, 2014/09/20
- Re: [O] [patch, ox] Unnumbered headlines, Nicolas Goaziou, 2014/09/21
- Re: [O] [patch, ox] Unnumbered headlines, Rasmus, 2014/09/21
- Re: [O] [patch, ox] Unnumbered headlines, Nicolas Goaziou, 2014/09/21
- Re: [O] [patch, ox] Unnumbered headlines, Rasmus, 2014/09/21
- Re: [O] [patch, ox] Unnumbered headlines, Nicolas Goaziou, 2014/09/22
- Re: [O] [patch, ox] Unnumbered headlines, Rasmus, 2014/09/22
- Re: [O] [patch, ox] Unnumbered headlines, Thomas S. Dye, 2014/09/22
- Re: [O] [patch, ox] Unnumbered headlines,
Nicolas Goaziou <=
- Re: [O] [patch, ox] Unnumbered headlines, Rasmus, 2014/09/26
- Re: [O] [patch, ox] Unnumbered headlines, Nicolas Goaziou, 2014/09/27
- Re: [O] [patch, ox] Unnumbered headlines, Rasmus, 2014/09/30