Re: [O] [PATCH] org-id-goto doesn't work if buffer is narrowed.

[Nicolas Goaziou]

Puneeth Chaganti <address@hidden> writes:

> Thanks for your careful review and detailed comments.  I've attached
> an updated patch.

Thank you. I have another suggestion about it.

> +(defun org-id-show (id cmd)
> +  "Show an entry with id ID by buffer-switching using CMD.
> +CMD is a function that takes a buffer or a string (buffer name)
> +as an argument, which will be used to switch to the buffer
> +containing the entry with id ID."
> +  (let ((m (org-id-find id 'marker)))
> +    (unless m
> +      (error "Cannot find entry with ID \"%s\"" id))
> +    (unless (equal (current-buffer) (marker-buffer m))
> +      (funcall cmd (marker-buffer m)))

Nitpick: (eq (current-buffer) (marker-buffer m))
  
> +    (when (let ((pos (marker-position m)))
> +         (or (< pos (point-min))
> +             (> pos (point-max))))
> +      (widen))
> +    (goto-char m)
> +    (move-marker m nil)
> +    (org-show-context 'link-search)))

If CMD raises an error, you have a dangling marker in the buffer, which
is not a great idea. I suggest to wrap everything into
a `unwind-protect' and add (set-marker m nil) as an unwindform, i.e.,

  (let ((m (org-id-find id 'marker)))
    (unless m (error "Cannot find entry with ID \"%s\"" id))
    (unwind-protect
        (progn
          (unless (eq (current-buffer) (marker-buffer m))
            (funcall cmd (marker-buffer m)))
          (when (let ((pos (marker-position m)))
                  (or (< pos (point-min))
                      (> pos (point-max))))
            (widen))
          (goto-char m)
          (org-show-context 'link-search))
      (set-marker m nil)))

Regards,