Ihor Radchenko <yanta...@posteo.net> writes:

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

changed.

>> +    (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 ...))
>

By this, you mean

(unless where
  (when 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)))))

? 

If so, the FILE check can be put under (unless where ...). For
avoiding code nesting too deep, I didn't do it.

Note: the twos (setq where ...) above seems can't merge into one,
because the code block try to find the id in the given FILE first, then,
if not id is found, goes into the id location update process and search
again.

>> +    ;; 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.
>

fixed, by:

;; Update id location cache.
(when where
  (setq pos (if markerp (marker-position where) (cdr where)))
  (when (or (not (consp loc)) (not (= (cdr loc) pos)))
    (puthash id (cons file pos) org-id-locations)))

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

A grep in codebase give me many references of `org-id-locations', which
indicates that this Enhance Request may involves a more complicated
situation than I expected before. Since I haven't entirely understood
the org-id module yet, and my time allocated to this may be unstable,
and I didn't sign any FSF copyright before, so I will leave this idea
here, and welcome anyone to catch up.

Best Regards.

Reply via email to