emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Nicolas Goaziou <mail@nicolasgoaziou.fr>
To: Rasmus <rasmus@gmx.us>
Cc: emacs-orgmode@gnu.org
Subject: Re: [patch, ox] Unnumbered headlines
Date: Fri, 26 Sep 2014 09:51:09 +0200	[thread overview]
Message-ID: <87d2aiesce.fsf@nicolasgoaziou.fr> (raw)
In-Reply-To: <87iokfdvi3.fsf@gmx.us> (rasmus@gmx.us's message of "Tue, 23 Sep 2014 02:35:16 +0200")

Hello,

Rasmus <rasmus@gmx.us> 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

  parent reply	other threads:[~2014-09-26  7:50 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-08-08 13:39 [patch, ox] Unnumbered headlines Rasmus
2014-08-08 22:35 ` Alan L Tyree
2014-08-09  1:04 ` [patch, ox] Unnumbered headlines - early test Alan L Tyree
2014-08-09  7:47 ` [patch, ox] Unnumbered headlines Detlef Steuer
2014-08-11 14:18 ` Nicolas Goaziou
2014-08-11 15:37   ` Rasmus
2014-08-12  8:58     ` Nicolas Goaziou
2014-09-20 16:02       ` Rasmus
2014-09-20 20:34         ` Alan L Tyree
2014-09-21 13:12         ` Nicolas Goaziou
2014-09-21 14:37           ` Rasmus
2014-09-21 19:40             ` Nicolas Goaziou
2014-09-21 20:13               ` Rasmus
2014-09-22 15:53                 ` Nicolas Goaziou
2014-09-23  0:35                   ` Rasmus
2014-09-23  1:10                     ` Thomas S. Dye
2014-09-26  7:51                     ` Nicolas Goaziou [this message]
2014-09-26 13:48                       ` Rasmus
2014-09-27  8:19                         ` Nicolas Goaziou
2014-09-30 22:54                       ` Rasmus
2014-10-02  0:35                         ` Rasmus
2014-10-03  7:56                         ` Nicolas Goaziou
2014-10-03  8:49                           ` Sebastien Vauban
2014-10-03 10:26                           ` Rasmus
2014-10-03 20:14                             ` Nicolas Goaziou
2014-10-03 20:31                               ` Rasmus
2014-10-05  8:06                                 ` Nicolas Goaziou

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87d2aiesce.fsf@nicolasgoaziou.fr \
    --to=mail@nicolasgoaziou.fr \
    --cc=emacs-orgmode@gnu.org \
    --cc=rasmus@gmx.us \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).