Hello,
Rasmus <ras...@gmx.us> writes:
> Alan did some testing on a slightly older version of this patch and he
> managed to publish his book without errors and with working links. So
> let's give it another shot.
>
> I briefly tested the output of LaTeX, html, texinfo, odt, md, and
> plaintext and made sure links work and that the right text is shown in
> the output.
Thanks. It looks mostly good. Some minor comments follow.
>> 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))
>
> But we are not checking that :CUSTOM_ID is unique.
This is not our problem, but user's.
> In ox-latex you're required to turn on a variable on to get this
> behavior (I could be mistaken here). For now I have done as you
> suggest. But I don't understand why we are favoring CUSTOM_ID here
> over the nice, unique label we've generated?
We could do the same as ox-latex, default to generated label, and
optionally allow users to use raw custom-id instead (with usual caveat).
Meanwhile, I think it is reasonable to stick to the current behaviour.
>> Last line:
>>
>> (list headline (incf num))
>
> Oh incf is quite handy. Didn't know that one.
>
> I leave it as (cons headline (list (incf num))). Why? 'Cause that's
> the format used by `org-export--collect-headline-numbering'. While
> simpler is nicer, I think it's better not to have to consider
> different data structures depending on whether data is from
> `org-export--collect-headline-numbering' or
> `org-export--collect-unnumbered-headline-id'.
I don't get your point. (cons 'a (list 'b)) is equivalent to (list 'a
'b). Why do you think this changes the data structure?
>> (unless (org-some
>> (lambda (h) (org-not-nil (org-element-property :UNNUMBERED h)))
>> (org-export-get-genealogy headline))
>> ...)
>
> Handy. AFAIK BLOB is not a member of (org-export-get-genealogy BLOB)
> (or so the output suggests), so (or · ·) is still needed.
I think
(org-some (lambda (h) ...)
(cons headline (org-export-get-genealogy headline)))
is more elegant.
> + ;; headline linked via CUSTOM_ID
;; Headline linked via CUSTOM_ID.
> + (or (and (string= type "custom-id")
> + (org-element-property :CUSTOM_ID destination))
> + (org-export-get-headline-id destination info)
> + (t (error "Shouldn't reach here"))))
> ;; What description to use?
> (desc
> ;; Case 1: Headline is numbered and LINK has no
> @@ -3073,13 +3063,16 @@ holding contextual information."
> (let* ((class-num (+ (org-export-get-relative-level parent info)
> (1- (plist-get info :html-toplevel-hlevel))))
> (section-number
> - (mapconcat
> - 'number-to-string
> - (org-export-get-headline-number parent info) "-")))
> + (and (org-export-numbered-headline-p parent info)
> + (mapconcat
> + 'number-to-string
Nitpick: #'number-to-string
> + ;; Test if destination is a numbered headline
Missing full stop.
> + (let ((num 0))
> + (org-element-map data 'headline
> + (lambda (headline)
> + (unless (org-export-numbered-headline-p headline options)
> + (cons headline (list (incf num))))))))
See above.
> + (unless
> + (or (org-element-property :UNNUMBERED headline)
> + (org-some (lambda (head) (org-not-nil (org-element-property
> :UNNUMBERED head)))
> + (org-export-get-genealogy headline)))
> + (let ((sec-num (plist-get info :section-numbers))
> + (level (org-export-get-relative-level headline info)))
> + (if (wholenump sec-num) (<= level sec-num) sec-num))))
Per above
(unless (org-some
(lambda (h) (org-not-nil (org-element-property :UNNUMBERED h)))
(cons headline (org-export-get-genealogy headline)))
...)
> +(ert-deftest test-org-export/org-export-get-headline-id ()
> + "Test `org-export-get-headline-id' specifications."
> + (should
> + (equal "sec-1"
> + (org-test-with-parsed-data "* Headline"
> + (org-export-get-headline-id
> + (org-element-map tree 'headline 'identity info t)
> + info))))
> + (should
> + (equal "unnumbered-1"
> + (org-test-with-parsed-data "* Headline\n:PROPERTIES:\n:UNNUMBERED:
> t\n:END:"
> + (org-export-get-headline-id
> + (org-element-map tree 'headline 'identity info t)
> + info))))
> + (should
> + (equal "unnumbered-1"
> + (org-test-with-parsed-data "* Headline\n#+OPTIONS: num:nil"
> + (org-export-get-headline-id
> + (org-element-map tree 'headline 'identity info t)
> + info)))))
I suggest to also test tricky inherited UNNUMBERED properties
* H
:PROPERTIES:
:UNNUMBERED: t
:END:
** H2
:PROPERTIES:
:UNNUMBERED: nil
:END:
*** H3
Regards,
--
Nicolas Goaziou
