[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [wip-cite-new] experimental citeproc-el based activation processor

From: András Simonyi
Subject: Re: [wip-cite-new] experimental citeproc-el based activation processor
Date: Tue, 15 Jun 2021 09:12:21 +0200

Dear All,

thanks for the positive feedback, and sorry for the late reply.

On Sat, 12 Jun 2021 at 16:46, Nicolas Goaziou <mail@nicolasgoaziou.fr> wrote:

> It may make sense to merge it with "oc-csl.el" at some point. If that
> suits you,

Absolutely, thanks for the suggestion!

> In addition, I have a couple of comments:
> - As suggested by Bruce D'Arcus, we might move `org-cite-get-boundaries'
>   in `oc.el' proper, since it is also used elsewhere (at least in
>   "oc-basic.el").

sure, it makes more sense there, especially because I took the
fragment from your code IIRC (apologies for the lack of explicit

> - Nitpick: (car element) => (org-element-type element)

I was actually wondering about this when I wrote the code and if I may
nitpick on the nitpick, I find the documentation a bit confusing in
this respect. If the list representation is meant to be
internal/private only (I guess that is the case), then maybe this
should be more explicit in the docstrings, because now the docstring
of `org-element-context' says

"Return smallest element or object around point.

Return value is a list like (TYPE PROPS) [...]"

Omitting the second part or having something like "Internally, return
value is [...]" would go a long way toward conveying the message that
one shouldn't rely on the list representation.

> - I think it is inefficient to call `org-element-context' in
>   `org-cite-csl-activate--sensor-fun'. You may as well store the parsed
>   object as a text property on the citation during fontification, and
>   read the property in the function above to know where you are.

Thanks for the suggestion, I'll certainly implement this!

> - I am also wondering about the call of `org-element-parse-buffer' in
>   `org-cite-csl-activate-render-all'. It is not wrong per se, but it is
>   only optimal when citation density in every part of the document is
>   not low. This might not be the most common case. The other option is
>   to `re-search-forward' for `org-element-citation-prefix-re' and then
>   call `org-element-context' at point.

>   Of course, optimizing `org-cite-csl-activate-render-all' may not be
>   the top priority, since some latency is to be expected anyway.

Thanks for this as well, I'll switch to the more efficient approach
you suggested.

Best regards,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]