jiri/org-ql
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix: (--value-at) On cache miss, return nil when function does

Browse source 
Previously, on a cache miss, if FN returned nil, the sentinel would be
returned instead of nil.

Closes #78.  Thanks to Josh Moller-Mara (@mm--) for reporting.
This commit is contained in:
Adam Porter 2019-12-21 16:28:51 -06:00
parent b9dfd7d93b
commit 091a732a88
2 changed files with 44 additions and 32 deletions
Download patch file Download diff file Expand all files Collapse all files

63
org-ql.el
View file

@ -447,37 +447,38 @@ Values compared with `equal'."
;; I'd like to use `-if-let*', but it doesn't leave non-nil variables ;; I'd like to use `-if-let*', but it doesn't leave non-nil variables
;; bound in the else clause, so destructured variables that are non-nil, ;; bound in the else clause, so destructured variables that are non-nil,
;; like found caches, are not available in the else clause. ;; like found caches, are not available in the else clause.
(if-let* ((buffer-cache (gethash (current-buffer) org-ql-node-value-cache)) (pcase (if-let* ((buffer-cache (gethash (current-buffer) org-ql-node-value-cache))
(modified-tick (car buffer-cache)) (modified-tick (car buffer-cache))
(position-cache (cdr buffer-cache)) (position-cache (cdr buffer-cache))
(buffer-unmodified-p (eq (buffer-modified-tick) modified-tick)) (buffer-unmodified-p (eq (buffer-modified-tick) modified-tick))
(value-cache (gethash position position-cache)) (value-cache (gethash position position-cache))
(cached-value (alist-get fn value-cache nil nil #'equal))) (cached-value (alist-get fn value-cache nil nil #'equal)))
;; Found in cache: return it. ;; Found in cache: return it.
(pcase cached-value cached-value
('org-ql-nil nil) ;; Not found in cache: call FN, cache and return its value.
(_ cached-value)) (let ((new-value (or (funcall fn) 'org-ql-nil)))
;; Not found in cache: get value and cache it. ;; Check caches again, because it may have been set now, e.g. by
(let ((new-value (or (funcall fn) 'org-ql-nil))) ;; recursively going up an outline tree.
;; Check caches again, because it may have been set now, e.g. by ;; TODO: Is there a clever way we could avoid doing this, or is it inherently necessary?
;; recursively going up an outline tree. (setf buffer-cache (gethash (current-buffer) org-ql-node-value-cache)
;; TODO: Is there a clever way we could avoid doing this, or is it inherently necessary? modified-tick (car buffer-cache)
(setf buffer-cache (gethash (current-buffer) org-ql-node-value-cache) position-cache (cdr buffer-cache)
modified-tick (car buffer-cache) value-cache (when position-cache
position-cache (cdr buffer-cache) (gethash position position-cache))
value-cache (when position-cache buffer-unmodified-p (eq (buffer-modified-tick) modified-tick))
(gethash position position-cache)) (unless (and buffer-cache buffer-unmodified-p)
buffer-unmodified-p (eq (buffer-modified-tick) modified-tick)) ;; Buffer-local node cache empty or invalid: make new one.
(unless (and buffer-cache buffer-unmodified-p) (setf position-cache (make-hash-table)
;; Buffer-local node cache empty or invalid: make new one. value-cache (gethash position position-cache))
(setf position-cache (make-hash-table) (puthash (current-buffer)
value-cache (gethash position position-cache)) (cons (buffer-modified-tick) position-cache)
(puthash (current-buffer) org-ql-node-value-cache))
(cons (buffer-modified-tick) position-cache) (map-put value-cache fn new-value)
org-ql-node-value-cache)) (puthash position value-cache position-cache)
(map-put value-cache fn new-value) new-value))
(puthash position value-cache position-cache) ;; Return nil or the non-nil value.
new-value))) ('org-ql-nil nil)
(else else)))
(defun org-ql--add-markers (element) (defun org-ql--add-markers (element)
"Return ELEMENT with Org marker text properties added. "Return ELEMENT with Org marker text properties added.

13
tests/test-org-ql.el
View file

@ -143,6 +143,7 @@ RESULTS should be a list of strings as returned by
sum 1))))) sum 1)))))
(describe "Caching" (describe "Caching"
(it "Clears value cache after buffer changes" (it "Clears value cache after buffer changes"
;; See <https://github.com/alphapapa/org-ql/issues/59>. ;; See <https://github.com/alphapapa/org-ql/issues/59>.
(with-temp-buffer (with-temp-buffer
@ -159,7 +160,17 @@ RESULTS should be a list of strings as returned by
(goto-char (point-min)) (goto-char (point-min))
(org-ql--value-at (point-min) #'point) (org-ql--value-at (point-min) #'point)
(expect (org-ql--value-at (point-min) #'org-get-heading) (expect (org-ql--value-at (point-min) #'org-get-heading)
:to-equal "Heading 2")))) :to-equal "Heading 2")))
(it "Returns nil when cache misses and function returns nil"
;; See <https://github.com/alphapapa/org-ql/pull/78>.
(with-temp-buffer
(org-mode)
(insert "* Heading 1")
;; FIXME: `--value-at' does not actually move point, so we do it here.
(goto-char (point-min))
(expect (org-ql--value-at (point-min) #'org-get-local-tags)
:to-be nil))))
(describe "Query functions/macros" (describe "Query functions/macros"