Hi,

Thanks for the comments.  I managed to make the patch less complicated.

Nicolas Goaziou <mail@nicolasgoaziou.fr> writes:

>> +      (let* ((lines (and lines (split-string lines "-")))
>> +	     (lbeg (and lines (string-to-number (car lines))))
>> +	     (lend (and lines (string-to-number (cadr lines)))))
>
> This is not needed. `point-min' and `point-max' refer to the boundaries
> of the area to be included. It avoids relying on the expensive
> `line-number-at-pos' function later.
>
> Moreover, I suggest store (point-max) as a marker since you are going to
> modify contents of the buffer (e.g., adding ID to labels).

I did not know markers but they seem perfect in this case.  The manual
mentions setting markers to nil after use.  I guess it's not necessary
here since they are in a (let ⋯)?

>> +	(goto-char (point-min))
>> +	(while (re-search-forward org-footnote-re nil t)
>> +	  (let* ((reference (org-element-context))
>> +		 (type (org-element-type reference))
>> +		 (footnote-type (org-element-property :type reference))
>> +		 (label (org-element-property :label reference)))
>> +	    (when (eq type 'footnote-reference)
>
> The order is confusing here. First you check if you're really at
> a footnote reference, then you bind LABEL and FOOTNOTE-TYPE. Actually,
> the latter doesn't even need to bound since you use it only once.

I check if I'm at a footnote reference 'cause I never want to deal with a
footnote-*definition* directly.  AFAIK org-footnote-re matches both.  It
seems a footnote-reference can never be at the beginning of line, but I
would still prefer a more explicit test through org-element.

>> +	      (goto-char (org-element-property :begin reference))
>> +	      (when label
>> +		(forward-char 4)
>> +		(insert (format "%d-" id))
>
> This looks wrong. Labels can be "1", "fn:label" or even "fn:" in
> anonymous footnotes. You need to check if label matches "\\`[0-9]+\\'",
> in which case prepending "fn:" is also necessary.

Ah, that explains the "\\`[0-9]+\\'" that always confused me.

>> +		      (let ((definition (org-element-context)))
>> +			(when (and lines
>> +				   (or (< lend (line-number-at-pos
>> +						(org-element-property
>> +						 :contents-begin definition)))
>> +				       (> lbeg (line-number-at-pos
>> +						(org-element-property
>> +						 :contents-begin definition)))))
>> +			  (puthash (org-element-property :label definition)
>> +				   (org-element-normalize-string
>> +				    (buffer-substring
>> +				     (org-element-property
>> +				      :contents-begin definition)
>> +				     (org-element-property
>> +				      :contents-end definition)))
>> +				   footnotes)))))))))))
>
> You don't need this part. Basically move to the definition of the
> current reference in a wide buffer. If you're not within narrowed
> boundaries stored before, put it in the hash table. Otherwise, skip it.
> It doesn't require `org-element-context' or `line-number-at-pos'.

OK.  I do it in a differently now, relying on org-footnote-get-definition.
Or do you have something more low-level in mind?  

The only "bug" *I'm aware of* is that it will pick up the wrong new-label
for footnote for something like [fn:ref with space].  But this is anyway
not a proper label, I think.  Is that OK?

Thanks,
Rasmus

-- 
Sådan en god dansk lagereddike kan man slet ikke bruge mere