From 1548260c063feafedcff117623ec119f62714f7f Mon Sep 17 00:00:00 2001 From: Richard Boyechko Date: Sun, 9 Jul 2023 16:21:58 -0700 Subject: [PATCH] Fix obscure bug with sorting zk-index buffer by size The bug is rare, but if `zk-id-regexp` happens to match a portion of zk-index buffer that is *not* a zk-id, then `zk-index--current-id-list` will include it in its returned list, which in turn is used in `zk-index--current-file-list` to return a list of files. But since it's not a zk-id, `zk--parse-id` will return nil, resulting in `zk-index--current-file-list` returning a list with at least one nil element. This becomes a problem in `zk-index--sort-size`, since it uses `>` (greater) function to compare file sizes, which signals an error if an argument is nil. However, `file-attributes` (and `file-attribute-size`) can return `nil` if one is passed to them or if the file does not exist (see [Emacs Lisp manual about "File Attributes"][1]). Additionally, `zk-index--format-candidates` does not expect nils in the list of the files it works with, which again causes an error when `string-match` ends up being passed a nil rather than a string. The fix involves having 1) `zk-index--sort-size` ensure that it's always passing numbers to `>`, but since that just propagates the nil inside the list of files returned by `zk-index--sort`, we also make sure 2) `zk-index--format-candidates` does not pass nil to `string-match`. Another possibility would be to have `zk-index--current-file-list` remove nils from the file list it returns (replacing `files` with `(delq nil files)` as the final line), but that results in a small performance loss because `delq` needs to traverse the list looking for nils. I think my two-pronged fix is more robust, anyway, since it deals with the errors at the level where they occur (in the calls to `>` and `string-match`). Of course, we can also wrap those calls in `ignore-errors`, but that would make debugging more difficult in the future. [1]: https://www.gnu.org/software/emacs/manual/html_node/elisp/File-Attributes.html --- zk-index.el | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/zk-index.el b/zk-index.el index fa324da..055a046 100644 --- a/zk-index.el +++ b/zk-index.el @@ -183,7 +183,7 @@ all files in `zk-directory' will be returned as formatted candidates." (zk--directory-files))) (output)) (dolist (file list) - (when (string-match (zk-file-name-regexp) file) + (when (and file (string-match (zk-file-name-regexp) file)) (let ((id (if zk-index-invisible-ids (propertize (match-string 1 file) 'invisible t) (match-string 1 file))) @@ -526,8 +526,7 @@ with query term STRING." "Return list files in current index." (let* ((ids (zk-index--current-id-list (buffer-name))) (files (zk--parse-id 'file-path ids))) - (when files - files))) + files)) (defun zk-index--sort-created (list) "Sort LIST for latest created." @@ -559,8 +558,10 @@ with query term STRING." "Sort LIST for latest modification." (sort list (lambda (a b) - (> (file-attribute-size (file-attributes a)) - (file-attribute-size (file-attributes b)))))) + ;; `>' signals an error if it's passed a nil, but `file-attributes' + ;; can return nil the file does not exist (or when passed a nil). + (> (or (file-attribute-size (file-attributes a)) -1) + (or (file-attribute-size (file-attributes b)) -1))))) ;;; ZK-Index Keymap Commands