Doerthous <doerth...@gmail.com> writes:

> +  (let* ((loc (org-id--find-id-location id))
> +         (file (if (consp loc) (car loc) loc))
> +         (pos (if (consp loc) (cdr loc)))
> +      org-agenda-new-buffers where buf)
> +    ;; When loc is a cons cell, check whether it's valid or not.
> +    (when (consp loc)

(when pos ...) will be more clean.

> +    (when (and (null where) file)
>        (setq where (org-id-find-id-in-file id file markerp)))
>      (unless where
>        (org-id-update-id-locations nil t)
>        (setq file (org-id-find-id-file id))
>        (when file
>       (setq where (org-id-find-id-in-file id file markerp))))

Maybe you can just put check for FILE under (unless where ...)

(unless where
   (unless file
     ...
     (setq file ...)
     ...)
   (setq where ...))

> +    ;; Update id location cache.
> +    (setq pos (if markerp (marker-position where) (cdr where)))

What if WHERE = nil (not found)?

> +    (when (and where (or (not (consp loc)) (not (= (cdr loc) pos))))
> +      (puthash id (cons file pos) org-id-locations))

This will fail when we go through (unless where ...) branch because POS
will be nil there; WHERE will be populated from `org-id-find-id-in-file'
directly.

> +  ;; org-id-locations: hash, ID -> file or ID -> (file-name . point)
> +  ;; .org-id-locations: alist, file-name -> IDs

Since you are changing the public variable representation, you should
(1) document the breaking change in etc/ORG-NEWS; (2) Update the
docstring.

-- 
Ihor Radchenko // yantar92,
Org mode maintainer,
Learn more about Org mode at <https://orgmode.org/>.
Support Org development at <https://liberapay.com/org-mode>,
or support my work at <https://liberapay.com/yantar92>

Reply via email to