add error-handling to key indicator functions

[bdarcus]

... Does occur to me I didn't really include error handling in the key functions, so that's a good next step.

Originally posted by @bdarcus in https://github.com/emacs-citar/citar/issues/753#issuecomment-1478025123

[robmoss]

I don't currently have any notes, and I was about to file a bug for citar-file--has-notes, which returns nil if citar-file--get-notes returns an empty hash table. This causes commands like citar-open to fail, because nil is not callable. As a temporary fix, I've replaced:

(defun citar-file--has-notes ()
  "Return predicate testing whether cite key has associated notes."
  (let ((notes (citar-file--get-notes)))
    (unless (hash-table-empty-p notes)
      (lambda (citekey) (and (gethash citekey notes) t)))))

with:

(defun citar-file--has-notes ()
  "Return predicate testing whether cite key has associated notes."
  (let ((notes (citar-file--get-notes)))
    (if (hash-table-empty-p notes)
        (lambda (_citekey) nil)
      (lambda (citekey) (and (gethash citekey notes) t)))))

I also have a related question: is there a specific reason to use (and (gethash citekey notes) t) instead of (gethash citekey notes)?

[bdarcus]

Thanks for those details.

Off-hand am not sure (I didn't write that code, and haven't ever touched it), but I'll take a look.

[bdarcus]

Am not sure, but maybe the better place to fix it would be in this line?

https://github.com/emacs-citar/citar/blob/0fb2e68e8be9d9657185354b552f454a90d52784/citar.el#L854

WDYT?

[bdarcus]

I've opened this PR; hope to merge it with another fix or two later today.

https://github.com/emacs-citar/citar/pull/760

[robmoss]

Am not sure, but maybe the better place to fix it would be in this line?

https://github.com/emacs-citar/citar/blob/0fb2e68e8be9d9657185354b552f454a90d52784/citar.el#L854

WDYT?

Yeah, that looks like a more general solution. I'll give it a test, I'm not sure if you need to check that (citar-indicator-compiledfunction iproc) is not nil before trying funcall.

[robmoss]

I tried modifying the internal function to check that the compiled indicator function is non-nil, and it worked:

   (lambda (iproc)
     (let ((ifunc (citar-indicator-compiledfunction iproc)))
       (when (and ifunc (funcall ifunc citekey))
         (propertize
          (concat " " (citar-indicator-tag iproc))
          'invisible t))))

[bdarcus]

I added a slightly different version (just using when-let* here:

https://github.com/emacs-citar/citar/pull/760/commits/47d6e931e6d7933d6c8a057a150b57845d4208de

Should I keep the other commit too, or is this one enough?

While you're looking at this code, can you see if you see anything with #759? It's the other little bug I want to fix.

[robmoss]

This when-let* version works for me, and I've now learned about when-let*. Nice job! I'll have a look at #759 ...

Edit: the when-let* commit solves the problem entirely, no need for the other commit :)

[bdarcus]

I ended up merging that, then. It was the main place I was worried about problems. Thanks!